From a393239f842c67f68cc4a1c4e2d3bf8f03162746 Mon Sep 17 00:00:00 2001 From: Jeffrey Girard Date: Sat, 31 Oct 2026 14:05:01 +0100 Subject: [PATCH 1/7] M165 T1: mark the three devel-vctrs watched test files --- cairn/ROADMAP.md | 2 +- cairn/milestones/M165-devel-vctrs-drift.md | 8 +++++--- tests/testthat/test-dplyr-compat.R | 2 ++ tests/testthat/test-nested-results-set-compat.R | 2 ++ tests/testthat/test-vctrs-compat.R | 2 ++ 5 files changed, 12 insertions(+), 4 deletions(-) diff --git a/cairn/ROADMAP.md b/cairn/ROADMAP.md index fa3f4525..7536b8fc 100644 --- a/cairn/ROADMAP.md +++ b/cairn/ROADMAP.md @@ -4,7 +4,7 @@ _Last hygiene check: 2026-10-31 M164 review. Graduated Settle oracle types for n ## Milestones | ID | Title | Status | Depends on | Priority | File/Archive | |---|---|---|---|---|---| -| M165 | Keep the devel-vctrs test choice from drifting | planned | — | normal | milestones/M165-devel-vctrs-drift.md | +| M165 | Keep the devel-vctrs test choice from drifting | in-progress | — | normal | milestones/M165-devel-vctrs-drift.md | | M164 | Give six one-type results a second oracle | done | — | normal | milestones/archive/M164-second-oracle-types.md | | M163 | Keep an oracle registry outside tests | done | — | normal | milestones/archive/M163-oracle-registry.md | | M162 | The shared test pool keeps both daemons | dropped | — | normal | milestones/archive/M162-shared-pool-daemon-loss.md | diff --git a/cairn/milestones/M165-devel-vctrs-drift.md b/cairn/milestones/M165-devel-vctrs-drift.md index b8869ec9..09075684 100644 --- a/cairn/milestones/M165-devel-vctrs-drift.md +++ b/cairn/milestones/M165-devel-vctrs-drift.md @@ -1,13 +1,13 @@ # M165: Keep the devel-vctrs test choice from drifting -- **Status:** planned +- **Status:** in-progress - **Priority:** normal - **Depends on:** — - **Driving RR:** — - **Principles touched:** — - **Resolves:** — - **Surface tier:** internal — a CI workflow and a marker comment in three test files; no package user relies on either -- **Branch/PR:** — +- **Branch/PR:** m165-devel-vctrs-drift ## Goal @@ -37,7 +37,7 @@ Remove the two places where `devel-vctrs.yaml` can drift without a signal, while ## Tasks -- [ ] T1: Add the line `# devel-vctrs.yaml runs this file on every push and pull request.` to `tests/testthat/test-vctrs-compat.R`, `test-dplyr-compat.R` and `test-nested-results-set-compat.R`, as a whole line after each file's header comment. +- [x] T1: Add the line `# devel-vctrs.yaml runs this file on every push and pull request.` to `tests/testthat/test-vctrs-compat.R`, `test-dplyr-compat.R` and `test-nested-results-set-compat.R`, as a whole line after each file's header comment. - [ ] T2: Rewrite the "Run the tests against it" step of `.github/workflows/devel-vctrs.yaml`. Its `env:` maps `inputs.full` alone; the R code sets the switch to true on `schedule`, and on `workflow_dispatch` when the full value is "true". When the switch is false, it reads `tests/testthat/test-*.R`, keeps the files holding the marker as a whole line, stops with an error naming the marker when none does, prints the selected files, and passes `testthat::test_local()` a filter built from their names with regex characters escaped. It prints `Whole suite:` as now. - [ ] T3: Key the concurrency group on `github.workflow`, `github.ref`, `github.event_name` and `inputs.full`. Rewrite the header paragraph that names the three files, and the concurrency comment, from a same-session read of the new step, to point at the marker line and the raw key. Edit the `devel-vctrs.yaml` concurrency sentence on `cairn/PROFILE.md` line 44 in place, adding no line (the file is at 119 of its <120 cap). - [ ] T4: Copy the step's R code to a scratch script and run it locally: the five cases of AC5, then the rename plant and the no-marker plant of AC2 on a copy of `tests/testthat/`, each plant seen to give its stated result. Record the outcomes in one work-log line. @@ -56,6 +56,8 @@ Remove the two places where `devel-vctrs.yaml` can drift without a signal, while - 2026-10-31: lessons used: 2026-09-02 (M50, extended M53/M58/M67/M79/M88/M153) on CI pushes and dispatch concurrency; 2026-09-11 (M86, extended M89) on a checker whose domain can empty. - 2026-10-31: criteria audit (reduced mode, fresh Opus reader): AC1 claimed every push and pull request and a log listing no task printed; narrowed to this PR's run against a grep of marker files, the print moved to T2, contexts named instead of files. AC2 narrowed to two named plants, search limited to `test-*.R`. AC3 none. AC4 narrowed to four bare properties. AC5 narrowed to a one-entry `env:` and five named cases with empty full values. Side effect of AC4 moved to T5. - 2026-10-31: no changelog entry: CI-only, as at M153. +- 2026-10-31: status in-progress, branch m165-devel-vctrs-drift cut from main at f9160fdc. +- 2026-10-31: T1 done: marker line added as its own paragraph at the end of each file's header comment; `grep -lxF` lists the three files; `air format --check .` clean. Comment-only change, so verify's `devtools::test()` waits for T4, which runs the three files through the new step. ## Decisions diff --git a/tests/testthat/test-dplyr-compat.R b/tests/testthat/test-dplyr-compat.R index 610677c6..5d95e3e8 100644 --- a/tests/testthat/test-dplyr-compat.R +++ b/tests/testthat/test-dplyr-compat.R @@ -10,6 +10,8 @@ # branch per entry rather than accepting either. Each verb appears by its own # literal name, and the verbs with something to say in both # directions get an entry each way. +# +# devel-vctrs.yaml runs this file on every push and pull request. # The completed fixture: the suite's most-requested three-fold run, written the # way test-nested-results-print.R and test-nested-results-plot.R write it, so diff --git a/tests/testthat/test-nested-results-set-compat.R b/tests/testthat/test-nested-results-set-compat.R index d3795925..74aaf8b4 100644 --- a/tests/testthat/test-nested-results-set-compat.R +++ b/tests/testthat/test-nested-results-set-compat.R @@ -10,6 +10,8 @@ # # As in test-dplyr-compat.R, every entry names the branch it must take, and # every door the check must cover appears by its own literal name. +# +# devel-vctrs.yaml runs this file on every push and pull request. # The suite's two-workflow grid run, served from the cache: `tuned` then # `fixed`, two outer folds each. diff --git a/tests/testthat/test-vctrs-compat.R b/tests/testthat/test-vctrs-compat.R index 86e1610c..9fc8dbb7 100644 --- a/tests/testthat/test-vctrs-compat.R +++ b/tests/testthat/test-vctrs-compat.R @@ -12,6 +12,8 @@ # A caller cannot see which door a verb uses, so the answer has to be the same # through all of them, and the assertions below are written per form rather # than in one loop so a failure says which form regressed. +# +# devel-vctrs.yaml runs this file on every push and pull request. # The completed fixture, written exactly as test-dplyr-compat.R writes it: same # function, same arguments, same RNG state, so `memoised()` serves this file From 6c3ded1f404b58146d5a8cb864de12d8dde663d3 Mon Sep 17 00:00:00 2001 From: Jeffrey Girard Date: Sat, 31 Oct 2026 14:07:28 +0100 Subject: [PATCH 2/7] M165 T2-T5: select watched files by marker, key concurrency on raw values --- .github/workflows/devel-vctrs.yaml | 53 +++++++++++++++------- cairn/DECISIONS.md | 6 +++ cairn/PROFILE.md | 2 +- cairn/ROADMAP.md | 2 +- cairn/milestones/M165-devel-vctrs-drift.md | 16 +++++-- 5 files changed, 55 insertions(+), 24 deletions(-) diff --git a/.github/workflows/devel-vctrs.yaml b/.github/workflows/devel-vctrs.yaml index 15cba9bb..0f3c5dce 100644 --- a/.github/workflows/devel-vctrs.yaml +++ b/.github/workflows/devel-vctrs.yaml @@ -17,10 +17,10 @@ # still needs the dispatch. # # What a run tests depends on its trigger. A push or pull request runs only -# the three vctrs compatibility test files (`test-vctrs-compat.R`, -# `test-dplyr-compat.R`, `test-nested-results-set-compat.R`; the `filter` in -# the last step). The weekly schedule runs the whole suite, because a -# development-vctrs break can reach files outside the three: other test files +# the vctrs compatibility test files that carry the marker line the last step +# names; a file joins or leaves the watch by that line. The weekly schedule +# runs the whole suite, because a development-vctrs break can reach files +# outside the watched ones: other test files # call vctrs functions too (`test-nested-tune-bayes-oracles.R` calls # `vec_cbind_frame_ptype()`), and dplyr and tibble build on vctrs. A dispatch # runs the whole suite unless its `full` input is set to false. @@ -63,7 +63,7 @@ on: workflow_dispatch: inputs: full: - description: 'Run the whole suite (false runs the three vctrs test files)' + description: 'Run the whole suite (false runs the watched vctrs test files)' type: boolean default: true @@ -81,11 +81,14 @@ permissions: # Extends the block in R-CMD-check.yaml. Keyed on the workflow as well as # the ref, so cancelling a superseded run of this leg never touches a check run. -# Also keyed on the test step's `FULL_SUITE` switch (the same expression), so a -# three-file run never cancels or replaces a pending or running whole-suite -# run: GitHub keeps one pending run per group and cancels the older one. +# Also keyed on the event name and the `full` input, the two values the test +# step decides its whole-suite switch from, so two runs in one group always run +# the same tests, and a subset run never cancels or replaces a pending or +# running whole-suite run: GitHub keeps one pending run per group and cancels +# the older one. A scheduled run and a full dispatch therefore sit in +# different groups and can both run. concurrency: - group: ${{ github.workflow }}-${{ github.ref }}-${{ github.event_name == 'schedule' || (github.event_name == 'workflow_dispatch' && inputs.full) }} + group: ${{ github.workflow }}-${{ github.ref }}-${{ github.event_name }}-${{ inputs.full }} cancel-in-progress: ${{ github.ref_name != github.event.repository.default_branch }} jobs: @@ -147,18 +150,34 @@ jobs: } shell: Rscript {0} - # testthat matches `filter` against each file name with `test-` and `.R` - # removed. A NULL filter runs every file. + # The whole-suite switch is decided here and nowhere else, from the two + # values the concurrency group above is keyed on. `inputs.full` renders + # as "" outside a dispatch. A subset run takes the test files that carry + # the marker line below as a whole line, so a renamed file stays in and + # a run that finds none stops red. testthat matches `filter` against + # each file name with `test-` and `.R` removed. A NULL filter runs every + # file. - name: Run the tests against it env: - FULL_SUITE: ${{ github.event_name == 'schedule' || (github.event_name == 'workflow_dispatch' && inputs.full) }} + FULL_INPUT: ${{ inputs.full }} run: | - full <- identical(Sys.getenv("FULL_SUITE"), "true") + event <- Sys.getenv("GITHUB_EVENT_NAME") + full <- event == "schedule" || + (event == "workflow_dispatch" && Sys.getenv("FULL_INPUT") == "true") cat("Whole suite: ", full, "\n", sep = "") - filter <- if (full) { - NULL - } else { - "^(vctrs-compat|dplyr-compat|nested-results-set-compat)$" + filter <- NULL + if (!full) { + marker <- "# devel-vctrs.yaml runs this file on every push and pull request." + files <- list.files("tests/testthat", "^test-.*\\.R$", full.names = TRUE) + watched <- files[vapply(files, function(f) marker %in% readLines(f, warn = FALSE), logical(1))] + if (length(watched) == 0) { + stop("No test file in tests/testthat carries the line '", marker, "'.") + } + cat("Watched files:", basename(watched), sep = "\n ") + cat("\n") + stems <- sub("^test-(.*)\\.R$", "\\1", basename(watched)) + stems <- gsub("([][{}()+*^$|\\\\?.])", "\\\\\\1", stems) + filter <- paste0("^(", paste(stems, collapse = "|"), ")$") } pkgload::load_all(".", quiet = TRUE) testthat::test_local(filter = filter, stop_on_failure = TRUE) diff --git a/cairn/DECISIONS.md b/cairn/DECISIONS.md index 356258d4..473da31e 100644 --- a/cairn/DECISIONS.md +++ b/cairn/DECISIONS.md @@ -2135,6 +2135,12 @@ upstream, either of which would let the file return to a shared blob. **Decision:** they do not. A table that only reshapes, labels or joins numbers that another group's oracles check is not a numeric result of its own, and a table that holds no number is not one either. Each such group in `cairn/ORACLES.md` carries one line naming the group it rides on, or saying it holds no number. The user chose this at M164's plan gate over adding a second type to all nine groups. **Consequences:** a table that computes a new number, such as an average or a rescoring, is a result of its own and owes two types. The three groups keep their one oracle, which checks the reshaping. Falsified by a reshaping table that changes a number (drops, duplicates or misplaces a value) with no test failing. +### D-122 (2026-10-31): `devel-vctrs.yaml` picks its push-time test files by a marker line and keys its concurrency group on the event name and the `full` input. Narrows D-119's filter and concurrency clauses + +**Context:** D-119's filter named three test files by hand, so a renamed file dropped out with no signal, and its concurrency group repeated the test step's switch expression with nothing keeping the two copies equal (M153 review). M165's plan gate chose to simplify the workflow over adding a drift test, and kept push and pull-request runs off the whole suite. +**Decision:** on a push or pull request the test step runs the `tests/testthat/test-*.R` files that carry the line `# devel-vctrs.yaml runs this file on every push and pull request.` as a whole line, and stops with an error when none does. The step decides the whole-suite switch from `GITHUB_EVENT_NAME` and `inputs.full` alone, and the concurrency group is keyed on those two raw values with the workflow and ref, so two runs in one group run the same tests. Rejected at the plan gate: a `-compat` file-name convention, which still drops a file renamed out of it. +**Consequences:** a test file joins the push-time watch by taking the marker line, replacing D-119's "adds itself to the filter". A scheduled run and a full dispatch no longer share a group, nor do a `full` false dispatch on the default branch and a push to it, so each pair can run side by side. Falsified by a watched file losing its marker in an edit that no run flags, or by a subset run cancelling or replacing a whole-suite run. (Narrows D-119.) +