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

Support pytest-describe in testing rewrite #21705

Closed
QuentinFchx opened this issue Jul 28, 2023 · 25 comments · Fixed by #24400
Closed

Support pytest-describe in testing rewrite #21705

QuentinFchx opened this issue Jul 28, 2023 · 25 comments · Fixed by #24400
Labels
area-testing community ask Feature request that the community expressed interest in feature-request Request for new features or functionality needs PR Ready to be worked on

Comments

@QuentinFchx
Copy link

QuentinFchx commented Jul 28, 2023

Type: Bug

Behaviour

Expected vs. Actual

With the following dependencies:

"pytest==7.4.0"
"pytest-describe==2.1.0"

Considering the following test file:

# test_pytest_describe.py

def describe_A():
    def test_1():
        pass

    def test_2():
        pass


def describe_B():
    def test_1():
        pass

    def test_2():
        pass

When discovering tests (with the new adapter, "python.experiments.optInto": ["pythonTestAdapter"]), only the last describe block is registered, here describe_B.
By "registered", I mean :

  • displayed in the tests explorer
  • having a "run/debug test" quick action in the editor

Neither the describe_A block, nor its tests appear anywhere. FWIW, I tested with different test names, not conflicting between describe blocks (describe_B, test_3/test_4 instead of test_1/test_2), the issue sill occurs.

Screenshots Screenshot 2023-07-28 at 14 16 55 Screenshot 2023-07-28 at 14 16 42

In the Output for Python section below, I added an extract of the test collection output which includes the describe_A block. I assume it means the test collection works well but there is an issue down the line?

Steps to reproduce:

  1. See above

Diagnostic data

  • Python version (& distribution if applicable, e.g. Anaconda): 3.9.16
  • Type of virtual environment used (e.g. conda, venv, virtualenv, etc.): Venv
  • Value of the python.languageServer setting: Pylance
Output for Python in the Output panel (ViewOutput, change the drop-down the upper-right of the Output panel to Python)

collected xx items

<Package test>
  [...]
  <Module test_pytest_describe.py>
    <DescribeBlock 'describe_A'>
      <Function test_1>
      <Function test_2>
    <DescribeBlock 'describe_B'>
      <Function test_1>
      <Function test_2>
  [...]

User Settings

Multiroot scenario, following user settings may not apply:

venvPath: "<placeholder>"

languageServer: "Pylance"

linting
• flake8Enabled: true
• mypyArgs: "<placeholder>"
• mypyEnabled: true

formatting
• provider: "black"
• blackArgs: "<placeholder>"

testing
• autoTestDiscoverOnSaveEnabled: false

experiments
• optInto: ["pythonTestAdapter"]

Extension version: 2023.12.0
VS Code version: Code 1.80.2 (Universal) (2ccd690cbff1569e4a83d7c43d45101f817401dc, 2023-07-27T21:05:41.366Z)
OS version: Darwin arm64 22.5.0
Modes:

System Info
Item Value
CPUs Apple M1 (8 x 24)
GPU Status 2d_canvas: enabled
canvas_oop_rasterization: disabled_off
direct_rendering_display_compositor: disabled_off_ok
gpu_compositing: enabled
metal: disabled_off
multiple_raster_threads: enabled_on
opengl: enabled_on
rasterization: enabled
raw_draw: disabled_off_ok
video_decode: enabled
video_encode: enabled
vulkan: disabled_off
webgl: enabled
webgl2: enabled
webgpu: enabled
Load (avg) 3, 3, 2
Memory (System) 16.00GB (0.07GB free)
Process Argv
Screen Reader no
VM 0%
@github-actions github-actions bot added the triage-needed Needs assignment to the proper sub-team label Jul 28, 2023
@QuentinFchx QuentinFchx changed the title When using pytest-describe, test discovery only register the last describe block When using pytest-describe, test discovery only register the last describe block in each file Jul 28, 2023
@eleanorjboyd
Copy link
Member

will investigate, thank you for filing!

@eleanorjboyd
Copy link
Member

What args are required for pytest to recognize describe blocks? I am not seeing even pytest from the CLI recognize it with the sample code you provided. Thanks

@github-actions github-actions bot added the info-needed Issue requires more information from poster label Jul 31, 2023
@QuentinFchx
Copy link
Author

Unless I'm mistaken, no particular arguments are required.
What can I do to help you reproduce, do you need a sample repository? (is there a sample repository with a boilerplate I can start from?)
Otherwise I can try to give you more information about the issue, are there any flags I can use to debug directly in my IDE?

@github-actions github-actions bot removed the info-needed Issue requires more information from poster label Aug 2, 2023
@eleanorjboyd
Copy link
Member

A sample repro would be extremely helpful! Just a folder with a python file (with the tests) then the .vscode folder (with the setttings.json) is what I should need. Thanks!

@github-actions github-actions bot added the info-needed Issue requires more information from poster label Aug 2, 2023
@QuentinFchx
Copy link
Author

I created https://github.com/QuentinFchx/pytestbug, can you tell me if that's working for you?

The interesting part is that on this repository the issue still occurs, but now only the first describe block is taken into account (and not only the last).

@github-actions github-actions bot removed the info-needed Issue requires more information from poster label Aug 3, 2023
@eleanorjboyd
Copy link
Member

Sorry for the delay on this- trying to determine prioritization with the rewrite. Is this a regression on the new rewrite? If so I can get to this more quickly, if it has never worked then I might have to mark this as a feature request. Thanks

@github-actions github-actions bot added the info-needed Issue requires more information from poster label Oct 4, 2023
@QuentinFchx
Copy link
Author

The previous implementation was not working with plugins such as pytest-describe, so I guess it's a feature request 😅.

I might find some time to investigate it further (and hopefully write a PR), but I'm not sure where to start.
Where can I find documentation about what part of the app (folders / files / classes) are entry points of "test discovery"?

Thank you!

@github-actions github-actions bot removed the info-needed Issue requires more information from poster label Oct 5, 2023
@eleanorjboyd
Copy link
Member

I have been planning on looking at plugins but not sure if a single solution will be generally applicable. If you subscribe to that other issue you can stay updated on the progress I make there.

For the PR writing, here is the file that contains all of the pytest logic. What happens is we call pytest in a subprocess with this vscode_pytest as the plugin and then all these hooks are called by pytest which is how we loop into their process. This includes run and discovery so you can look for specific discovery related sections like build_test_tree. You can view the pytest API docs to learn about the hooks here.

Thanks and let me know if you have questions!

@github-actions github-actions bot added the info-needed Issue requires more information from poster label Oct 5, 2023
@QuentinFchx
Copy link
Author

I dived a bit into the code : pytest-describe makes use of pytest "Modules" (as seen here), but it seems the current implementation of the test's tree builder only handles pytest "Class".

Most likely, supporting pytest'sModule as part of the test collection can help fixing this issue.

Are there known limitations of the pytest Module usage?
Otherwise I'll continue to explore.

@github-actions github-actions bot removed the info-needed Issue requires more information from poster label Oct 9, 2023
@eleanorjboyd
Copy link
Member

eleanorjboyd commented Oct 9, 2023

Hi! There are no limitations I can see with Module. I did not implement it in the first place because I did not see where it was used but there was no other reason. Adding module logic would be great since it extends Bases: _pytest.nodes.File, _pytest.python.PyCollector then it should behave similarly to File and Class logic. Thank you for your exploration already let me know if any other questions arise!

@github-actions github-actions bot added the info-needed Issue requires more information from poster label Oct 9, 2023
Copy link

github-actions bot commented Nov 9, 2023

Because we have not heard back with the information we requested, we are closing this issue for now. If you are able to provide the info later on, then we will be happy to re-open this issue to pick up where we left off.

Happy Coding!

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 9, 2023
@eleanorjboyd eleanorjboyd reopened this Nov 27, 2023
@eleanorjboyd eleanorjboyd added needs PR Ready to be worked on and removed info-needed Issue requires more information from poster triage-needed Needs assignment to the proper sub-team labels Nov 27, 2023
@eleanorjboyd
Copy link
Member

Took another look at this, I am seeing describe blocks as "modules" which is the same as "files" in pytest. My concern would be that a "module" check would not be equivalent because it would catch files as well as describe blocks. The issue here is that for all file objects I use the path as the nodeID, which then causes the describe blocks to have the same IDs as their parent path. Steps likely needed would be to add a new else if statement to the type check like this:

elif isinstance(test_case.parent, pytest.Module) and isinstance(test_case.parent.parent, pytest.Module):
            # Create a describe node
            # then create a file node, add describe node as child of file node
  • if the test_case.parent and its parent it is a module then its a describe block with a file block above it (otherwise it would be a Package or Session type as either a test or the session root).

  • would then use the nodeid to calculate the new absolute nodeid for the object we build similar to how it is node with test cases.

@eleanorjboyd eleanorjboyd changed the title When using pytest-describe, test discovery only register the last describe block in each file Support pytest-describe in testing rewrite Dec 19, 2023
@eleanorjboyd eleanorjboyd added feature-request Request for new features or functionality needs community feedback Awaiting community feedback labels Dec 19, 2023
Copy link

Thanks for the feature request! We are going to give the community 60 days from when this issue was created to provide 7 👍 upvotes on the opening comment to gauge general interest in this idea. If there's enough upvotes then we will consider this feature request in our future planning. If there's unfortunately not enough upvotes then we will close this issue.

@eleanorjboyd eleanorjboyd removed their assignment Dec 20, 2023
@golanmelio
Copy link

Yes please, this will be very appreciated!

@shaulzorea
Copy link

+1

@amihay-hollinger
Copy link

please consider this feature request in your future planning.
thanks!

@benjam1337
Copy link

+1

@idodav
Copy link

idodav commented Feb 14, 2024

We @golanmelio really want this!
+1

@AceCodePt
Copy link

+1

4 similar comments
@golankiviti
Copy link

+1

@Nathaniel-Reeves
Copy link

+1

@dinzone
Copy link

dinzone commented Feb 14, 2024

+1

@nkotzrenko
Copy link

+1

@alexkubica
Copy link

+1 🙏

@brettcannon
Copy link
Member

Thank you to everyone who upvoted this issue! Since the community showed interest in this feature request we will leave this issue open as something to consider implementing at some point in the future.

We do encourage people to continue 👍 the first/opening comment as it helps us prioritize our work based on what the community seems to want the most.

@brettcannon brettcannon added community ask Feature request that the community expressed interest in and removed needs community feedback Awaiting community feedback labels Feb 22, 2024
eleanorjboyd added a commit that referenced this issue Nov 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-testing community ask Feature request that the community expressed interest in feature-request Request for new features or functionality needs PR Ready to be worked on
Projects
None yet
Development

Successfully merging a pull request may close this issue.