claudiomaradonna/azkaban
1
1
Fork 0
Code Issues Pull requests Packages Activity Actions

test: remove tests that repeat another test #73

Merged
claudiomaradonna merged 3 commits from test/drop-duplicate-tests into master 2026-11-11 15:08:20 +01:00
Conversation 0 Commits 3 Files changed 19 +135 -682
motoko commented 2026-11-11 14:47:40 +01:00
Collaborator
Copy link

Why

The project is tested by a Bash suite that runs the launcher and the
installer against a stub container engine: 352 tests in two levels,
unit and integration. Its rule is one test per behavior at each level,
so that whoever changes a behavior edits one test.

An audit of the suite found 30 tests that repeat another of the same
level and seven that repeat a case inside themselves. A contributor
changing, say, how a launcher setting is read met the same assertion in
three files. Three of the copies were worse than redundant: they passed
with the code they name removed, so they read as guards and guarded
nothing.

The suite's speed is not the reason: it runs in about six seconds
before and after.

What changed

Before, 352 tests. Now 323, or 325 on top of the lint pull request,
which adds two: 15 tests are removed because another test asserts the
same behavior through the same code path, and 14 are merged into the
test that keeps the behavior, which gains every assertion the other
had. Seven tests lose a case another test already asserts. Each removal
was read against its survivor twice, by the implementer and by a
reviewer.

Only files under tests/ change; the launcher and the installer are
untouched. Two removals the audit proposed stay out because they would
have dropped a guard nothing else holds: OpenCode's test that the
identity's env file is never mounted, and the scan that checks where
--cidfile sits among the engine's options. Twelve surviving tests are
renamed for what they now assert.

How to verify

Have Git, GNU Make, Bash 4.4 or newer, and script from util-linux, on
Linux. No container engine is needed.

git clone https://git.unitoo.it/claudiomaradonna/azkaban.git
cd azkaban
grep -h '^ *run_test ' tests/unit/*.sh tests/integration/test_*.sh | wc -l
sed -i '/^            --no-tty)$/,/^                ;;$/d' bin/azkaban
TEST_FILTER=no_tty_cli_control bash tests/integration/test_runtime.sh
git checkout bin/azkaban

The sed line breaks the launcher on purpose: it deletes the handling
of --no-tty. On master the count is 352, and of the two tests of
that option one still passes,
ok test_common_no_tty_cli_control_is_always_accepted, while the
other fails: the first guards nothing. After
git switch test/drop-duplicate-tests the count is 325, the test
that passed is gone, and the one that fails is the only one left,
FAIL: test_common_no_tty_cli_control_wins_under_a_real_terminal.
Then make test passes with bin/azkaban restored.

Checks run on the tip: make lint && .agents/hooks/shellcheck.sh repo && make test
passes, 325 tests. Since no code under test changes, every surviving
test passes on master's code as it does here. Not tried: the audit's
other mutation checks were not repeated on the final tip.

Considerations

This branch sits on top of #69, #70, #72, and #71, in that order, so it
merges last; until they merge, this page lists their commits too. The
audit's proposals that add or reshape shared test helpers are left for
a decision of their own.

## Why The project is tested by a Bash suite that runs the launcher and the installer against a stub container engine: 352 tests in two levels, unit and integration. Its rule is one test per behavior at each level, so that whoever changes a behavior edits one test. An audit of the suite found 30 tests that repeat another of the same level and seven that repeat a case inside themselves. A contributor changing, say, how a launcher setting is read met the same assertion in three files. Three of the copies were worse than redundant: they passed with the code they name removed, so they read as guards and guarded nothing. The suite's speed is not the reason: it runs in about six seconds before and after. ## What changed Before, 352 tests. Now 323, or 325 on top of the lint pull request, which adds two: 15 tests are removed because another test asserts the same behavior through the same code path, and 14 are merged into the test that keeps the behavior, which gains every assertion the other had. Seven tests lose a case another test already asserts. Each removal was read against its survivor twice, by the implementer and by a reviewer. Only files under `tests/` change; the launcher and the installer are untouched. Two removals the audit proposed stay out because they would have dropped a guard nothing else holds: OpenCode's test that the identity's env file is never mounted, and the scan that checks where `--cidfile` sits among the engine's options. Twelve surviving tests are renamed for what they now assert. ## How to verify Have Git, GNU Make, Bash 4.4 or newer, and `script` from util-linux, on Linux. No container engine is needed. ```bash git clone https://git.unitoo.it/claudiomaradonna/azkaban.git cd azkaban grep -h '^ *run_test ' tests/unit/*.sh tests/integration/test_*.sh | wc -l sed -i '/^ --no-tty)$/,/^ ;;$/d' bin/azkaban TEST_FILTER=no_tty_cli_control bash tests/integration/test_runtime.sh git checkout bin/azkaban ``` The `sed` line breaks the launcher on purpose: it deletes the handling of `--no-tty`. On `master` the count is `352`, and of the two tests of that option one still passes, `ok test_common_no_tty_cli_control_is_always_accepted`, while the other fails: the first guards nothing. After `git switch test/drop-duplicate-tests` the count is `325`, the test that passed is gone, and the one that fails is the only one left, `FAIL: test_common_no_tty_cli_control_wins_under_a_real_terminal`. Then `make test` passes with `bin/azkaban` restored. Checks run on the tip: `make lint && .agents/hooks/shellcheck.sh repo && make test` passes, 325 tests. Since no code under test changes, every surviving test passes on `master`'s code as it does here. Not tried: the audit's other mutation checks were not repeated on the final tip. ## Considerations This branch sits on top of #69, #70, #72, and #71, in that order, so it merges last; until they merge, this page lists their commits too. The audit's proposals that add or reshape shared test helpers are left for a decision of their own. 
No test showed that make lint fails, and names the finding, when
ShellCheck objects to a source that is not the first in the list. The
recipe's ShellCheck pass is about to run in parallel and must keep
that.
bash -n reads its first file and takes the rest as that script's
arguments, so make lint parsed bin/azkaban alone. ShellCheck hid it
where it is installed; where it is not, the recipe skips it and a
syntax error in any other source passed.
One ShellCheck process checked the 53 sources in turn, about 11 s on a
20-CPU host against 6 s for the whole test suite, at every commit.
ShellCheck checks each file on its own, so one process per CPU gives
the same findings in about a third of the time.

The processes share the terminal, where the default multi-line reports
of two files could interleave, so each finding is one line in the gcc
format; ShellCheck run on a file gives its snippet and suggestion.
The refusal of a symlinked ~/.azkaban was tested on a real launch only.
A dry run reaches it through the same checks, which the coming removal
of the launcher's shadowed MCP component check must keep.
Every component of ~/.azkaban/mcp but the last also lies on the
identity folder's path, which is checked first on every launch, so the
MCP component check could not fire and no test reached it. The check
of the directory itself stays, and a real launch still checks every
component after creating the directory.
"${a[@]+"${a[@]}"}" protects an empty array from set -u in Bash older
than 4.4, which the launcher's gate refuses before this code runs.
install.sh already expands empty arrays plainly.
npm install -g leaves the tarballs it downloaded in ~/.npm, so the
Codex and OpenCode images carried them in the client layer for nothing;
the base already removes them after installing Prettier. The Claude
target installs without npm.
The documents drifted from the launcher: the first-argument rule
omitted that a non-folder argument makes the current directory the
workspace, which a typo turns into mounting it whole. Also state the
name rules, the 0 or 1 controls, the dropped container values, the
symlink refusals, the TERM, COLUMNS, and LINES pass-through, and the
approvals containers never see; list AZKABAN_UNMASK_PROC in the env
example; add the two installer failures to troubleshooting.
docs: align contributor documents with the code
All checks were successful
publish-images / publish (push) Successful in 37m16s
Details
2eebef022a
 
Document running the launcher from a checkout, the Makefile's TZ, and
which integration tests fail in Claude Code's Bash sandbox. Keep the
optional API-key field rule and OpenCode's release-gate status in one
home each and link to it.
Fifteen integration tests repeat behavior that a named survivor at the
same level asserts on the same launcher path, so a change to that
behavior meant editing each copy, and three of them (the --no-tty
check without a terminal, the keyless-client explicit key, and the
client-file controls) still passed with the code they name removed.

Removing the last callers of the keyless-client helper in
tests/helpers.sh removes the helper too. The launcher is untouched.
Fourteen tests asserted the same behavior on the same code path as
another test of their level, so a change to that behavior had to be
made twice. Each survivor takes over the assertions it lacked and is
renamed where its behavior grew; the code under test is unchanged.

The OpenCode optional env file test stays: its no-mount check runs
through the OpenCode adapter's mounts, which the Codex survivor never
reaches.
Each case below is asserted again on the same code path elsewhere in
the suite, so keeping it costs an edit per copy and guards nothing
more: the macOS invalid-unmask value, codex's second pass in the
client-spelling and own-file tests, the macOS engine version table,
the recipe-text grep of the Makefile targets, the env.<client>
control, exported-value, workspace-file and double-slash repeats, and
the hand-written --cidfile scans, now assert_argv_sequence.
Account's report test loops over the core clients only.
claudiomaradonna deleted branch test/drop-duplicate-tests 2026-11-11 15:08:25 +01:00
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels
No items
No labels
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference
claudiomaradonna/azkaban!73
No description provided.