Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Submit coverage reports to codecov.io on master merge #4270

Merged
merged 7 commits into from
Jul 25, 2018
Merged

Conversation

Kami
Copy link
Member

@Kami Kami commented Jul 25, 2018

This pull request fixes a regression introduced in #4147 and updates the code so we again submit coverage reports to coveralls.io on merge to master.

@Kami Kami changed the title [WIP] Submit coverage reports to coveralls on master merge Submit coverage reports to coveralls on master merge Jul 25, 2018
@Kami
Copy link
Member Author

Kami commented Jul 25, 2018

Alright, codecov coverage submission is working again. Fix is in 7c40ecd.

The issue is related to our code utilizing eventlet which makes coverage use eventlet concurrency coverage mechanism. This means that eventlet library is also needs to be available for coverage combine step for it to work.

That means we also need to install eventlet in addition to codecov when coverage is enabled. Keep in mind that those libraries are installed outside of tests virtual environment and that's why we need to install them again.

Related PR which allowed me to discover the root cause - codecov/codecov-python#153 (codecov has a bug which simply swallows any command output which makes such issues hard / impossible to troubleshoot).

@Kami Kami changed the title Submit coverage reports to coveralls on master merge Submit coverage reports to codecov.io on master merge Jul 25, 2018
@Kami
Copy link
Member Author

Kami commented Jul 25, 2018

We are back in business - https://codecov.io/github/StackStorm/st2?branch=master.

screenshot from 2018-07-25 13-32-59

Full coverage will be available once this branch is merged into master. I only enabled partial coverage for this branch / PR so I could test my fix is working.

I think the issue with combing unit + integration test coverage might still exist, but to get around that, it would require a lot more work and not something I will do in this PR.

We would need to run unit and integration tests in the same target. Right now we run it twice - one for unit and once for integration tests and we submit those reports separately to codecov and I believe codecov doesn't automatically combine them because they are part of different builds.

@Kami Kami merged commit e4ef8fa into master Jul 25, 2018
@arm4b arm4b deleted the coverall_hook_up branch July 25, 2018 12:10
- if [ "${TASK}" = 'ci-unit' ] || [ "${TASK}" = 'ci-integration' ] && [ "${ENABLE_COVERAGE}" = 'yes' ]; then pip install codecov; fi
# NOTE: We need eventlet installed so coverage can be correctly combined. This is needed because we are covering code which utilizes eventlet.
# Without eventlet being available to the coverage command it will fail with "Couldn't trace with concurrency=eventlet, the module isn't installed."
- if [ "${TASK}" = 'ci-unit' ] || [ "${TASK}" = 'ci-integration' ] && [ "${ENABLE_COVERAGE}" = 'yes' ]; then pip install eventlet ; pip install -e "git+https://github.com/Kami/codecov-python.git@better_error_output#egg=codecov"; fi
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/Kami/ 🙄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fork it to the StackStorm org and move it there.

Hopefully upstream project will merge the fix shortly and submit a new version to PyPi.

@Kami
Copy link
Member Author

Kami commented Jul 25, 2018

Everything also looks good on master - https://codecov.io/gh/StackStorm/st2/commit/e4ef8fa8fec1a15b997c1b27dbc863324817f5b7

Good news also appears to be that codecov automatically merges coverage data if it comes as part of the same parent build (those are indeed 3 different builds, but they belong to the same parent Travis build) https://codecov.io/gh/StackStorm/st2/commit/e4ef8fa8fec1a15b997c1b27dbc863324817f5b7/build so it looks like additional changes like the ones I mentioned above might not be needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants