Skip to content

Initial Implementation for supporting EXTRA_ARGS for command line options for tests#53

Merged
buddly27 merged 13 commits intopython-cmake:mainfrom
AmeyaVS:add-pytest-args
Feb 9, 2025
Merged

Initial Implementation for supporting EXTRA_ARGS for command line options for tests#53
buddly27 merged 13 commits intopython-cmake:mainfrom
AmeyaVS:add-pytest-args

Conversation

@AmeyaVS
Copy link
Contributor

@AmeyaVS AmeyaVS commented Feb 6, 2025

This PR adds initial support to specify custom EXTRA_ARGS to pytest_discover_tests function in FindPytest.cmake.

Add support for #19 .

Copy link
Collaborator

@buddly27 buddly27 left a comment

Choose a reason for hiding this comment

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

Thanks for your contribution, this is great!

As discussed in my review notes, I feel like this PR should focus on handling extra arguments for running the tests and leave the handling of extra arguments for test discovery to a separate PR, as it would likely require additional checks to ensure they don't interfere with the parsing logic.

Let me know what you think!

@buddly27 buddly27 added the enhancement New feature or request label Feb 8, 2025
@AmeyaVS AmeyaVS changed the title Initial Implementation for supporting PYTEST_ARGS Initial Implementation for supporting EXTRA_ARGS for command line options for tests Feb 9, 2025
@AmeyaVS
Copy link
Contributor Author

AmeyaVS commented Feb 9, 2025

Thanks for your contribution, this is great!

As discussed in my review notes, I feel like this PR should focus on handling extra arguments for running the tests and leave the handling of extra arguments for test discovery to a separate PR, as it would likely require additional checks to ensure they don't interfere with the parsing logic.

Let me know what you think!

I have updated the implementation to reflect the changes.
Let me know if the changes aligns better for the feature of introducing EXTRA_ARGS support to pytest_discover_tests?
For DISCOVER_EXTRA_ARGS, I would also prefer it to be a separate PR.
There are multiple nuances that will have to handle before it being enabled.

Copy link
Collaborator

@buddly27 buddly27 left a comment

Choose a reason for hiding this comment

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

It looks great, thank you! I really appreciate you adding unit tests as well.

@buddly27 buddly27 merged commit cdb37a3 into python-cmake:main Feb 9, 2025
72 checks passed
@AmeyaVS AmeyaVS deleted the add-pytest-args branch February 10, 2025 03:42
@AmeyaVS AmeyaVS restored the add-pytest-args branch February 10, 2025 03:42
@AmeyaVS AmeyaVS deleted the add-pytest-args branch February 15, 2025 03:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants