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

Fix publishing of coverage results to coveralls.io (+ misc test config updates) #890

Merged
merged 7 commits into from
Sep 11, 2019

Conversation

lukpueh
Copy link
Member

@lukpueh lukpueh commented Jun 25, 2019

Fixes issue #:
None

Description of the changes being introduced by the pull request:
Code coverage results from our Travis builds have not been published to coveralls.io in a while, due to the required coveralls publishing tool not being properly installed.

Besides fixing publishing of coverage results, this PR cleans up/ updates other testing configurations:

  • Update travis to run on xenial
  • Add a build matrix to run each tox env in a corresponding travis env as per travis/tox best practices.
    https://docs.travis-ci.com/user/languages/python/#using-tox-as-the-build-script
  • Enable Python 3.5 tests, after all we claim to support Python 3.5
  • Remove "only build certain branch" restrictions
  • Use "install" instead of "before_script" to install dependencies.
    Explicitly listing "install" prevents Travis from automatically running pip install -r requirements.txt, which is not necessary because most of those requirements are installed again in each tox environment.
  • Move pylint and bandit calls to tox (pylint requires runtime dependencies to be installed).

Please verify and check that the pull request fulfills the following
requirements
:

  • The code follows the Code Style Guidelines
  • Tests have been added for the bug fix or new feature
  • Docs have been added for the bug fix or new feature

Coveralls is used to publish coverage results online via
coveralls.io.

Travis is already configured to run it "after_success", but this
has failed for a while, because it was not installed.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
- Add a build matrix to run each tox env in a corresponding
  travis env as per travis/tox best practices.
  https://docs.travis-ci.com/user/languages/python/#using-tox-as-the-build-script
- Add Python 3.5 tests
- Remove only build on certain branch restrictions
- Use "install" instead of "before_script"  to install dependencies.
  Explicitly listing "install" prevents Travis from automatically
  running `pip install -r requirements.txt`, which is not necessary
  because most of those requirements are installed again in each
  tox environment.
- Move pylint and bandit calls to tox (pylint requires
  dependencies) to be installed.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@lukpueh
Copy link
Member Author

lukpueh commented Jun 25, 2019

WIP! don't merge!

Coveralls does run now, but it has problems reading the .coverage report...

...
coverage.misc.CoverageException: Couldn't read data from '/home/travis/build/theupdateframework/tuf/tests/.coverage': CoverageException: Doesn't seem to be a coverage.py data file
...
coverage.misc.CoverageException: Couldn't read data from '/home/travis/build/theupdateframework/tuf/tests/.coverage': UnicodeDecodeError: 'utf-8' codec can't decode byte 0xa8 in position 31: invalid start byte

We install coverage inside tox builds to generate test coverage
reports. These reports need to be created with a version supported
by coveralls, which we use (outside of tox) to publish coverage
reports to coveralls.io.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@lukpueh lukpueh force-pushed the fix-coveralls branch 3 times, most recently from 62015c6 to b2dd8e9 Compare June 26, 2019 15:40
This replicates behavior of unittest's `discover` method, and
allows `coverage` and the tool that posts coverage reports to
coveralls.io, i.e. `coveralls`, to record the correct paths and
left-strip the parts leading to the project directory.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@lukpueh lukpueh force-pushed the fix-coveralls branch 2 times, most recently from 4fdc395 to 157167e Compare June 27, 2019 08:57
@lukpueh
Copy link
Member Author

lukpueh commented Jun 27, 2019

Fixed! See most recent Travis builds and corresponding coverage reports on coveralls.

The badge on the main README should be updated once this PR is merged and IIUC we will see GitHub status (as in the table at the bottom) in subsequent PRs, because coveralls needs a reference build.

Copy link

@tanishqjasoria tanishqjasoria left a comment

Choose a reason for hiding this comment

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

I went through the PR and it looks good to me!

This commit partially reverts the workaround introduced by
157167e. Instead of patching the
path, we configure tox to install TUF in editable mode, which makes
the tests run against the same files as if the path were patched.
This makes it so that coverage records paths that it can then
normalize when sending them to coveralls.io (see .travis.yml).

See uptane/obsolete-reference-implementation@af22701
for detailed background information.

As a consequence we can now skip building of sdist and installing it
into a virtual env directory in tox.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
Coverage used to be configured to omit certain directories while
reporting.

This commit slightly optimizes coverage to already omit those
directories while measuring coverage.

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
.travis.yml Outdated
# FIXME: Consider refactoring the tests to not require the test aggregation
# script being invoked from the `tests` directory, so that `.coverage` is
# written to and .coveragrc can also reside in the project root directory, as
# as the convention.
Copy link
Collaborator

Choose a reason for hiding this comment

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

*is

Signed-off-by: Lukas Puehringer <lukas.puehringer@nyu.edu>
@lukpueh lukpueh merged commit 4fb4cb2 into theupdateframework:develop Sep 11, 2019
@lukpueh
Copy link
Member Author

lukpueh commented Sep 11, 2019

Thanks, @adityasaky!

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