CI: refactoring #4812
Merged
CI: refactoring #4812
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
Our CI test system invokes ctest with the name of the given tests it wishes to invoke. ctest (with the `-R` flag) treats this name as a regular expression. Provide anchors in the regular expression to avoid matching additional tests in this search.
This reverts commit a2d73f5. Using clar to propagate the XML settings was a mistake.
Add the clar flags to produce JUnit-style XML output before invocation.
Introduce SKIP_*_TEST variables for Windows builds to match POSIX builds.
ethomson
force-pushed
the
ethomson/ci-refactor
branch
3 times, most recently
from
154dfac
to
44383dc
Compare
pks-t
previously requested changes
Sep 28, 2018
| @@ -41,7 +41,7 @@ die() { | |||
| # test configuration in a single place (tests/CMakeLists.txt) instead of running clar | |||
| # here as well. But it allows us to wrap our test harness with a leak checker like valgrind. | |||
| run_test() { | |||
| TEST_CMD=$(ctest -N -V -R $1 | sed -n 's/^[0-9]*: Test command: //p') | |||
| TEST_CMD=$(ctest -N -V -R "^${1}$" | sed -n 's/^[0-9]*: Test command: //p') | |||
There was a problem hiding this comment.
Ohhh, I love this. It has bothered me too many times
ci/test.ps1
Outdated
| $TestName = $args[0] | ||
|
|
||
| $TestCommand = (ctest -N -V -R "^$TestName$") -join "`n" -replace "(?ms).*\n^[0-9]*: Test command: ","" -replace "\n.*","" | ||
| $TestCommand = "C:\LibGit2\libgit2-2\build\Debug\libgit2_clar.exe -sa" |
There was a problem hiding this comment.
This doesn't look intended? You first set TestCommand to some magic voodoo and then overwrite it with a hard-coded path. I guess the second line was for debugging purposes?
There was a problem hiding this comment.
Oh, I guess you accidentally squashed the fix into the wrong commit. Feel free to fix it up if you want to, I'm happy to have this merged either way.
Similar to the way we parse the ctest output on POSIX systems, do the same on Windows. This allows us to append the `-r` flag to clar after we've identified the command to run.
Don't stop on test failures; run all the tests, even when a test fails.
ethomson
force-pushed
the
ethomson/ci-refactor
branch
from
44383dc
to
429c7f1
Compare
|
Thanks, I did fixup the wrong commit. I've updated this. |
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.
When I introduced #4723, @pks-t asked why we were stopping on the first failure instead of continuing to run all the tests and simply report the errors. Now that our CI run is faster, I've changed our CI to continue and simply report an error at the end.
In addition, I've reverted the CMake changes to enable XML output. It was a gross hack, and we can instead append
-rduring the CI test runs. This is still a bit of a hack, but a less gross one. 馃槈