weiqingy commented on issue #1035: URL: https://github.com/apache/flink-agents/issues/1035#issuecomment-5390141175
Two corrections to the numbers above, found while working on this. **It is 33, not 42, and across 6 files rather than 5.** The count for `checkpoint_recovery_harness.bats` was `38 grep matches - 5 compound = 33`, which never subtracted the 22 that are the final statement of their block. A final `[[ ]]` supplies the body's exit status and fails correctly even on 3.2, so those were never affected. That file actually has 11. `bootstrap_gum_temp.bats` drops to 0, since all 5 of its matches are inside single-quoted shim-script arguments and are data rather than code. Two files that post-date the original count add 15. | File | Above | Actual | |---|---|---| | `unit/checkpoint_recovery_harness.bats` | 33 | 11 | | `unit/check_license.bats` | - | 10 | | `integration/build_help.bats` | - | 5 | | `unit/verify_example_job.bats` | 4 | 3 | | `unit/revalidate_python_constraint.bats` | 2 | 2 | | `unit/ui_helpers.bats` | 2 | 2 | | `integration/bootstrap_gum_temp.bats` | 1 | 0 | | **Total** | **42** | **33** | **"The bulk of the cleanup is my own" is also wrong.** `checkpoint_recovery_harness.bats` is 11 of 33, about a third. The worst file is `check_license.bats`, which has no final `[[ ]]` at all: every one of its tests ends on a `[ ]`, so all ten of its `[[ ]]` assertions are inert. Three other things that turned up: **The floor is 4.1, not 4.** `run.sh` tests the major version, so it admits bash 4.0, which still has the defect. `[[ ]]` became subject to `set -e` and the ERR trap in 4.1, per Chet Ramey's Bash-4.1 announcement, item j. bats-core says the same in `docs/source/gotchas.rst`, which ships in the version this repo pins and also recommends `|| false` as the fix. **The `(( ))` and `! cmd` classes are both empty here.** Worth stating so nobody re-derives it: all 18 `(( ))` occurrences are arithmetic expansion or `if` conditions, and all 32 lines containing `!` are `[ ! -f x ]` operands, `!=` comparisons, or shebangs in fixture data. Note that a non-final `! cmd` would be inert on every bash including 5.x, not just below 4.1, since `!` exempts a command from `errexit`. **Nothing is silently failing right now**, and CI was never exposed. No vacuous assertion is currently false, and `ci.yml` runs `bash tools/test/run.sh` unqualified, so the same `PATH` lookup feeds both the gate and the test bodies. The exposure is on developer machines, and specifically for anyone who follows the gate's own advice to re-run under `/opt/homebrew/bin/bash`, which satisfies the check and leaves `PATH` untouched. A PR is on the way for both parts. -- 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]
