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

Correct unittest discovery for n-depth test source trees #2066

Merged
merged 13 commits into from
Jun 28, 2018

Conversation

d3r3kk
Copy link

@d3r3kk d3r3kk commented Jun 28, 2018

Store full testId for files & suites during unittest discovery.

In testing the cause of the problem, I found that the testId for each test method must be prefixed by the testSuite or testFile 'nameToRun'. The testSuite/File nameToRun field was being inserted as the folderName.testClassFileName only, ignoring any further depth of the path to that testSuite/File.

Take for instance:

tests/
  app/
    suite1/
      test_suite1.py 
        [class TestSuite1]
          [function test_something]

The testFunction would have a 'nameToRun' field generated that would be:
tests.app.suite1.test_suite1.TestSuite1.test_something
However, the testSuite would get a 'nameToRun' field generated as such:
test_suite1.TestSuite1
And the testFile would get a 'nameToRun' field generated like so:
TestSuite1.test_something

The testSuite/testFile nameToRun must be the exact prefix to the testFunction's nameToRun in order for that testFunction to be selected for execution.

Fixes #2044

This pull request:

  • Has a title summarizes what is changing
  • Includes a news entry file (remember to thank yourself!)
  • Has unit tests & code coverage is not adversely affected (within reason)
  • Works on all actively maintained versions of Python (e.g. Python 2.7 & the latest Python 3 release)
  • Works on Windows 10, macOS, and Linux (e.g. considered file system case-sensitivity)

@d3r3kk d3r3kk requested a review from DonJayamanne June 28, 2018 02:19
if opts.tests is None:
# Run everything in the test file
tests = suites
else:
# Run a specific test class or test method
for suite in suites._tests:
for cls in suite._tests:
for test_suite in suites._tests:
Copy link
Author

Choose a reason for hiding this comment

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

This is my change, renaming the loop-variable from the clashing suite to the non-clashing test_suite. The rest of the change in this file is trailing whitespace automatically cleaned up by our extension 😄.

- we don't need to test these directly (yet)
- for file-only test select
- for suite-only test select
@codecov
Copy link

codecov bot commented Jun 28, 2018

Codecov Report

Merging #2066 into master will decrease coverage by 0.09%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #2066     +/-   ##
=========================================
- Coverage   79.95%   79.85%   -0.1%     
=========================================
  Files         307      307             
  Lines       14064    14064             
  Branches     2494     2494             
=========================================
- Hits        11245    11231     -14     
- Misses       2807     2821     +14     
  Partials       12       12
Flag Coverage Δ
#MacOS 74.08% <75%> (-0.01%) ⬇️
#Windows 74.16% <75%> (-0.02%) ⬇️
Impacted Files Coverage Δ
src/client/unittests/main.ts 46.69% <ø> (ø) ⬆️
src/client/unittests/common/types.ts 100% <ø> (ø) ⬆️
src/client/unittests/unittest/socketServer.ts 26.02% <0%> (ø) ⬆️
...lient/unittests/unittest/services/parserService.ts 97.67% <100%> (ø) ⬆️
src/client/debugger/PythonProcess.ts 56.25% <0%> (-2.92%) ⬇️
...unittests/common/testVisitors/flatteningVisitor.ts 84.61% <0%> (-2.57%) ⬇️
src/client/interpreter/helpers.ts 88.37% <0%> (-2.33%) ⬇️
src/client/common/process/pythonProcess.ts 93.18% <0%> (-2.28%) ⬇️
...reter/locators/services/cacheableLocatorService.ts 93.87% <0%> (-2.05%) ⬇️
src/client/common/platform/fileSystem.ts 71.42% <0%> (-1.2%) ⬇️
... and 2 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 b769f55...837ccc6. Read the comment docs.

@d3r3kk d3r3kk merged commit a2544d1 into microsoft:master Jun 28, 2018
@d3r3kk d3r3kk deleted the unittest branch June 28, 2018 15:19
@mgoyder
Copy link

mgoyder commented Sep 23, 2018

Which release will this change be in? I've been told that my issue #2660 would be fixed by this change.

@mgoyder
Copy link

mgoyder commented Sep 23, 2018

Never mind, I see this change in the PythonTools\visualstudio_py_testlauncher.py source locally. Issue still remains though :(

@lock lock bot locked as resolved and limited conversation to collaborators Jul 31, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Running all tests in a test class raises an error
3 participants