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

Avoid spurious output during tests #1253

Merged
merged 1 commit into from Dec 4, 2020
Merged

Avoid spurious output during tests #1253

merged 1 commit into from Dec 4, 2020

Conversation

jdufresne
Copy link
Member

@jdufresne jdufresne commented Nov 28, 2020

It is often convenient to use the pytest option "-s" (shortcut for
--capture=no) to view one's own debugging print() output. When there is
already lots of spurious output, it produces lots of noise and it may be
difficult to view the intended debugging output. By avoiding unnecessary
output, it is easier to find.

Tests that have intentional output now assert that output. For example,
the output of the sync command is now asserted. In addition to the
advantage above, this creates a more robust test suite as the expected
behavior is now more explicit, precise, and better covered.

Changelog-friendly one-liner:

Contributor checklist
  • Provided the tests for the changes.
  • Gave a clear one-line description in the PR (that the maintainers can add to CHANGELOG.md on release).
  • Assign the PR to an existing or new milestone for the target version (following Semantic Versioning).

@jdufresne jdufresne added the tests Testing and related things label Nov 28, 2020
@jdufresne jdufresne added this to the 5.5.0 milestone Nov 28, 2020
@jdufresne jdufresne added the maintenance Related to maintenance processes label Nov 29, 2020
@codecov
Copy link

codecov bot commented Dec 3, 2020

Codecov Report

Merging #1253 (7cd9d58) into master (fd81ee0) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1253   +/-   ##
=======================================
  Coverage   99.58%   99.59%           
=======================================
  Files          36       36           
  Lines        2923     2947   +24     
  Branches      332      333    +1     
=======================================
+ Hits         2911     2935   +24     
  Misses          6        6           
  Partials        6        6           
Impacted Files Coverage Δ
tests/conftest.py 100.00% <100.00%> (ø)
tests/test_cli_compile.py 100.00% <100.00%> (ø)
tests/test_repository_local.py 100.00% <100.00%> (ø)
tests/test_repository_pypi.py 100.00% <100.00%> (ø)
tests/test_sync.py 100.00% <100.00%> (ø)
tests/test_utils.py 100.00% <100.00%> (ø)
tests/test_writer.py 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fd81ee0...7cd9d58. Read the comment docs.

tests/conftest.py Outdated Show resolved Hide resolved
tests/test_sync.py Outdated Show resolved Hide resolved
It is often convenient to use the pytest option "-s" (shortcut for
--capture=no) to view one's own debugging print() output. When there is
already lots of spurious output, it produces lots of noise and it may be
difficult to view the intended debugging output. By avoiding unnecessary
output, it is easier to find.

Tests that have intentional output now assert that output. For example,
the output of the sync command is now asserted. In addition to the
advantage above, this creates a more robust test suite as the expected
behavior is now more explicit, precise, and better covered.
Copy link
Member

@atugushev atugushev left a comment

Choose a reason for hiding this comment

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

Thanks! 👍

@jdufresne jdufresne merged commit 749f782 into jazzband:master Dec 4, 2020
@jdufresne jdufresne deleted the cleanup-test-output branch December 4, 2020 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Related to maintenance processes tests Testing and related things
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants