Support list for TAAT() PASS_REGULAR_EXPRESSION and FAIL_REGULAR_EXPRESSION (#464) #481
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ESSION (#464) Turns out that the tribits_add_advanced_test() test-block arguments PASS_REGULAR_EXPRESSION and FAIL_REGULAR_EXPRESSION did not support taking in a list of regexes like the built-in CMake CTest properties of the same names. This commit updates TAAT() to allow a list of arguments and they behave the exact same way as the built-in properties (i.e. matching any). I also updated the documentation to show how to pass in a list of regexes correctly for PASS_REGULAR_EXPRESSION, FAIL_REGULAR_EXPRESSION, FINAL_PASS_REGULAR_EXPRESSION and FINAL_FAIL_REGULAR_EXPRESSION. I also fixed the documentation for tribits_add_test() as well for PASS_REGULAR_EXPRESSION and FAIL_REGULAR_EXPRESSION. Basically, a well constructed CMake function should almost never take in a list of arguments expecting an explicit semi-colon. This was part of the learning in PR #464 with how to treat semi-colons in CMake. This commit was driven by a usage of FAIL_REGULAR_EXPRESSION in Trilinos while testing updated TriBITS as part of #299 when I realized the documentation was wrong for how to deal with these lists of regexes.
rabartlett1972
approved these changes
May 16, 2022
There was a problem hiding this comment.
@KyleFromKitware, I am going to allow the merge and then a post-merge review can be done with the advantage of having the updated documentation deployed. Any issues can be addressed with a follow-up PR.
|
@KyleFromKitware, if you have time, can you please do a post-merge review of this PR? See the "Review Checklist" above. This should not take more than 30 minutes. |
|
Looks good to me. |
bartlettroscoe
commented
May 17, 2022
bartlettroscoe
added a commit
to bartlettroscoe/TriBITS
that referenced
this pull request
Jul 18, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
See the commit log for commit fd681d2 for details.
I noticed this as part of working on testing PR #479 with Trilinos as part of #299. This should have been addressed as part of PR #464.
Review Checklist
TribitsAddTest.cmakeandTribitsAddAdvancedTest.cmakewhich is displayed fortribits_add_test()andtribits_add_advanced_test(). Look for typos, bad logic, etc.If you find any obvious problems (and I already see a couple of them), please create a PR against 'master' to fix them.
TIME LIMIT: According to code review best practices, given this PR has about 200 lines of code, this review should not take any longer than 30 minutes (at a review rate of 400 LOC per hour).