[Nexthop] Build missing test binaries automatically when running tests #722
+84
−0
Conversation
This file contains hidden or 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
When building only a specific cmake target to save time, getdeps removes the "built marker” under `$builddir/installed/fboss/.built-by-getdeps`, which makes it impossible to subsequently run the unit tests as without this file getdeps bails out with `project fboss has not been built`. What’s more, because of a missing `return` the exit code was 0, which was confusing. This fix ensures that the return code is propagated properly and also improves the cmake test runner to automatically detect missing executables and build them on the fly. This will not rebuild the test binaries if they've already been built and are merely stale (because the source code has changed) but that was already the prior behavior. At least now it's possible to rebuild a single cmake test target and re-run the tests in one shot.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Pre-submission checklist
pip install -r requirements-dev.txt && pre-commit installpre-commit runSummary
When building only a specific cmake target to save time, getdeps removes
the "built marker” under
$builddir/installed/fboss/.built-by-getdeps,which makes it impossible to subsequently run the unit tests as without
this file getdeps bails out with
project fboss has not been built.What’s more, because of a missing
returnthe exit code was 0, whichwas confusing.
This fix ensures that the return code is propagated properly and also
improves the cmake test runner to automatically detect missing
executables and build them on the fly. This will not rebuild the test
binaries if they've already been built and are merely stale (because the
source code has changed) but that was already the prior behavior. At
least now it's possible to rebuild a single cmake test target and re-run
the tests in one shot.
Test Plan
Not needed.