-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Django Test Compatibility #23935
Django Test Compatibility #23935
Conversation
subprocess_discovery = subprocess.run( | ||
command, | ||
capture_output=True, | ||
text=True, | ||
env=env, | ||
) | ||
print(subprocess_discovery.stderr, file=sys.stderr) | ||
print(subprocess_discovery.stdout, file=sys.stdout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You might be able to stream the output from the subprocess. The way it is right now, the output will not be seen this later. This could be handled in a separate PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sweet ya let me add a new issue to do this in a separate PR
subprocess_execution = subprocess.run( | ||
command, | ||
capture_output=True, | ||
text=True, | ||
env=env, | ||
) | ||
print(subprocess_execution.stderr, file=sys.stderr) | ||
print(subprocess_execution.stdout, file=sys.stdout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same as above, output from subprocess can be streamed.
@eleanorjboyd thank you very much for this work, which the Django community has been awaiting for years. Is it already released? Is there a documentation on how to use it? |
Yes happy to help and appreciate the community support! Here are the instructions: #73 (comment) It is released on the pre-release version of the extension so you can access it and give it a try! I would appreciate if you are able to try it and let me know how it goes or any bugs! Thanks |
@eleanorjboyd I'm getting this error on Django 5, looks like the verbosity param is missing a level in the command
|
After removing the
|
Hi @msmitherdc thank you so much for trying this! Glad discovery worked and you got the arg issue figured out- I will need to update my instructions to reflect that. Secondly for the module not found error- very interesting. If you see in this file, both discovery and execution are called the same way with the addition to the path variable and then calling the subprocess with that custom runner. https://github.com/microsoft/vscode-python/blob/main/python_files/unittestadapter/django_handler.py. What type of computer are you on? Also what Django and python versions are you on? My best guess is that these lines are the issue https://github.com/microsoft/vscode-python/blob/main/python_files/unittestadapter/django_handler.py#L69C4-L79C56 and that somehow the PYTHONPATH is not being built correctly even though the steps are the same as for discovery. We might be hitting the first flow in the if statement instead of the second and the pathsep isn't working. Let me investigate a bit more and ill get back to you- thanks! |
For the computer and django type, UI VSCode is Mac ARM64 calling over remote-tunnel to docker connection to container app. Django is 5.0.8 with python 3.12 |
@msmitherdc are you able to try it without the remote connection? Sometimes this can add a layer of complexity especially when we are spinning up subprocesses. |
Got it fired up today. A little tricky to setup but great as a first step. Thank you so much for your work on this. One thing I noticed is that, in our case, the Django apps are in a subdirectory. I cam make things work by using the manage.py toplevel option, however, the "open unit test" button on the discovery screen cannot find files. Update, setting the CWD for tests seems to help |
hi @mbidewell, excited to hear you got it up and working! The setup is definitely challenging now and we plan to make improvements so we will look to gather community feedback then. Glad you got it working and happy coding! |
It's a great step forward. One thing I noticed though. I work on 3 projects. 2 seem to work correctly. 1 has a large number of tests. That project never returns when tests are run. Are there currently any known limits? |
@mbidewell What do you mean by never returns? And how many tests are you talking about? Only know limitation right now is its incompatible with custom django runners but otherwise no others have been surfaced. This issue you are facing seems like a bug- would you be able to create an issue for it and we can talk / track it there? |
After some more experimentation I found the issue. If a test run is aborted uncleanly it can leave behind a test database that manage.py will prompt you to remove on the next run. The text for the prompt was being swallowed and manage.py was blocking waiting for a response. Once I manually removed the test DB everything worked. I added --noinput to the args to hopefully prevent the issue. The swallowing of output might be related to: |
implements and thus closes #22206
resolves #73!