lacs-project / lacs-project/sysknife

Nothing requires that a tests/release/*.test.sh is actually invoked by a gate

Open
#331 4 comments 0 reactions 1 assignee Claimed by @be-student View on GitHub
claimed easy enhancement good first issue
Dominant language
Rust
Stars
12
Forks
19
Avg merge
18h 57m
Merged PRs (30d)
116

Description

`tests/release/database-path-agreement.test.sh` was in the tree, correct, and wired into nothing. Zero references in `ci.yml`, `e2e.yml`, `release.yml` or `scripts/ci-local.sh`, and it never had any since the day it was added.

It guards a real past defect: the installer and the daemon resolving different SQLite paths, which splits the audit chain in two and leaves `audit verify` reading a store nothing writes to. Reverting the installer's path resolution to the old value left every required check green.

#330 wires that one file in. The gap it exposes is not that one file.

## The gap

`scripts/check_repo_completeness.sh` checks that required files exist. Nothing checks that a release test is reachable from a gate, so adding `tests/release/foo.test.sh` and forgetting the two lines in `ci.yml` and `ci-local.sh` produces a test that runs only when someone types its name. There is no error, no warning, and the file looks maintained.

Same shape as #278: an artifact that is correct and unread.

## Scope

A check that, for every `tests/release/*.test.sh`, the filename appears in at least one of `.github/workflows/ci.yml`, `.github/workflows/e2e.yml`, `.github/workflows/release.yml` or `scripts/ci-local.sh`.

Two details that decide whether it is worth having:

- It must fail when the glob comes back empty. A broken glob passes over zero files and reports success, which is the same defect the check exists to catch, one level up. That shape has shipped in this repository before: see #347 and #355, where nine guards exited 1 with no output because a `grep` that matched nothing killed the script before its own diagnostic could run.
- Path-filtered jobs count as wired. `e2e.yml` only runs on some paths, and a test reachable only from there is still reachable.

Worth extending the same idea to `tests/e2e/*.test.sh`, which has the same exposure.

## Tests first

Add a `tests/release/foo.test.sh` that nothing invokes and watch the new check name it. Then delete the `ci.yml` line for an existing one and watch it name that. Both directions, because a check that only fires on a new file misses the deletion case, which is how this one got lost.

## Difficulty

`easy`. One shell script and two lines of wiring, and the derivation is a glob plus a grep.

## Getting started

[CONTRIBUTING.md](https://github.com/lacs-project/sysknife/blob/main/CONTRIBUTING.md) has the build and test commands. `bash scripts/ci-local.sh` runs the whole board locally. No CLA and no copyright waiver. The project is MIT.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.