Skip to content

Update 'make test' to run all unit tests#143

Merged
calvinnhieu merged 1 commit intomasterfrom
calvinnhieu-run-all-tests
Jul 30, 2018
Merged

Update 'make test' to run all unit tests#143
calvinnhieu merged 1 commit intomasterfrom
calvinnhieu-run-all-tests

Conversation

@calvinnhieu
Copy link
Copy Markdown

@calvinnhieu calvinnhieu commented Jul 27, 2018

Actually, with this change, make test doesn't run any more tests than the old one (both run 54 tests), but overall code coverage increases by ~25%. Not sure what's going on here, but I'm inclined to ignore this coverage increase (and close this PR) if we can't explain why it's happening.

@calvinnhieu calvinnhieu self-assigned this Jul 27, 2018
@codecov-io
Copy link
Copy Markdown

codecov-io commented Jul 27, 2018

Codecov Report

Merging #143 into master will increase coverage by 23.92%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #143       +/-   ##
==========================================
+ Coverage   61.98%   85.9%   +23.92%     
==========================================
  Files          31      31               
  Lines        1168    1185       +17     
==========================================
+ Hits          724    1018      +294     
+ Misses        444     167      -277
Impacted Files Coverage Δ
hca/util/__init__.py 91.1% <0%> (-0.72%) ⬇️
hca/upload/cli/upload_command.py 82.85% <0%> (+31.42%) ⬆️
hca/upload/cli/creds_command.py 82.35% <0%> (+35.29%) ⬆️
hca/upload/s3_agent.py 71.42% <0%> (+37.46%) ⬆️
hca/upload/cli/list_areas_command.py 100% <0%> (+38.46%) ⬆️
hca/upload/credentials_manager.py 93.93% <0%> (+43.93%) ⬆️
hca/upload/cli/forget_command.py 100% <0%> (+46.66%) ⬆️
hca/upload/api_client.py 88.88% <0%> (+50%) ⬆️
hca/upload/cli/list_area_command.py 100% <0%> (+50%) ⬆️
hca/upload/cli/select_command.py 100% <0%> (+54.54%) ⬆️
... and 4 more

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 ed13906...185f98e. Read the comment docs.

@calvinnhieu calvinnhieu requested a review from ttung July 27, 2018 20:40
Comment thread Makefile Outdated
# https://github.com/HumanCellAtlas/dcp-cli/issues/127
coverage run --source=hca -m unittest discover -v -t . -s test/upload
coverage run --source=hca -m unittest discover -v -t . -s test -p test_dss_*.py
coverage run --source=hca -m unittest discover -v -t . -s test -p test_*.py
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would just remove the -p parameter. The default 'test*.py' seems reasonable. Furthermore, I think without quotes, this could potentially expand to a path.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Do you know what the flags -s and -p do? I don't see -s in the docs, and -p doesn't seem to mean --parallel-mode since removing it breaks the command:

coverage run --source=hca -m unittest discover -v -t . -s test test*.py
Traceback (most recent call last):
  File "/usr/local/Cellar/python@2/2.7.15_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/unittest/__main__.py", line 12, in <module>
    main(module=None)
  File "/usr/local/Cellar/python@2/2.7.15_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/unittest/main.py", line 94, in __init__
    self.parseArgs(argv)
  File "/usr/local/Cellar/python@2/2.7.15_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/unittest/main.py", line 113, in parseArgs
    self._do_discovery(argv[2:])
  File "/usr/local/Cellar/python@2/2.7.15_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/unittest/main.py", line 214, in _do_discovery
    self.test = loader.discover(start_dir, pattern, top_level_dir)
  File "/usr/local/Cellar/python@2/2.7.15_1/Frameworks/Python.framework/Versions/2.7/lib/python2.7/unittest/loader.py", line 204, in discover
    raise ImportError('Start directory is not importable: %r' % start_dir)
ImportError: Start directory is not importable: 'test*.py'
Coverage.py warning: No data was collected. (no-data-collected)
make: *** [test] Error 1

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Just make it coverage run --source=hca -m unittest discover -v -t . -s test

% python -m unittest --help
usage: python -m unittest [-h] [-v] [-q] [--locals] [-f] [-c] [-b]
                          [tests [tests ...]]

positional arguments:
  tests           a list of any number of test modules, classes and test
                  methods.

optional arguments:
  -h, --help      show this help message and exit
  -v, --verbose   Verbose output
  -q, --quiet     Quiet output
  --locals        Show local variables in tracebacks
  -f, --failfast  Stop on first fail or error
  -c, --catch     Catch Ctrl-C and display results so far
  -b, --buffer    Buffer stdout and stderr during tests

Examples:
  python -m unittest test_module               - run tests from test_module
  python -m unittest module.TestClass          - run tests from module.TestClass
  python -m unittest module.Class.test_method  - run specified test method
  python -m unittest path/to/test_file.py      - run tests from test_file.py

usage: python -m unittest discover [-h] [-v] [-q] [--locals] [-f] [-c] [-b]
                                   [-s START] [-p PATTERN] [-t TOP]

optional arguments:
  -h, --help            show this help message and exit
  -v, --verbose         Verbose output
  -q, --quiet           Quiet output
  --locals              Show local variables in tracebacks
  -f, --failfast        Stop on first fail or error
  -c, --catch           Catch Ctrl-C and display results so far
  -b, --buffer          Buffer stdout and stderr during tests
  -s START, --start-directory START
                        Directory to start discovery ('.' default)
  -p PATTERN, --pattern PATTERN
                        Pattern to match tests ('test*.py' default)
  -t TOP, --top-level-directory TOP
                        Top level directory of project (defaults to start
                        directory)

For test discovery all test modules must be importable from the top level
directory of the project.

Copy link
Copy Markdown
Author

@calvinnhieu calvinnhieu Jul 27, 2018

Choose a reason for hiding this comment

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

Ah, I was checking the coverage docs. I guess I overlooked the -m unittest. Thanks!

Copy link
Copy Markdown
Collaborator

@Bento007 Bento007 left a comment

Choose a reason for hiding this comment

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

LGTM

@calvinnhieu calvinnhieu merged commit 3e5fb32 into master Jul 30, 2018
@calvinnhieu
Copy link
Copy Markdown
Author

Thanks to @Bento007, we realized the coverage increase was the result of running 1 coverage command instead of 2 to run the same number of tests. It appears the diff from the Codecov report only compares the 1st coverage runs, thus reporting a coverage diff between coverage run --source=hca -m unittest discover -v -t . -s test/upload (some of the tests) and coverage run --source=hca -m unittest discover -v -t . -s test (all of the tests).

@ttung ttung deleted the calvinnhieu-run-all-tests branch August 1, 2018 21:12
@ttung
Copy link
Copy Markdown
Member

ttung commented Aug 1, 2018

👍

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.

4 participants