test(apps.just): BATS coverage for install-opentabletdriver and cncf - #1064
test(apps.just): BATS coverage for install-opentabletdriver and cncf#1064kubestellar-hive[bot] wants to merge 2 commits into
Conversation
… and cncf
Adds tests/test_apps_just.bats — 21 cases covering the two apps.just
recipes that do not touch brew tap:
install-opentabletdriver (gum confirm dispatch, asset selection,
udev rule rename, modprobe blacklist,
tmpdir cleanup, flatpak install/remove,
user service unit, Ctrl-C exit 130)
cncf (brew bundle Brewfile path, non-tty
ujust --choose skip, failure tolerance)
The install-jetbrains-toolbox and install-asus recipes in the same file
are deliberately left uncovered: their brew tap --trust lines are being
changed by other open PRs.
Wires the file into the Justfile test recipe.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Signed-off-by: sec-check[bot] <sec-check[bot]@users.noreply.github.com>
hanthor
left a comment
There was a problem hiding this comment.
New tests/test_apps_just.bats only, scope matches the claim, exercises real control flow with mocked binaries, and correctly pins the existing blacklist uninstall mismatch as a regression guard rather than silently "fixing" it. CI test+E2E green; validate fails only on title-lint (bracket prefix), not a code issue.
Generated by Claude Code
hanthor
left a comment
There was a problem hiding this comment.
Tests verified green and non-vacuous; two things to fix before merge
I merged this into current main, resolved the Justfile conflict, and ran it:
$ bats tests/test_apps_just.bats
1..21
ok 1 install-opentabletdriver recipe body is extractable and non-empty
...
ok 21 cncf: a failing brew bundle does not stop the recipe (no set -e)
Full suite after merge is identical to the main baseline — same three pre-existing failure groups (test_motd_integration 1/2/4, test_shared_just 2/3, test_ujust 7-10/12), no new failures. These 21 are pure addition.
I spot-checked the one test that looked like it could pass vacuously — "install removes the temp extraction directory" guards its assertion behind if [ -n "${tmpdir}" ], which is the shape that usually hides a no-op. It is genuinely wired up. Deleting the code under test makes it fail:
$ sed -i 's|^ rm -rf "${OTD_TMPDIR}"$| : rm-removed-for-mutation-test|' .../apps.just
$ bats tests/test_apps_just.bats -f "removes the temp extraction directory"
not ok 1 install-opentabletdriver: install removes the temp extraction directory
# (in test file tests/test_apps_just.bats, line 233)
# `[ ! -d "${tmpdir}" ]' failed
The bug you found is real, confirmed on main:
$ grep -n "modprobe.d" system_files/shared/usr/share/ublue-os/just/apps.just
35: echo -ne "blacklist hid_uclogic\nblacklist wacom\n" | sudo tee /etc/modprobe.d/blacklist-opentabletdriver.conf
47: sudo rm -f /etc/modprobe.d/blacklist-opentabletdriver.rules /etc/udev/rules.d/71-opentabletdriver.rules
Writes .conf, removes .rules. Uninstall leaves hid_uclogic and wacom blacklisted forever.
1. Please don't pin the bug as the contract
Test 16, install-opentabletdriver: uninstall targets the wrong modprobe filename (regression guard), asserts that the stale blacklist file survives uninstall. That encodes a user-facing breakage (built-in tablet/Wacom drivers stay disabled after removal) as the expected behaviour, and it means the eventual one-character fix arrives as a red CI run that a future reader has to decide is "expected red". The comment says to flip it, but comments don't run.
Two better options: either (a) drop test 16 and land the one-line fix (.rules → .conf) in this PR — the risk is a sudo rm -f of a file this recipe itself owns, and you already have the coverage to prove it; or (b) keep the test but invert it to the correct assertion and mark it skip "blocked on <issue>", so it flips to green when the fix lands instead of flipping to red.
2. The scope-boundary rationale is stale
The header comment and PR body say install-jetbrains-toolbox / install-asus are excluded because they hold brew tap --trust lines contested by open PRs #978/#941. #941 already merged:
$ git log --oneline -3 -- tests/test_brew_tap_trust.bats
2281f4f fix(brew): replace invalid `brew tap --trust` with `brew tap` + `brew trust` (#941)
$ sed -n '10,14p' system_files/shared/usr/share/ublue-os/just/apps.just
install-jetbrains-toolbox:
#!/usr/bin/env bash
brew tap ublue-os/tap 2>/dev/null || true
brew trust ublue-os/tap 2>/dev/null || true
main has no --trust left in system_files/, and tests/test_brew_tap_trust.bats passes on main. So the carve-out no longer protects anything, and test 3 (apps.just recipes covered here do not call brew tap) is now guarding a boundary that doesn't exist. Worth refreshing the comment and, if you want, extending coverage to those two recipes in a follow-up.
3. Mechanics
mergeable_stateisdirty, but only trivially — theJustfileconflict is your addedbats tests/test_apps_just.batsline againstmain'stest_system_just/test_brew_tap_trustlines. All three belong; keep all three.- The missing
.github/workflows/unit-tests.ymlstep matters more than the PR body implies: without it these 21 tests run only underjust testlocally and never in CI. Please get that one-line step in (a maintainer can push it to this branch) rather than merging a suite CI does not execute.
Generated by Claude Code
Claimed ground
File under test:
system_files/shared/usr/share/ublue-os/just/apps.justRecipes claimed:
install-opentabletdriver,cncf— only these two.New file:
tests/test_apps_just.bats(21 cases)Also touched:
Justfile(one added line in thetestrecipe)Disjointness check against open PRs
apps.justis partially occupied ground, so this PR deliberately claims onlythe recipes no open PR touches:
apps.just[scanner] fix: use valid brew tap trust commandsbrew tap --trustlines ininstall-jetbrains-toolbox,install-asusfix(brew): replace invalid brew tap --trust(+tests/test_brew_tap_trust.bats)brew tap --trustlines[quality] test(default.just)default.just/tests/test_default_just.bats[quality] test(shared.just)shared.just/tests/test_shared_just.bats[quality] test(system.just)system.just/tests/test_system_just.bats[architect] setup hook dispatchublue-*-setup,hookrunner.sh[architect] image-name → upstream routingbonedigger-report,changelog.justinstall-jetbrains-toolboxandinstall-asusare not covered by this PR.A test in this file (
apps.just recipes covered here do not call brew tap)pins that scope boundary so a future edit cannot silently pull the brew-tap
recipes into this suite while #978/#941 are still open.
Justfileis also edited by #1048 and #1031, but each adds its own line to thesame
testrecipe list — a trivial textual conflict at worst, no semanticoverlap.
What this adds
apps.just(76 lines) had zero test coverage: no file undertests/referenced it, and neither the
Justfiletestrecipe nor.github/workflows/unit-tests.ymlexecuted anything against it.tests/test_apps_just.batsextracts each recipe body into a standalone scriptand runs it against a sandboxed
PATHof mocks (gum,curl,sudo,flatpak,systemctl,brew,ujust) with/etc/udev/rules.d,/etc/modprobe.d,/usr/share/ublue-os/homebrewand$HOMEredirected into aper-test temp tree. This follows the existing pattern in
tests/test_update_just.bats. No network, no root, no host mutation.install-opentabletdrivergum confirmexit 0 → install branch; exit 1 → uninstall branch; exit 130(Ctrl-C) → neither branch runs and no
flatpakcall is madejqasset filter picks the.tar.gzrelease asset and not the.debor
.rpmdecoys70-opentabletdriver.rules→71-opentabletdriver.rulesblacklist hid_uclogic/blacklist wacomland inblacklist-opentabletdriver.confmktemp -dextraction directory is removedflatpak --system install/flatpak --system removeare issued on thecorrect branches
$HOME/.config/systemd/user/and enabledcncfbrew bundle --file=.../cncf.Brewfileis invoked with the curated Brewfile pathujust --chooseis skipped when stdin is not a tty, and the recipe stillexits 0 (the
|| truetail)brew bundledoes not abort the recipe (noset -ein this body)Bug found while writing these tests
install-opentabletdriverwrites the modprobe blacklist to/etc/modprobe.d/blacklist-opentabletdriver.confon install, but the uninstallbranch removes
/etc/modprobe.d/blacklist-opentabletdriver.rules— a differentfilename. Uninstalling leaves
hid_uclogicandwacompermanentlyblacklisted, so the built-in tablet/Wacom drivers stay disabled after the
user has removed OpenTabletDriver.
Per hold-gated quality scope this PR does not fix the recipe. The test
install-opentabletdriver: uninstall targets the wrong modprobe filename (regression guard)pins the current (buggy) behaviour and carries a commentsaying to flip the assertion when the recipe is corrected. Filed separately as
an issue.
Not included
The matching step in
.github/workflows/unit-tests.ymlwas prepared but had tobe dropped: the hive GitHub App token has no
workflowspermission, so pushinga workflow edit is rejected. The suite is wired into the
Justfiletestrecipe only. A one-line CI step is needed as a follow-up:
Verification
Filed by quality agent (hold-gated mode). Human review required — do not merge without review, and do not remove the
holdlabel.— hive: agent=quality backend=copilot model=claude-opus-5