Skip to content

test(apps.just): BATS coverage for install-opentabletdriver and cncf - #1064

Open
kubestellar-hive[bot] wants to merge 2 commits into
mainfrom
quality/test-apps-just-otd-cncf
Open

test(apps.just): BATS coverage for install-opentabletdriver and cncf#1064
kubestellar-hive[bot] wants to merge 2 commits into
mainfrom
quality/test-apps-just-otd-cncf

Conversation

@kubestellar-hive

Copy link
Copy Markdown
Contributor

Claimed ground

File under test: system_files/shared/usr/share/ublue-os/just/apps.just
Recipes claimed: install-opentabletdriver, cncfonly these two.
New file: tests/test_apps_just.bats (21 cases)
Also touched: Justfile (one added line in the test recipe)

Disjointness check against open PRs

apps.just is partially occupied ground, so this PR deliberately claims only
the recipes no open PR touches
:

Open PR Ground it holds in apps.just Overlap here
common#978 [scanner] fix: use valid brew tap trust commands brew tap --trust lines in install-jetbrains-toolbox, install-asus none
common#941 fix(brew): replace invalid brew tap --trust (+ tests/test_brew_tap_trust.bats) same brew tap --trust lines none
common#1048 [quality] test(default.just) default.just / tests/test_default_just.bats none
common#1034 [quality] test(shared.just) shared.just / tests/test_shared_just.bats none
common#1031 [quality] test(system.just) system.just / tests/test_system_just.bats none
common#1063 [architect] setup hook dispatch ublue-*-setup, hookrunner.sh none
common#1045 [architect] image-name → upstream routing bonedigger-report, changelog.just none

install-jetbrains-toolbox and install-asus are 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.

Justfile is also edited by #1048 and #1031, but each adds its own line to the
same test recipe list — a trivial textual conflict at worst, no semantic
overlap.

What this adds

apps.just (76 lines) had zero test coverage: no file under tests/
referenced it, and neither the Justfile test recipe nor
.github/workflows/unit-tests.yml executed anything against it.

tests/test_apps_just.bats extracts each recipe body into a standalone script
and runs it against a sandboxed PATH of mocks (gum, curl, sudo,
flatpak, systemctl, brew, ujust) with /etc/udev/rules.d,
/etc/modprobe.d, /usr/share/ublue-os/homebrew and $HOME redirected into a
per-test temp tree. This follows the existing pattern in
tests/test_update_just.bats. No network, no root, no host mutation.

install-opentabletdriver

  • gum confirm exit 0 → install branch; exit 1 → uninstall branch; exit 130
    (Ctrl-C) → neither branch runs and no flatpak call is made
  • the jq asset filter picks the .tar.gz release asset and not the .deb
    or .rpm decoys
  • the udev rule is copied 70-opentabletdriver.rules71-opentabletdriver.rules
  • blacklist hid_uclogic / blacklist wacom land in
    blacklist-opentabletdriver.conf
  • the mktemp -d extraction directory is removed
  • flatpak --system install / flatpak --system remove are issued on the
    correct branches
  • the user unit is written to $HOME/.config/systemd/user/ and enabled
  • uninstall makes no network calls and does not create the user unit

cncf

  • brew bundle --file=.../cncf.Brewfile is invoked with the curated Brewfile path
  • ujust --choose is skipped when stdin is not a tty, and the recipe still
    exits 0 (the || true tail)
  • a failing brew bundle does not abort the recipe (no set -e in this body)

Bug found while writing these tests

install-opentabletdriver writes the modprobe blacklist to
/etc/modprobe.d/blacklist-opentabletdriver.conf on install, but the uninstall
branch removes /etc/modprobe.d/blacklist-opentabletdriver.rules — a different
filename. Uninstalling leaves hid_uclogic and wacom permanently
blacklisted
, 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 comment
saying to flip the assertion when the recipe is corrected. Filed separately as
an issue.

Not included

The matching step in .github/workflows/unit-tests.yml was prepared but had to
be dropped: the hive GitHub App token has no workflows permission, so pushing
a workflow edit is rejected. The suite is wired into the Justfile test
recipe only. A one-line CI step is needed as a follow-up:

      - name: Run bats (apps.just — install-opentabletdriver, cncf)
        run: bats tests/test_apps_just.bats

Verification

$ bats tests/test_apps_just.bats
1..21
ok 1 .. ok 21   # all pass

Filed by quality agent (hold-gated mode). Human review required — do not merge without review, and do not remove the hold label.

— hive: agent=quality backend=copilot model=claude-opus-5

… 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 hanthor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@castrojo castrojo changed the title [quality] test(apps.just): BATS coverage for install-opentabletdriver and cncf test(apps.just): BATS coverage for install-opentabletdriver and cncf Sep 10, 2026
@castrojo castrojo closed this Sep 10, 2026
@castrojo castrojo reopened this Sep 10, 2026
@castrojo castrojo closed this Sep 10, 2026
@castrojo castrojo reopened this Sep 10, 2026

@hanthor hanthor left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_state is dirty, but only trivially — the Justfile conflict is your added bats tests/test_apps_just.bats line against main's test_system_just / test_brew_tap_trust lines. All three belong; keep all three.
  • The missing .github/workflows/unit-tests.yml step matters more than the PR body implies: without it these 21 tests run only under just test locally 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

hold Work is intentionally paused.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants