Conversation
The default-recipe-dependencies test asserted exactly one dnf5 call for the baseline `install -y tmux gum` line. Forks following the documented packages/COPR customization guidance add extra dnf5 calls (a single copr_install_isolated call alone contributes 3 more), which broke this test even though the resulting build was correct. Keep only the check that the baseline install is first, dropping the exact count assertion. Fixes projectbluefin#354 Assisted-by: Claude Opus 4.5 via pi
hanthor
left a comment
There was a problem hiding this comment.
The remaining assertion still has teeth — but the same pattern survives four lines below
The change is safe
My first concern with "drop an assertion to fix a test" is that the test stops testing anything. It does not here. I removed the code under test and confirmed the test still catches it:
$ git checkout test/355 # this branch merged into origin/main
$ sed -i 's/^dnf5 install -y tmux gum$/: removed/' build/10-build.sh
$ bats tests/unit/10-build_test.bats
not ok 9 10-build: installs the packages the default ujust recipes depend on
With the install line gone, calls[0] is empty and [ "${calls[0]}" = "install -y tmux gum" ] fails. The surviving assertion is load-bearing, and it still pins the stronger property — that the baseline install is first, not merely present.
Gates on this branch merged into origin/main @ 1db684b: bats tests/unit 180/180 ok, just lint exit 0, just check exit 0 — identical to the baseline I measured on unmodified main.
The premise also holds: this is a template repo whose own docs tell forks to add packages and COPRs to build/10-build.sh, and copr_install_isolated emits three dnf5 calls per invocation (verified by copr_install_isolated: enables, disables, then installs from the repo id in copr-helpers_test.bats). An exact count of 1 is genuinely hostile to the documented customization path.
The inconsistency
The very next test in the same file has the identical problem and is untouched:
@test "10-build: enables exactly the podman and brew units" {
...
mapfile -t calls <"${SYSTEMCTL_LOG}"
[ "${#calls[@]}" -eq 4 ] # <-- same brittleness, still here
[ "${calls[0]}" = "enable podman.socket" ]
[ "${calls[1]}" = "enable brew-setup.service" ]
...
}A fork that enables one extra systemd unit — which is at least as common as adding a package — breaks on -eq 4 for exactly the reason #354 describes for -eq 1. If the principle is "exact call counts are wrong in a template repo", it should be applied to both; if the principle is narrower than that, the commit message should say what makes the dnf5 case special. As it stands the next person to hit this files #356 and we do this again.
My suggestion: either extend this PR to the systemctl case, or keep the counts in both and change them to lower bounds plus ordered-prefix assertions, e.g.
[ "${#calls[@]}" -ge 1 ]
[ "${calls[0]}" = "install -y tmux gum" ]which preserves "nothing runs before the baseline install" while tolerating fork additions. That is strictly more information than dropping the count outright.
Blocker
No CI has run on this PR. get_check_runs returns total_count: 0, and the run is parked awaiting fork-workflow approval:
Unit Tests, run_number 57, head_sha a28f676
event: pull_request, status: completed, conclusion: action_required
Needs a maintainer to approve before BATS unit tests can report.
Generated by Claude Code
|
Yeah, I'd agree with that. As written, these particular tests preclude customization rather than planning for it. |
|
Just a heads up, even though I think this PR is on the right track, I realized I don't care about it. Bluefin is transitioning from a Fedora base to GNOME OS, and the stuff in this repo doesn't seem to represent the future state, unless I'm missing something. |
Fixes #354
The
10-build: installs the packages the default ujust recipes depend ontest asserted exactly onednf5call for the baselineinstall -y tmux gumline. Forks following the documented packages/COPR customization guidance add extradnf5calls (a singlecopr_install_isolatedcall alone contributes 3 more:copr enable,copr disable,install --enablerepo=...), which broke this test even though the resulting build was correct.This drops the exact-count assertion and keeps only the check that the baseline install runs (and is first).
Assisted-by: Claude Opus 4.5 via pi