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

Fix the execution of NPY tests #2842

Merged
merged 4 commits into from
Mar 5, 2024
Merged

Fix the execution of NPY tests #2842

merged 4 commits into from
Mar 5, 2024

Conversation

daljit46
Copy link
Member

@daljit46 daljit46 commented Mar 4, 2024

As mentioned in #2836, when #2437 was merged, NPY tests were mistakenly added to the codebase without accounting for the new testing structure post CMake.
This PR aims to address that following the discussion in #2836.
In particular, the CMake logic for our "unit tests" and any executables (under testing/tools) used by a test has now been modified to account for Python files.

The Python and C++ testing tools are executed by other tests and need to reside in tools.
For C++, the tools are automatically compiled to the build directory.
A Python tool needs to be manually copied.
@daljit46 daljit46 marked this pull request as draft March 4, 2024 10:32
@daljit46 daljit46 requested a review from Lestropie March 4, 2024 10:33
Copy link

github-actions bot commented Mar 4, 2024

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

1 similar comment
Copy link

github-actions bot commented Mar 4, 2024

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

Copy link

github-actions bot commented Mar 4, 2024

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

Copy link

github-actions bot commented Mar 4, 2024

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

@Lestropie Lestropie marked this pull request as ready for review March 5, 2024 00:12
Copy link

github-actions bot commented Mar 5, 2024

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

@Lestropie
Copy link
Member

Did just a little bit of re-arrangement.

  • Renamed from "Python tests" to "Bash tests". These are not Python tests. They are tests that make use of a combination of C++ and Python tools to evaluate a particular C++ capability. The test invocations themselves are just Bash. They differ from the other "unit tests" in that there is no longer a direct correspondence between tool name (as a .cpp file) and execution of the binary compiled from that file as the test itself.

  • The tests initially failed. This happened because function add_bash_tests() defined in cmake/BashTests.cmake is utilised. This function treats each individual line of the input file as an individual test, and explicitly inserts "cleanup" jobs at the head and tail of this list; essentially replicating the behaviour of the old run_tests script. It does this by erasing tmp* and -tmp- relative to the data directory. But for the unit tests, no such data directory existed. For now I've just set it to use the source directory as the data directory; but in Python CLI changes #2678 there will be a new directory for storing data relevant to "unit tests".

Copy link
Member

@Lestropie Lestropie left a comment

Choose a reason for hiding this comment

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

Confirmed working locally & executing in CI.

@Lestropie Lestropie enabled auto-merge March 5, 2024 00:18
@Lestropie Lestropie merged commit 8248365 into dev Mar 5, 2024
6 checks passed
@Lestropie Lestropie deleted the fix_npy_tests branch March 5, 2024 00:21
Lestropie added a commit that referenced this pull request Mar 5, 2024
Updates Python CLI tests proposed as part of #2678 to conform to changes discussed in #2836 and implemented in #2842.
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