Repository navigation
test(e2e): spell the find negation as -not in select-e2e_test - #4748
Aleksei Sviridkin (lexfrei) wants to merge 1 commit into
Conversation
The negated-assertion audit reads a continued line that starts with `!` as a bash statement, so the `! -type d` operand inside the find expression on the continuation line turned the unit lane red on main. `-not` is the same find predicate and is not a statement prefix. Assisted-by: LLM Signed-off-by: Aleksei Sviridkin <[email protected]>
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. 🧰 Additional context used📚 Code guidelines (1)No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (1)
Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 6 remain after this review. 📝 WalkthroughWalkthrough
ChangesSuite Enumeration
Priority: ⬇️ Low Estimated code review effort: 1 (Trivial) | ~2 minutes Change: Bug fix Merge Risk: ⚪ Minimal · up to The change is intended to preserve suite selection, and no failure is established. The unit-test job exercises the helper, so normal CI should confirm it before merge. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings
🧪 Generate unit tests (beta)
🛠️ Fix failing CI checks 💡
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
IvanHunters
left a comment
There was a problem hiding this comment.
LGTM. Verified locally: -not -type d and ! -type d produce byte-identical find output (81 paths), and full_suite_list() returns the same 24 suites. Confirmed the root cause too: on the old spelling hack/bats-no-negated-assert.bats goes red at select-e2e_test.bats:34 (its lexical scan reads the ! after \( as a negated subshell assertion, the audit's own documented over-count), and with -not the audit is green 10/10. select-e2e_test.bats passes on the head. Equivalent predicate, minimal diff, unblocks the unit lane.
#4751) ## What this PR does Unit tests on main are red, and that blocks E2E on every open PR. Three contracts broke the same way. In each case a check from one PR and a line from another merged a few minutes apart on 2026-10-05, with no textual conflict. Any one of them alone keeps the job red, so they are fixed together. The first is the pin on how a suite reads HelmRelease history. #4689 added it. It accepts `$(kubectl ...` and, since #3594, the bounded `$(_cozy_guard_kubectl ...` wrapper. Nothing in front of kubectl is allowed. The monitoring suite from #4678 bounds that read with `$(timeout 30 kubectl ...`, and the bound is wanted. So the pin now accepts an optional `timeout [-k N] N` in front of kubectl. The prefix is spelled out on purpose. Any text would also let through `$(echo kubectl ...)`, which names the shared expansion without running it, and that form is now a refuse case. I checked it with mutations: the new accept cases fail against the old pattern, and a pin that takes any prefix fails a refuse case. The second is the flux install in workflows. #4645 forbids running fluxcd.io/install.sh and moved every workflow to the SHA-pinned fluxcd/flux2 action. The isp-hosted lane from #4712 still ran the script. It now uses the same action and version. The third is the negated-assertion audit. It reads the find operand in `\( ! -type d` in select-e2e_test.bats as a shell negation and fails. The line now says `-not -type d`. This is the same change as #4748, so this PR supersedes it. The audit bug itself is #4752. After the Go fix, the full log of the unit job had only these two bats failures left. Closes #4750 Closes #4755 ### Screenshots ### Downstream repositories - [x] No downstream repository is affected by this change - [ ] [cozystack/website](https://github.com/cozystack/website) - follow-up: - [ ] [cozystack/terraform-provider-cozystack](https://github.com/cozystack/terraform-provider-cozystack) - follow-up: - [ ] [cozystack/ansible-cozystack](https://github.com/cozystack/ansible-cozystack) - follow-up: - [ ] [cozystack/ccp](https://github.com/cozystack/ccp) - follow-up: - [ ] [cozystack/talm](https://github.com/cozystack/talm) - follow-up: - [ ] [cozystack/cozyhr](https://github.com/cozystack/cozyhr) - follow-up: - [ ] [cozystack/cozy-proxy](https://github.com/cozystack/cozy-proxy) - follow-up: - [ ] [cozystack/cozystack-telemetry-server](https://github.com/cozystack/cozystack-telemetry-server) - follow-up: - [ ] [cozystack/external-apps-example](https://github.com/cozystack/external-apps-example) - follow-up: - [ ] [cozystack/examples](https://github.com/cozystack/examples) - follow-up: - [ ] [cozystack/community](https://github.com/cozystack/community) - follow-up: ### Release note ```release-note fix(ci): fix three unit-test contracts that went red on main after adjacent merges. ``` <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * History checks now recognize direct `kubectl` reads and reads through the supported wrapper, including timeout-bounded commands with an optional kill-grace setting. Commands that only include `kubectl` as an argument, use similarly named commands, or place a timeout before the wrapper are not treated as reads. * **Chores** * The hosted end-to-end workflow now uses a pinned Flux version, and suite selection handles directory exclusions consistently. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
|
Closing, #4751 landed the same |
Pull request was closed
What this PR does
Turns the unit lane on main green again.
hack/bats-no-negated-assert.batsfails on main because it reads the continuation line of a multi-linefindinhack/select-e2e_test.batsas a bash statement starting with!. That!is afindoperand inside\( ... \), not a negated assertion, so the finding is a false positive, but it fails every PR rebased onto main and skips its E2E.The two changes met on main without a run in between: #4658 last ran CI before the audit landed, and was merged after it. This PR spells the operand as
-not, which is the samefindpredicate. Teaching the audit to join continued lines is a separate change.Both
hack/bats-no-negated-assert.batsandhack/select-e2e_test.batspass locally.Screenshots
Not a UI change.
Downstream repositories
Release note
Summary by CodeRabbit