Add new CLI testing platform #3020
Merged
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
terrelln
force-pushed
the
cli-tests
branch
5 times, most recently
from
bf1432e
to
a833889
Compare
terrelln
changed the title
terrelln
force-pushed
the
cli-tests
branch
6 times, most recently
from
4ea97cd
to
943ef41
Compare
Passing 0 inputs to `DiB_shuffle()` caused an assertion failure where it should just return. A test is added in a later commit, with the initial introduction of the new testing framework. Fixes facebook#3007.
The option `--auto-threads` should still be accepted and parsed, even if `ZSTD_MULTITHREAD` is not defined. It doesn't mean anything, but we should still accept the option. Since we want scripts to be able to work generically. This bug was caught by tests I added to the new testing framework.
`datagen` was printing a `\n` even when it had no other output. Raise the output level for the final `\n` to the minimum output level used. This minor bug was caught by the new testing framework.
Adds the new CLI testing platform that I'm proposing. See the added `README.md` for details.
Add cli-tests to `make test`. This adds a `python3` dependency to `make test`, but not `make check`. We could make this dependency optional by skipping the tests if `python3` is not present.
|
The only remaining failure is in the static-analysis. It seems unrelated to code that I've modified. However, I only see if failing on this PR, so I will attempt to fix it. |
* It couldn't detect that the `fastCoverParams` can't be non-null, since it was just an assertion. * It thought we were accesing `wksp->dtable` beyond the bounds because we were using it to set the `workSpace` value. Instead, compute the workspace size used in a different way.
|
The new Travis CI test failure is an infra error unrelated to the PR. And I don't know why the MSAN regression was skipped, but it passed on the previous revision. So I'm tempted to just merge as-is. |
Cyan4973
approved these changes
Jan 30, 2022
This was referenced Jan 30, 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.
The first 3 commits fix bugs found by the CLI testing platform and fix
The 4th commit adds the new CLI testing platform that I'm proposing.
See
tests/cli-tests/README.mdhere.The final commit hooks the CLI test suite into
make test, but notmake check. This runs in CI everywhere thatmake testruns.