Skip KUnit tests in the configuration, not on the command line

configs/fragment.M14

@@ -31,5 +31,5 @@ CONFIG_KUNIT_ALL_TESTS=y
 # The kunit_fault suite oopses on purpose to check that KUnit catches it, which PANIC_ON_OOPS turns into a dead machine. 7.2.8 skips it on its own, but 6.18 does not.
 CONFIG_KUNIT_FAULT_TEST=n
 
-# Suites that fail under TCG on a loaded host whatever the compiler: the DRM scheduler tests race their own hrtimers, and the ratelimit stress test counts on timing. KUNIT_ALL_TESTS hides their options, so the KUnit boot skips them by module instead.
-# gk:kunit-skip drm_sched_tests test_ratelimit
+# Tests that fail under TCG on a loaded host whatever the compiler: the DRM scheduler tests race their own hrtimers, and the ratelimit stress test counts on timing. KUNIT_ALL_TESTS hides their options, so gk turns them off in a second configuration pass with KUNIT_ALL_TESTS off. Every other test keeps the value KUNIT_ALL_TESTS gave it.
+# gk:kunit-skip DRM_SCHED_KUNIT_TEST RATELIMIT_KUNIT_TEST

crates/gk/src/boot.rs

@@ -58,8 +58,6 @@ pub struct Boot<'a> {
     pub dir: &'a Path,
     /// Their file name without the extension, as `boot` for `boot.log` and `boot.json`.
     pub stem: &'a str,
-    /// KUnit modules whose suites are skipped, from the fragment.
-    pub skip: &'a [String],
 }
 
 /// One `GK-CHECK` line.
@@ -233,33 +231,20 @@ pub fn strip_timestamp(line: &str) -> &str {
 /// The kernel command line for a boot.
 ///
 /// The fragments build every KUnit suite in, and they run before init, which under TCG takes longer than the whole boot budget. So every suite but `kunit` turns them off: `kunit.enable=0` from 6.2, and a filter that matches no suite from 5.10 to 6.1. A kernel ignores the one it does not know.
-///
-/// On the `kunit` boot, the suites of the modules in `skip` are reported as skipped, through the attribute filter KUnit has from 6.6.
 #[must_use]
-pub fn append(console: &str, suite: &str, version: &Version, skip: &[String]) -> String {
-    let quiet = if suite != "kunit" {
-        " kunit.enable=0 kunit.filter_glob=gk-none".to_owned()
-    } else if skip.is_empty() {
-        String::new()
+pub fn append(console: &str, suite: &str, version: &Version) -> String {
+    let quiet = if suite == "kunit" {
+        ""
     } else {
-        let filter: Vec<String> = skip.iter().map(|m| format!("module!={m}")).collect();
-        format!(
-            " kunit.filter={} kunit.filter_action=skip",
-            filter.join(",")
-        )
+        " kunit.enable=0 kunit.filter_glob=gk-none"
     };
     format!(
         "console={console} panic=-1 oops=panic gk.suite={suite} gk.kernel={version} gk.cpus={CPUS}{quiet}"
     )
 }
 
 /// The QEMU command, with the kernel and the initramfs at the paths the container mounts them on.
-pub fn qemu_command(
-    p: &Platform,
-    version: &Version,
-    suite: &str,
-    skip: &[String],
-) -> Result<Vec<String>, String> {
+pub fn qemu_command(p: &Platform, version: &Version, suite: &str) -> Result<Vec<String>, String> {
     let missing = |what: &str| format!("platforms.toml has no {what} for {version} on {}", p.name);
     let machine = p.machine_for(version).ok_or_else(|| missing("machine"))?;
     let cpu = p.cpu_for(version).ok_or_else(|| missing("cpu"))?;
@@ -281,7 +266,7 @@ pub fn qemu_command(
         "-initrd",
         "/boot/initramfs.cpio",
         "-append",
-        &append(console, suite, version, skip),
+        &append(console, suite, version),
         "-nographic",
         "-monitor",
         "none",
@@ -305,7 +290,7 @@ pub fn run(repo: &Repo, b: &Boot<'_>) -> Result<Outcome, String> {
     let (init, archive) = initramfs::for_platform(repo, b.platform)?;
     let image = forge::image_for(repo, "gk-boot")?;
     let qemu = crate::cell::image_digest(&image)?;
-    let command = qemu_command(b.platform, b.version, b.suite, b.skip)?;
+    let command = qemu_command(b.platform, b.version, b.suite)?;
     let kernel = std::fs::canonicalize(b.image)
         .map_err(|e| format!("reading {}: {e}", b.image.display()))?;
     std::fs::create_dir_all(b.dir).map_err(|e| format!("creating {}: {e}", b.dir.display()))?;
@@ -472,7 +457,6 @@ pub fn command(repo: &Repo, args: &[String]) -> Result<bool, String> {
             suite: &suite,
             dir: &dir,
             stem: "boot",
-            skip: &[],
         },
     )?;
     let passed = o.checks.iter().filter(|c| c.pass).count();
@@ -577,23 +561,11 @@ mod tests {
         assert!(o.panic.is_none());
     }
 
-    #[test]
-    fn the_kunit_boot_skips_the_fragment_modules() {
-        let v: Version = "7.2.8".parse().unwrap();
-        let skip = ["drm_sched_tests".to_owned(), "test_ratelimit".to_owned()];
-        let line = append("ttyS0", "kunit", &v, &skip);
-        assert!(line.ends_with(
-            " kunit.filter=module!=drm_sched_tests,module!=test_ratelimit kunit.filter_action=skip"
-        ));
-        assert!(!append("ttyS0", "kunit", &v, &[]).contains("kunit."));
-        assert!(!append("ttyS0", "smoke", &v, &skip).contains("filter="));
-    }
-
     #[test]
     fn the_command_line_names_the_suite_and_the_kernel() {
         let v: Version = "7.2.8".parse().unwrap();
         assert_eq!(
-            append("ttyS0", "smoke", &v, &[]),
+            append("ttyS0", "smoke", &v),
             "console=ttyS0 panic=-1 oops=panic gk.suite=smoke gk.kernel=7.2.8 gk.cpus=2 kunit.enable=0 kunit.filter_glob=gk-none"
         );
     }

crates/gk/src/cell.rs

@@ -106,7 +106,7 @@ pub struct Setup {
     pub coordinates: Coordinates,
     /// Whether the cell boots after L4.
     pub boots: bool,
-    /// The KUnit modules the fragment skips, from its `# gk:kunit-skip` lines.
+    /// The KUnit test options the fragment turns off, from its `# gk:kunit-skip` lines.
     pub kunit_skip: Vec<String>,
 }
 
@@ -564,7 +564,7 @@ struct Booted {
     steps: Vec<Step>,
 }
 
-/// One boot of a cell's image. Only the KUnit boot skips the modules the fragment names.
+/// One boot of a cell's image.
 fn boot_once(
     repo: &Repo,
     s: &Setup,
@@ -573,7 +573,6 @@ fn boot_once(
     suite: &str,
     stem: &str,
 ) -> Result<boot::Outcome, String> {
-    let skip: &[String] = if suite == "kunit" { &s.kunit_skip } else { &[] };
     boot::run(
         repo,
         &boot::Boot {
@@ -583,7 +582,6 @@ fn boot_once(
             suite,
             dir,
             stem,
-            skip,
         },
     )
 }
@@ -931,6 +929,19 @@ pub fn run(
                 .map_err(|e| format!("writing {}: {e}", dot.display()))?;
             configured = make(&["olddefconfig"], "fragment.log", 1)? == 0;
             fragment_missed = kconfig::missed(&kconfig::load(&dot)?, &fragment);
+            if configured && !s.kunit_skip.is_empty() && !BUILD_ONLY.contains(&s.config) {
+                // With KUNIT_ALL_TESTS off the tests keep the values it gave them, and their options can be turned off one by one.
+                let off: Vec<(String, String)> = std::iter::once("KUNIT_ALL_TESTS")
+                    .chain(s.kunit_skip.iter().map(String::as_str))
+                    .map(|o| (o.to_owned(), "n".to_owned()))
+                    .collect();
+                let resolved = std::fs::read_to_string(&dot)
+                    .map_err(|e| format!("reading {}: {e}", dot.display()))?;
+                std::fs::write(&dot, kconfig::merge(&resolved, &off))
+                    .map_err(|e| format!("writing {}: {e}", dot.display()))?;
+                configured = make(&["olddefconfig"], "kunit-skip.log", 1)? == 0;
+                fragment_missed.extend(kconfig::missed(&kconfig::load(&dot)?, &off[1..]));
+            }
             let _ = std::fs::copy(&dot, cell_dir.join(".config"));
         }
         steps.push(Step {
@@ -1173,7 +1184,7 @@ fn hostname() -> String {
         .unwrap_or_default()
 }
 
-/// The modules named on the `# gk:kunit-skip` lines of a fragment. Kconfig reads those lines as comments, and the KUnit boot skips the suites of these modules. A suite goes there when it fails under TCG on a loaded host for reasons that have nothing to do with the compiler, and when `KUNIT_ALL_TESTS` hides its option so the fragment cannot turn it off. Being in the fragment puts the list in the cell's identity.
+/// The options named on the `# gk:kunit-skip` lines of a fragment, which Kconfig reads as comments. A KUnit test goes there when it fails under TCG on a loaded host for reasons that have nothing to do with the compiler. `KUNIT_ALL_TESTS` hides its option, so a plain `=n` in the fragment would not take, and the configuration step turns these off in a second pass with `KUNIT_ALL_TESTS` off instead. Being in the fragment puts the list in the cell's identity.
 fn kunit_skip(fragment: &str) -> Vec<String> {
     fragment
         .lines()
@@ -1196,9 +1207,12 @@ mod tests {
     use super::*;
 
     #[test]
-    fn the_fragment_names_the_skipped_modules() {
-        let text = "CONFIG_KUNIT=y\n# gk:kunit-skip drm_sched_tests test_ratelimit\n# a comment\n";
-        assert_eq!(kunit_skip(text), ["drm_sched_tests", "test_ratelimit"]);
+    fn the_fragment_names_the_skipped_tests() {
+        let text = "CONFIG_KUNIT=y\n# gk:kunit-skip DRM_SCHED_KUNIT_TEST RATELIMIT_KUNIT_TEST\n# a comment\n";
+        assert_eq!(
+            kunit_skip(text),
+            ["DRM_SCHED_KUNIT_TEST", "RATELIMIT_KUNIT_TEST"]
+        );
         assert!(kunit_skip("CONFIG_KUNIT=y\n").is_empty());
     }
 

docs/spec/07-build-and-boot.md

@@ -59,7 +59,7 @@ The fragment turns two things off, `CONFIG_WERROR` and `CONFIG_GCC_PLUGINS`. Tha
 
 `CONFIG_DEBUG_INFO_BTF=n` keeps `pahole` out of the build. BTF generation depends on the pahole version, a host tool with its own history of breaking kernels, and that is not the question here.
 
-Era fragments drop what the era does not have: KUnit before 5.5, devtmpfs before 2.6.32, `PANIC_ON_OOPS` before 2.6.x, the PL011 console for trees without it. The M13 and M14 fragments also turn off `KUNIT_FAULT_TEST`, whose suite oopses on purpose and so panics a kernel built with `PANIC_ON_OOPS`. The first M13 cell, 6.12.111 with gcc-12.2.0, died that way in its KUnit boots, and so did the M14 era cell for 6.18.54. 7.2.8 skips the suite by itself. Two other suites, the DRM scheduler tests and the ratelimit stress test, fail under TCG on a loaded host with any compiler, and `KUNIT_ALL_TESTS` hides their options so a fragment cannot turn them off. A fragment line `# gk:kunit-skip <module>...` names such modules instead, and the KUnit boot passes them to `kunit.filter` with `kunit.filter_action=skip`, so they show up as skipped rather than vanish. M14 skips `drm_sched_tests` and `test_ratelimit`. `configs/fragment.<era>` holds each, and `gk` records which requested options did not take effect after the configuration step. An option that did not take effect with the era GCC is expected. One that took effect with the era GCC and not with another column is a finding: it means a Kconfig compiler probe disagreed, which document 11.3 wants.
+Era fragments drop what the era does not have: KUnit before 5.5, devtmpfs before 2.6.32, `PANIC_ON_OOPS` before 2.6.x, the PL011 console for trees without it. The M13 and M14 fragments also turn off `KUNIT_FAULT_TEST`, whose suite oopses on purpose and so panics a kernel built with `PANIC_ON_OOPS`. The first M13 cell, 6.12.111 with gcc-12.2.0, died that way in its KUnit boots, and so did the M14 era cell for 6.18.54. 7.2.8 skips the suite by itself. Two other suites, the DRM scheduler tests and the ratelimit stress test, fail under TCG on a loaded host with any compiler, and `KUNIT_ALL_TESTS` hides their options so a fragment cannot turn them off. A fragment line `# gk:kunit-skip <option>...` names such tests instead. After the fragment is applied, `gk` turns `KUNIT_ALL_TESTS` off in the resolved `.config`, which shows the test options again with the values it gave them, turns the named ones off and runs `olddefconfig` a second time into `kunit-skip.log`. On 7.2.8 that pass changes nothing but the named tests and `DRM_SCHED`, which only the scheduler test selects. M14 skips `DRM_SCHED_KUNIT_TEST` and `RATELIMIT_KUNIT_TEST`. The KUnit `module!=` filter on the command line looked simpler and was tried first, but on 7.2.8 it panics in `attr_string_filter` on a test case with no module name, so it is not used. `configs/fragment.<era>` holds each, and `gk` records which requested options did not take effect after the configuration step. An option that did not take effect with the era GCC is expected. One that took effect with the era GCC and not with another column is a finding: it means a Kconfig compiler probe disagreed, which document 11.3 wants.
 
 Two kinds of configuration differences between columns are expected and not failures: `CC_VERSION_TEXT`, `GCC_VERSION`, `AS_VERSION`, `LD_VERSION` and the `CC_HAS_*` family that probes the compiler. They are what the configuration differential measures.