weiqingy opened a new issue, #1035: URL: https://github.com/apache/flink-agents/issues/1035
### Search before asking - [X] I searched in the [issues](https://github.com/apache/flink-agents/issues) and found nothing similar. ### Description `tools/test/run.sh` refuses to start under bash 3.2, and says why: ``` # Bash 4+ is required: bash 3.2 (macOS default) does not trigger `set -e` # ... substring assertions in this suite would silently pass on bash 3.2. ``` The reasoning is right, but the guard only covers the launcher. `bats-core`'s helpers, including `bats-exec-test`, start with `#!/usr/bin/env bash`, so a test body runs under whatever `bash` resolves to on `PATH` rather than under the interpreter that satisfied the gate. On macOS that is `/bin/bash` 3.2, so the check passes and the bodies still run on 3.2. Reproduced with the vendored bats, launched by a bash the gate accepts: ``` $ /opt/homebrew/bin/bash --version | head -1 GNU bash, version 5.3.15(1)-release (aarch64-apple-darwin24.6.0) $ /opt/homebrew/bin/bash tools/test/.bats-cache/bats-core/bin/bats demo.bats 1..3 BODY BASH_VERSION=3.2.57(1)-release BODY bash path=/bin/bash ok 1 reports which bash runs this body REACHED THE LINE AFTER A FALSE ASSERTION ok 2 a bare non-final [[ ]] that is FALSE not ok 3 the same check with || return 1 ``` Test 2 is the problem. Under bash 3.2 `errexit` does not apply to `[[ ]]`, so a false `[[ ]]` that is not the last command in the body neither fails the test nor stops execution. It reports `ok`. The same condition written `[[ ... ]] || return 1` fails correctly, as test 3 shows, and `[ ... ]` fails correctly too. A final `[[ ]]` is fine, since it supplies the body's exit status. Only non-final ones are affected. Counting those across the suites: | File | Non-final bare `[[ ]]` | |---|---| | `tools/test/unit/checkpoint_recovery_harness.bats` | 33 | | `tools/test/unit/verify_example_job.bats` | 4 | | `tools/test/unit/revalidate_python_constraint.bats` | 2 | | `tools/test/unit/ui_helpers.bats` | 2 | | `tools/test/integration/bootstrap_gum_temp.bats` | 1 | | **Total** | **42** | These are real assertions rather than conditions. A representative case, where the first two checks are inert and only the third can fail the test: ```bash @test "wait_for_rest: probe readable but target not blames the field name" { ... [ "$status" -eq 1 ] [[ "$output" == *"so the endpoint is right"* ]] [[ "$output" == *"never parsed 'counts.completedx'"* ]] [[ "$output" == *"NOT evidence"* ]] } ``` The failure mode is quiet in the direction that matters: an assertion that stops holding still reports `ok`, so a suite can go green while covering less than it appears to. It also varies by machine, since a developer whose `PATH` finds a 4+ `bash` first gets real assertions while CI or a stock mac does not, which makes a regression reproduce for some people and not others. Two things seem worth separating: 1. Stop the bodies from running on 3.2, or detect it from inside a body and fail loudly rather than silently degrading. Pinning the interpreter bats uses looks more promising than adding another launcher check, since the launcher check is what already fails to bind. 2. Convert the 42 assertions to a form that fails on every supported bash, so the suite is honest even where 3.2 is what runs. Worth noting for whoever picks this up: `tools/test/unit/checkpoint_recovery_harness.bats` accounts for 33 of the 42, and that file is mine from #992, so the bulk of the cleanup is my own. I will take a first pass at both parts. ### Are you willing to submit a PR? - [X] I'm willing to submit a PR! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
