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

PASS_REGULAR_EXPRESSION tests don't work #4

Closed
MathiasMagnus opened this issue Nov 11, 2019 · 4 comments
Closed

PASS_REGULAR_EXPRESSION tests don't work #4

MathiasMagnus opened this issue Nov 11, 2019 · 4 comments

Comments

@MathiasMagnus
Copy link

I was trying to demonstrate the use of tests using regular expressions instead of just exit codes. I was surprised to see that setting a regular expression as a test property overrides exit code, and even if the exit code is hardcoded to fail, if console output matches the regex, the test still passes as far as CTest is considered.

My guess is that you should be surprised too, because the extension doesn't handle this well. Having a test passing on a regex, ctest and the extension disagree on the test's success.

Screenshot from 2019-11-11 11-24-19

I was trying to look at the code and it seems to me as if the ctest executable would be circumvented altogether and the extension would launch the test on it's own in the working directory and args obtained from the ctest JSON output. This seems like the wrong approach to take, because there is a lot more going on under the hood of CTest which the extension should not try to reproduce. (CTest implements load balancing, processor affinity, etc.) The extension should invoke ctest and parse output and look for test names/labels and Pass/Failed.

Note: the only thing where the extension should circumvent ctest is looking for the DEPENDS property on a test, and auto-reset the state of tests where the executable or any input datafile is newer than the timestamp of the test run. CTest cannot figure this out on its own.

@fredericbonnet
Copy link
Owner

Hi Máté, first of all thanks for your feedback!

You're right, the current version bypasses the ctest command and runs the test command directly. This works in the general case but breaks in such situations you describe. The current behavior was simpler to implement in the first versions of the extension (no parsing needed), but I intend to fix it in the next version. I first have to check if there is any impact on the performances or stability. However don't expect it to support advanced CTest features such as parallel execution until at least a couple versions because it requires a significant amount of work to parse the command output properly, for now the extension will run & report tests sequentially (the fixed code will use the ctest -R option).

Regarding your last note, Test Explorer UI has nice test state management features that would allow such behavior to be implemented in a future version. For now the extension does not auto-update the test list/state and requires manual refresh.

Thanks again!

@fredericbonnet
Copy link
Owner

The newest version should fix this issue, please tell me if it works on your side.

@MathiasMagnus
Copy link
Author

Yes, indeed it does. Thank you for your efforts!

@fredericbonnet
Copy link
Owner

Great!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants