Skip to content

"Quick" testing of Python scripts#2825

Closed
Lestropie wants to merge 2 commits intodevfrom
test_scripts_quick
Closed

"Quick" testing of Python scripts#2825
Lestropie wants to merge 2 commits intodevfrom
test_scripts_quick

Conversation

@Lestropie
Copy link
Member

@Lestropie Lestropie commented Feb 26, 2024

Creating a new PR to deprecate #2154 given the incompatibility of some of those differences with a cmake-based dev.

At initial posting, this just includes a single text file.

  • Determine appropriate filesystem location for this file.
    CI: Perform limited script testing #2154 kind of hacked its way into providing this test in terms of setting the data location / working directory. I doubt that just dumping this file in its current location will work, since it needs to (obtain and) set its location to the script data directory.
  • Add this file to the list of tests to be performed as part of CI.
    Obviously will need to look at its total runtime to see if it's prohibitive to include as part of CI.
    Edit: Evaluating this set of tests within a Python 3.7 environment could kill two birds with one stone (Test Python 3.7 compatibility #2821).

Initial contents of this file included in this commit are taken directly from #2154.
@Lestropie Lestropie changed the title Test scripts quick "Quick" testing of Python scripts Feb 26, 2024
@github-actions
Copy link

clang-tidy review says "All clean, LGTM! 👍"

@Lestropie Lestropie mentioned this pull request Feb 28, 2024
@Lestropie
Copy link
Member Author

@daljit46 Open to feedback on how to best achieve this, given I'd like to combine it with #2821 before merging #2678.

Previously, this was slated to be achieved by adding an extra text file, which could be specified as the input argument to run_tests. With cmake this makes less sense: the test suite wasn't built from the ground up with ctest in mind, and so the direct translation from what we had to cmake is a little hacky given that origin (#2789 could hopefully help improve that a little), and the proposal here feels like exacerbating that. What's proposed here isn't a new set of tests; it's just a chosen subset of the existing tests. So I think what would better suit would be to create a new test label? This would be reasonably easy if the list of tests was explicit, but harder when it's grepped. #2789 would at least give each a more meaningful name for identification, but there'd still need to be a way to cross-check tests against this list as they are registered.

(On reading for the first time I also wonder if using labels to separate between C++ command / Python command / unit tests would be better than grepping on test names?)

@daljit46
Copy link
Member

If this suite of tests is under the category of "scripts", then I think it would make sense to place this under testing/scripts.

If we want to create a subset of an existing set of tests, then I think we need a way to parse a "label" for a test from a text file. Something like a comment or another syntax that tells us the specific label of the test. Then, we can easily run a specific set of test matching a label.
CMake does support labels as part of a test's properties, so we could make use of that (BTW I can't remember why I didn't add labels to our current tests, possibly because I intended to do it at a later point, but we definitely should do that regardless of this PR).

@Lestropie Lestropie mentioned this pull request Mar 18, 2024
9 tasks
@Lestropie
Copy link
Member Author

Given dc3566f in #2865 lists each test explicitly for cmake, it should be possible there to manually add a label to those tests of Python commands that are manually chosen to execute as part of CI, rather than relying on the interrogation of test shell script content. Since that is already listed as one of the tasks for #2865, I'm content to close this PR, though it will continue to serve as a reference for the set of tests that have previously been flagged as suitable for inclusion in CI.

@Lestropie Lestropie closed this Mar 25, 2024
@Lestropie Lestropie deleted the test_scripts_quick branch August 27, 2025 01:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants