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

Python Unittest isues #7176

Closed
rob-miller opened this issue Sep 3, 2019 · 19 comments
Closed

Python Unittest isues #7176

rob-miller opened this issue Sep 3, 2019 · 19 comments
Assignees
Labels
info-needed Issue requires more information from poster

Comments

@rob-miller
Copy link

Issue Type: Bug

Synopsis: unexpected behaviours - (1) running a file of tests doesn't run all tests in the file, (2) auto-configuration of testing subdir doesn't seem complete/correct; feature request - (3) nice if could have a 'run tests' button for a single file or selected subset of tests.

My test case is biopython - not small or minimal, but easy to access and a good example of large, collaborative project that I don't get to change the structure of to make things work. I believe I have all the Unittest functionality working, below is what I had to do to make it so. I think with the info below #5252 can be closed?

Tests are in a 'Tests' subdirectory, named test_*.py, based on python unittest. Putting this info into the Testing configuration tool, it set up python.unitTest.cwd to ${workspaceFolder}/Tests and the -s option in python.unittest.args to ./Tests.

Issue: In order to make test discovery work, I needed to change .Tests to ${workspaceFolder}/Tests in python.unittest.args -s option.

Issue: When I run a test_*.py file with the green 'play' arrow, the '?' changes to a green checkbox indicating test passed. However, on opening the file in the outline pane, the indication is that only the first test has been run.

subtests

Issue: When I click the 'bug' icon next to the 'play' arrow to debug a unittest on a test that reads from a subdirectory, the test is running in ${workspaceFolder} instead of ${workspaceFolder}/Tests -- even though the run test ('play') succeeded fine. To fix this I created a test entry in launch.json (entry found in some old files, don't know where it came from so new user would need more help here) and added a $cwd entry:

   {
        "name": "Unit Tests",
        "type": "python",
        "request": "test",
        "cwd": "${workspaceFolder}/Tests"
    } 

Feature Request: Biopython has 287 files in the Tests subdir, it would be really nice if the 'Run tests' button in the bottom status bar could be configured to e.g. just run the 12 files of tests relevant to the section of code that I am working on.

Despite these issues, I really, really applaud the testing framework in VS Code! I have never written unit tests before (suspect I'm not alone in this), and having the nice, slick framework and buttons all set up makes me want to implement and use them. Trying not to overstate this, but having this all working easy and friendly really seems like it could have a huge impact on the world of coding quality.

Extension version: 2019.8.30787
VS Code version: Code 1.37.1 (f06011ac164ae4dc8e753a3fe7f9549844d15e35, 2019-08-15T16:16:34.800Z)
OS version: Darwin x64 18.7.0

System Info
Item Value
CPUs Intel(R) Core(TM) i5-6500 CPU @ 3.20GHz (4 x 3200)
GPU Status 2d_canvas: enabled
flash_3d: enabled
flash_stage3d: enabled
flash_stage3d_baseline: enabled
gpu_compositing: enabled
multiple_raster_threads: enabled_on
native_gpu_memory_buffers: enabled
oop_rasterization: unavailable_off
protected_video_decode: unavailable_off
rasterization: unavailable_off
skia_deferred_display_list: disabled_off
skia_renderer: disabled_off
surface_synchronization: enabled_on
video_decode: enabled
viz_display_compositor: disabled_off
webgl: enabled
webgl2: enabled
Load (avg) 2, 2, 2
Memory (System) 32.00GB (9.98GB free)
Process Argv
Screen Reader no
VM 0%
@ghost ghost added the triage-needed Needs assignment to the proper sub-team label Sep 3, 2019
@karrtikr karrtikr added the triage label Sep 3, 2019
@ghost ghost removed the triage-needed Needs assignment to the proper sub-team label Sep 3, 2019
@kimadeline kimadeline assigned karrtikr and unassigned kimadeline Sep 3, 2019
@karrtikr
Copy link

karrtikr commented Sep 3, 2019

Hi @rob-miller 👋

Issue: In order to make test discovery work, I needed to change .Tests to ${workspaceFolder}/Tests in python.unittest.args -s option.

It should be ./Tests, not .Tests, please try that. This doesn't seem like an issue.

Issue: When I run a test_*.py file with the green 'play' arrow, the '?' changes to a green checkbox indicating test passed. However, on opening the file in the outline pane, the indication is that only the first test has been run.

This is the expected behavior right now. The logic is,
If any test fails, the parent should be failure, otherwise pass if all tests that ran passed, and not run if no tests ran.

So a node is '?' only if all the nodes below it are '?'. If you wish to know more details about a test node, you can just hover over any test item to see if it contains any tests that did not run.

Feature Request: Biopython has 287 files in the Tests subdir, it would be really nice if the 'Run tests' button in the bottom status bar could be configured to e.g. just run the 12 files of tests relevant to the section of code that I am working on.

It could be useful, could please open up a new issue on this feature request, so that we can make a decision on whether we want this.

@karrtikr
Copy link

karrtikr commented Sep 3, 2019

subtests

Btw are the test nodes with '?' being skipped intentionally?(using unittest.skip decorator or something) Try running them individually.

@karrtikr
Copy link

karrtikr commented Sep 4, 2019

Issue: When I click the 'bug' icon next to the 'play' arrow to debug a unittest on a test that reads from a subdirectory, the test is running in ${workspaceFolder} instead of ${workspaceFolder}/Tests -- even though the run test ('play') succeeded fine. To fix this I created a test entry in launch.json (entry found in some old files, don't know where it came from so new user would need more help here) and added a $cwd entry:

I will follow up later regarding this issue.

@rob-miller
Copy link
Author

Cheers @karrtikr,

Issue: In order to make test discovery work, I needed to change .Tests to ${workspaceFolder}/Tests in python.unittest.args -s option.

It should be ./Tests, not .Tests, please try that. This doesn't seem like an issue.

Sorry, typo in my bug report - it was script-generated as ./Tests and I had to change to ${workspaceFolder}/Tests. When left as ./Tests, test discovery did not find any tests.

Issue: When I run a test_*.py file with the green 'play' arrow, the '?' changes to a green checkbox indicating test passed. However, on opening the file in the outline pane, the indication is that only the first test has been run.

This is the expected behavior right now. The logic is,
[edit]
Btw are the test nodes with '?' being skipped intentionally?(using unittest.skip decorator or something) Try running them individually.

No, and this is the issue I am reporting. Referring to the screenshot, when I run (green arrow) test_PDB_parse_pdb_header.py, I get the result shown (only the test_parse_header_line appears to have run, and if I had not opened the selected line I would think that all four sub-tests ran and passed). If I run from the ParseReal line, all four sub-tests run and succeed.

I will open the feature request point separately as you suggest.

@karrtikr
Copy link

karrtikr commented Sep 4, 2019

No, and this is the issue I am reporting. Referring to the screenshot, when I run (green arrow) test_PDB_parse_pdb_header.py, I get the result shown (only the test_parse_header_line appears to have run, and if I had not opened the selected line I would think that all four sub-tests ran and passed). If I run from the ParseReal line, all four sub-tests run and succeed.

I am able to confirm this, thanks.

@thornhillcody
Copy link

I've run into this same problem. It really should be treated as a bug, not a request for a new feature. When running a module of tests, it doesn't make sense that only the first test case should run. I've included a zip of a very small project for you to reproduce. In the Test window, just click the run button on the module as shown:
image

pyunittestproblem.zip

@karrtikr
Copy link

karrtikr commented Sep 4, 2019

@thornhillcody Like I commented earlier, I am able to confirm this as a bug. This is not expected. Thanks for giving a minimal sample btw, it really helps!

@karrtikr
Copy link

karrtikr commented Sep 4, 2019

No, and this is the issue I am reporting. Referring to the screenshot, when I run (green arrow) test_PDB_parse_pdb_header.py, I get the result shown (only the test_parse_header_line appears to have run, and if I had not opened the selected line I would think that all four sub-tests ran and passed). If I run from the ParseReal line, all four sub-tests run and succeed.

@rob-miller @thornhillcody This issue is a duplicate of #4567. Can you guys please upvote that issue, it helps us prioritize it.

@karrtikr
Copy link

karrtikr commented Sep 4, 2019

Sorry, typo in my bug report - it was script-generated as ./Tests and I had to change to ${workspaceFolder}/Tests. When left as ./Tests, test discovery did not find any tests.

Unfortunately I am not able to reproduce this. I had used the following settings:

{
    "python.pythonPath": "<path to python>",
    "python.testing.unittestArgs": [
        "-v",
        "-s",
        "./Tests",
        "-p",
        "*test_*.py"
    ],
    "python.testing.pytestEnabled": false,
    "python.testing.nosetestsEnabled": false,
    "python.testing.unittestEnabled": true
}

I am able to discover and run tests just fine. Can you please paste your settings.json?

@karrtikr
Copy link

karrtikr commented Sep 4, 2019

Issue: When I click the 'bug' icon next to the 'play' arrow to debug a unittest on a test that reads from a subdirectory, the test is running in ${workspaceFolder} instead of ${workspaceFolder}/Tests -- even though the run test ('play') succeeded fine. To fix this I created a test entry in launch.json (entry found in some old files, don't know where it came from so new user would need more help here) and added a $cwd entry:

The default value of cwd is ${workspaceFolder}, so it's not surprising. Even when the cwd is set to ${workspaceFolder}, you should be able to debug tests. It's just that these particular tests are assuming that they will always be run from ${workspaceFolder}/tests directory (which is not correct, as tests can be run from anywhere), due to which debugging fails.

So it's not a bug, but we can do a few things here out of the box:

  • Set cwd by default pointing it to directory set by -s flag, instead of the ${workspaceFolder}
  • Users aren't aware of "request": "test" entry in launch.json. We can probably add it in suggestions, so that they can atleast know they can set the current working directory :
    image

The second option doesn't seem right, because the user is not supposed to run the "test" debug configuration. So we could go with option 1 as a feature request.

@karrtikr
Copy link

karrtikr commented Sep 5, 2019

I created a new issue #7204 to track this, I hope that addresses your concern.

@ghost ghost removed the triage label Sep 5, 2019
@rob-miller rob-miller reopened this Sep 5, 2019
@ghost ghost added the triage-needed Needs assignment to the proper sub-team label Sep 5, 2019
@rob-miller
Copy link
Author

rob-miller commented Sep 5, 2019

I created a new issue #7204 to track this, I hope that addresses your concern.

Thanks, I have upvoted #7204. Sorry I hit the 'close' button

@rob-miller
Copy link
Author

rob-miller commented Sep 5, 2019

  • Users aren't aware of "request": "test" entry in launch.json. We can probably add it in suggestions,

I was thinking that a 'request: test' entry should be generated in launch.json as needed by test configuration, but did not realise it was not required for the test framework in general. Otherwise it seems like several 'user discovery' steps are still needed to make things work after s/he described the desired situation in the test configuration sequence?

Ideally this debug entry would be auto-selected when debug icon clicked in test explorer, but I can see how this might cause more problems.

@rob-miller
Copy link
Author

Unfortunately I am not able to reproduce this. I had used the following settings:

I am able to discover and run tests just fine. Can you please paste your settings.json?

OK, I have done more testing and conclude that the issue is a conflict with the python.testing.cwd entry. There is caching involved to add confusion, so on the Mac I have to close the VS Code window (but not 'quit' from the menu) and re-open at the "Editing evolved" screen where I pick the folder to work on to see the effect of changing python.testing.cwd.

In my case I need "python.testing.cwd": "${workspaceFolder}/Tests" in order for the tests to succeed, as they read files from Tests/subdir. I think setting testing.cwd does not happen in the test configuration process. With this setting, python.testing.unittestArgs "-s ." and "-s ${workspaceFolder}/Tests" work for test discovery, but "-s ./Tests" does not (this now makes sense to me).

Please see if you can reproduce this? I can see that it would be expected behavior but it was confusing.

Not knowing the further ramifications especially for the other testing frameworks, it seems like a better situation here would be for test configuration to set testing.cwd for a testing subdir and leave it out of testing.unittestargs (no -s option at all works for me). This way the user would not have to discover testing.cwd if they need it.

@ghost ghost removed the triage-needed Needs assignment to the proper sub-team label Sep 5, 2019
@karrtikr
Copy link

karrtikr commented Sep 5, 2019

Ideally this debug entry would be auto-selected when debug icon clicked in test explorer, but I can see how this might cause more problems.

Yes, it's better not to expose it to users. We add the configuration ourselves internally while sending the debug configurations to debugger.

Will be looking into the other issue shortly.

@karrtikr karrtikr added the info-needed Issue requires more information from poster label Sep 5, 2019
@karrtikr
Copy link

karrtikr commented Sep 6, 2019

If you have both "python.testing.cwd": "${workspaceFolder}/Tests" and "-s ./Tests", then it won't work because "./Tests" would be resolved to "${workspaceFolder}/Tests/Tests". I think you realise this now.

I understand it was confusing but I don't think there's much we can do here. -s option is definitely needed because it gives the location where to discover tests. It's just for your particular configuration, it didn't work. Also setting "python.testing.cwd" could affect other aspects of testing, so setting it just for your case doesn't seem right.

Thanks for reporting these anyways. Closing it now as issues has been addressed.

@karrtikr karrtikr closed this as completed Sep 6, 2019
@ghost ghost removed the triage label Sep 6, 2019
@karrtikr
Copy link

karrtikr commented Sep 6, 2019

You can let me know if there's a better approach here, and we can reconsider doing something about it.

@rob-miller
Copy link
Author

Yes, I understand the situation.
May I suggest adding some further warning or information about the conflict to the current testing.cwd and testing.unittest text:

Python › Testing: Cwd
Optional working directory for tests.

Python › Testing: Unittest Args
Arguments passed in. Each argument is a separate item in the array.

Not sure what the best wording is, but somehow something should highlight an issue if testing.cwd is set. Presumably testing.cwd similarly affects the other testing frameworks?

@karrtikr
Copy link

karrtikr commented Sep 7, 2019

So basically the message we want to convey is that, if testing.cwd is set, then . in directory paths resolve to the value of the setting, right? (If you would like this, please open up a new issue on this, marking it as code health). I think it would be better if somehow let user know the directory user is discovering tests in. Maybe in the logs.
And yes, it affects other frameworks.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 15, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
info-needed Issue requires more information from poster
Projects
None yet
Development

No branches or pull requests

5 participants