Repository navigation
SOLR-18609: Packaging BATS tests accept failures they should catch: harden the assertions - #5299
Draft
nick-boss-tech wants to merge 5 commits into
Draft
nick-boss-tech wants to merge 5 commits into
nick-boss-tech wants to merge 5 commits into
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
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.
🤖 AI text below 🤖 (reviewed by Nick Shanin)
Draft status is deliberate: parts of this suite can only run under upstream CI, as the Limits section states. The change is complete and ready for review.
Jira: SOLR-18609. This work grew out of the one-line BATS fix in SOLR-18565 and the example in PR #5225.
What happens today
Several packaging BATS suites assert on command output with ad-hoc shell comparisons that can pass when they should fail, and that hide the actual output when they do fail.
Suites capture command output into local variables and compare with
[[ ]]tests, contrary to the repo's own guidance for these files (userunwithassert_output/refute_output). A partial match can satisfy a[[ == *...* ]]comparison the author intended as exact, and a failure prints the comparison rather than what the command produced.What this change does
Fourteen packaging test and helper files move to the bats-support assertion helpers, and a new suite covers the helpers themselves.
Assertions across the suites are converted to
runplusassert_output/refute_outputform, the rolling-upgrade health checks route through the sharedassert_healthyhelper, and the newtest_bats_assertions.batsexercises the shared assertion helpers directly (including the TLS helpers: a connection refusal or a closed channel alone cannot satisfy a TLS assertion, a handshake failure can, and an unrelated TLS failure cannot satisfy a certificate assertion) so a broken helper fails loudly instead of silently passing every suite that uses it. No production code changes.Proof
The helper suite passes 14 of 14 at the current head on both JDK versions, its guards are mutation-proven, and the repository checks pass; the Solr-starting matrix and the Docker execution are named as unproven in Limits.
Verified 2026-11-05 at
f342e160417(the review-follow-up commit over99d4b8e68b): thetest_bats_assertions.batssuite passes 14 of 14 in JDK 21 and JDK 25 environments, including the five new hostname-predicate cases. Weakening the predicate in isolation fails the named cases: wrapped-message-only matching fails the direct IP and DNS cases, ignoring the log offset fails the earlier-diagnostics case, accepting arbitrary handshake text fails the unrelated-failures case, and removing the wrapped-diagnostic recognition fails its positive case. The six original helper guards (failed ZK listing, nested healthy status, nonzero exit, connection refusal, bare closed channel, unrelated TLS error) each fail their named case when weakened. The new rolling-upgrade CI job's report guard was executed against fixture reports: a single passing case is accepted, and missing, empty, skipped, failed, errored, and two-case reports are all rejected. Tidy,git diff --check, the packaging module check, and rootcheck -x testall pass at this head.Limits
The Solr-starting suites have not been re-run at this head, and the Docker rolling-upgrade case has still never executed.
The follow-up wires a dedicated CI job that builds the candidate image and requires an executed, passing rolling-upgrade case; that job has not run yet, so the rolling upgrade itself remains unproven until upstream CI or a Docker host executes it. The matched/mismatched client control for the hostname predicate is likewise unrun: the predicate is proven against the JDK producer forms and fixtures, not against a live pair of clients. The earlier verification round's matrix numbers quoted for the parent head remain historical and are not current-head proof.
Nick Shanin directed the work and takes responsibility for this contribution.