mirror of
https://github.com/cozystack/cozystack.git
synced 2026-11-03 02:27:01 +09:00
Aleksei Sviridkin
b7e60855e9
<!-- Thank you for making a contribution! Here are some tips for you:
- Use Conventional Commits for the PR title: `type(scope): description`
- Types: feat, fix, docs, style, refactor, perf, test, build, ci, chore
- Scopes are not an exhaustive list — pick the most specific scope for
the change and extend the list when a genuinely new area appears.
Examples:
- System components: dashboard, platform, operator, cilium, kube-ovn,
linstor, fluxcd, cluster-api
- Managed apps: postgres, mariadb, redis, kafka, clickhouse,
virtual-machine, kubernetes
- Development and maintenance: api, hack, tests, ci, docs, maintenance
- Breaking changes: append `!` after type/scope (`feat(api)!: ...`) or
add a `BREAKING CHANGE:` footer
- If it's a work in progress, consider creating this PR as a draft.
- Don't hesistate to ask for opinion and review in the community chats,
even if it's still a draft.
- Add the label `kind/backport` if it's a bugfix that needs to be
backported to a previous version.
-->
## What this PR does
`hack/helm-unit-tests.sh` decides whether to enter a package by asking
`make -C <pkg> -n test`. `packages/system/gpu-operator` declared only
`update:`, so the sweep skipped it silently and the three test cases in
`tests/kernel_module_config_test.yaml` had never run. They pass, so
nothing was hiding behind them. Still worth fixing: the directory reads
as coverage to whoever opens it, and a suite nothing runs is where a
stale assertion sits.
The target is phony. `hack/package.mk`'s `.PHONY` line lists only the
targets that file defines, so a path named `test` next to the Makefile
would make the target up to date and skip the recipe.
Rather than fix the one package I enumerated the rest of the tree. 84
packages hold a `tests/` directory, 83 ship at least one
`tests/*_test.yaml`, and gpu-operator was the only one of those without
the target. The 84th is `packages/system/nfs-driver`, whose `tests/`
holds a PersistentVolumeClaim and a StorageClass to apply by hand. No
suite there, so a `test:` target would only make the sweep fail it for
reporting zero suites. Left alone.
`hack/package-test-target-coverage.bats` pins the class. Every other
guard in the sweep runs inside a directory it already reached, so a
package with no target escapes all of them at once and the run still
reports success. The new case probes with `make -n` instead of grepping
for `test:`, which is the question the sweep asks, and it mirrors the
sweep's other precondition too: the sweep requires `$dir/Makefile` and
returns before probing, where make would also accept `GNUmakefile` or no
makefile at all. The verdict comes from the sweep's command unchanged,
and a failing package is probed a second time with `LC_ALL=C
--no-print-directory` purely to read the diagnostic, since both of those
are visible to a Makefile through `LC_ALL` and `MAKEFLAGS`.
Mutations, because a guard that has never been red proves nothing:
```text
target removed from gpu-operator -> red, names packages/system/gpu-operator, quotes make's error
Makefile renamed to GNUmakefile -> red; make still answers 0 here, so this leg is load-bearing
glob pointed at a missing dir -> red via the blindness leg, instead of passing on an empty set
unmodified tree -> green, 83 packages scanned
```
The full `hack/helm-unit-tests.sh` sweep now enters gpu-operator and
finishes green.
One detail that is invisible on a Mac: the probe runs under
`--no-print-directory`. On modern GNU make `-C` turns on `-w`, and the
fatal path prints `Leaving directory` after the error, so `tail -n 1`
picks up the directory trace and drops the diagnostic. Measured on 4.4.1
where that happens, and on 3.81 (still `/usr/bin/make` on macOS) where
`-C` prints no trace at all.
### Screenshots
No UI changes.
### Downstream repositories
<!--
Cozystack is upstream for repositories that are NOT kept in sync with it
automatically, and nothing in CI compares the two sides, so a change
here can break them silently.
Which change forces what, and which file to touch, is the trigger map in
docs/agents/contributing.md under "Downstream Repositories". Walk it
before you tick anything.
IF YOU ARE AN AI AGENT OR AN LLM, THIS IS PART OF THE TASK, NOT A FORM
TO FILL IN:
1. Walk the trigger map against the actual diff of this PR, file by
file. Do not decide from the PR title or from your memory of what you
changed.
2. For every repository you tick, open the follow-up PR there and link
it on the same line. A ticked box with no link claims work that does not
exist, which is worse than an empty box. Search that repository for an
open PR or issue covering it first, and link that instead of filing a
duplicate.
3. When the follow-up is out of scope, or needs a decision that is not
yours to make, open an issue in that repository instead, link that, and
say so in the PR body.
4. Do not tick "No downstream repository is affected" to make the
checklist go away. If you are unsure, leave every box empty and say so
in the PR body, so a human decides.
-->
Walked the trigger map against both changed files. Two entries come
close and both miss. `ccp` gates on `hack/package.mk`,
`hack/common-envs.mk` and the `packages/{apps,system,extra,core}`
layout; nothing here is moved or renamed and no existing target changes
what it does, so a new `test:` target sitting beside 80 identical ones
is not something its skills read. The `website` entry for developer
tooling points at `content/en/docs/next/development.md`, whose Testing
section covers the e2e sandbox under `packages/core/testing` and says
nothing about the per-package helm-unittest sweep.
- [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
test(tests): run the gpu-operator helm-unittest suite, which the sweep had been skipping because the package declared no `test:` target
```
<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->
## Summary by CodeRabbit
- **Tests**
- Added coverage checks to verify that all Helm test suites are included
in the package test sweep.
- Added diagnostics for packages missing a runnable test target.
- **Bug Fixes**
- Added a test target to the GPU operator package, enabling its Helm
tests to run through the standard package test workflow.
<!-- end of auto-generated comment: release notes by coderabbit.ai -->