fix[notask]: decouple mobile shard validation from desktop integration tests

packages/vla-ggml/package.json

@@ -24,8 +24,9 @@
     "test:integration": "npm run build:ts && npm run test:integration:generate && bare test/integration/all.js --exit",
     "test:integration:generate": "brittle -r test/integration/all.js test/integration/*.test.js && npm run test:mobile:generate",
     "test:unit:generate": "brittle -r test/unit/all.js test/unit/*.test.js",
-    "test:unit": "npm run build:ts && npm run test:unit:generate && bare test/unit/all.js --exit && npm run test:prestage",
+    "test:unit": "npm run build:ts && npm run test:unit:generate && bare test/unit/all.js --exit && npm run test:prestage && npm run test:mobile:groups && npm run test:mobile:validate",
     "test:prestage": "node --test scripts/__tests__/generate-prestage-block.test.js",
+    "test:mobile:groups": "node --test scripts/__tests__/mobile-test-groups.test.js",
     "test:cpp:build": "bare-make generate -D BUILD_TESTING=ON && bare-make build --target addon-test",
     "test:cpp:run": "cd build/test/unit/ && ./addon-test --gtest_output=xml:cpp-test-results.xml",
     "test:cpp": "npm run test:cpp:build && npm run test:cpp:run",

packages/vla-ggml/scripts/__tests__/mobile-test-groups.test.js

@@ -0,0 +1,106 @@
+'use strict'
+
+const test = require('node:test')
+const assert = require('node:assert/strict')
+const fs = require('node:fs')
+const path = require('node:path')
+
+const { validateTestGroups, platformNames } = require('../lib/validate-test-groups.js')
+const groups = require('../../test/mobile/test-groups.json')
+
+const integrationAutoPath = path.resolve(__dirname, '../../test/mobile/integration.auto.cjs')
+
+function generatedRunners() {
+  const content = fs.readFileSync(integrationAutoPath, 'utf8')
+  return Array.from(content.matchAll(/^async function (run[A-Za-z0-9_]+)\s*\(/gm), (m) => m[1])
+}
+
+test('the committed test-groups.json covers every generated runner', () => {
+  assert.deepEqual(validateTestGroups(groups, generatedRunners()), [])
+})
+
+test('deferred runners are declared, not silently absent', () => {
+  // pi05 mobile coverage is deferred pending a project-owned CDN mirror, and
+  // pi05.test.js is gated on-device by `_skipMobilePi05`. Recording it here is
+  // what keeps "not scheduled" distinguishable from "forgotten".
+  assert.deepEqual(groups.deferred, ['runPi05Test'])
+  for (const platform of platformNames(groups)) {
+    const scheduled = Object.values(groups[platform]).flat()
+    assert.ok(
+      !scheduled.includes('runPi05Test'),
+      `runPi05Test must not be scheduled on ${platform}`
+    )
+  }
+})
+
+test('"deferred" is a top-level key, never a platform', () => {
+  // The Device Farm composites read only `.<platform>`, so a `deferred` key
+  // nested inside ios/android would be scheduled as a real shard.
+  assert.ok(!platformNames(groups).includes('deferred'))
+  assert.deepEqual(platformNames(groups).sort(), ['android', 'ios'])
+})
+
+test('a "deferred" key nested inside a platform is reported', () => {
+  // The assertion above only covers the committed file's shape. This covers the
+  // hazard itself: nested under a platform, `deferred` is just another array of
+  // runner names, so every other rule is satisfied and the file would otherwise
+  // validate clean — while upload-to-devicefarm schedules it as a real shard.
+  const nested = {
+    ios: { smolvla: ['runAddonTest'], deferred: ['runPi05Test'] },
+    android: { smolvla: ['runAddonTest'], deferred: ['runPi05Test'] }
+  }
+  const problems = validateTestGroups(nested, ['runAddonTest', 'runPi05Test'])
+
+  assert.equal(problems.length, 2, 'exactly one problem per platform')
+  for (const platform of ['ios', 'android']) {
+    const reported = problems.some(
+      (p) => p.startsWith(`[${platform}]`) && p.includes('nested inside the platform map')
+    )
+    assert.ok(reported, `${platform} must report the nested "deferred" key`)
+  }
+})
+
+test('an unassigned runner is reported', () => {
+  const problems = validateTestGroups(groups, [...generatedRunners(), 'runBrandNewTest'])
+  assert.equal(problems.length, platformNames(groups).length)
+  assert.ok(problems.every((p) => p.includes('runBrandNewTest')))
+})
+
+test('a typo in a group is reported', () => {
+  const typo = {
+    ios: { smolvla: ['runAddonTest', 'runTypoTest'] },
+    deferred: []
+  }
+  const problems = validateTestGroups(typo, ['runAddonTest'])
+  assert.ok(problems.some((p) => p.includes('runTypoTest') && p.includes('do not exist')))
+})
+
+test('a stale deferred entry is reported', () => {
+  const stale = {
+    ios: { smolvla: ['runAddonTest'] },
+    deferred: ['runRemovedTest']
+  }
+  const problems = validateTestGroups(stale, ['runAddonTest'])
+  assert.ok(problems.some((p) => p.includes('runRemovedTest') && p.includes('do not exist')))
+})
+
+test('a runner that is both scheduled and deferred is reported', () => {
+  const contradictory = {
+    ios: { smolvla: ['runAddonTest'] },
+    deferred: ['runAddonTest']
+  }
+  const problems = validateTestGroups(contradictory, ['runAddonTest'])
+  assert.ok(problems.some((p) => p.includes('both scheduled and listed')))
+})
+
+test('metadata keys that are not platform maps are ignored', () => {
+  // OCR ships a top-level `perf_report_filter` string; the shape must tolerate
+  // sibling metadata without treating it as a platform.
+  const withMetadata = {
+    ios: { smolvla: ['runAddonTest'] },
+    perf_report_filter: 'something|else',
+    deferred: []
+  }
+  assert.deepEqual(platformNames(withMetadata), ['ios'])
+  assert.deepEqual(validateTestGroups(withMetadata, ['runAddonTest']), [])
+})

packages/vla-ggml/scripts/generate-mobile-integration-tests.js

@@ -7,7 +7,6 @@ const repoRoot = path.resolve(__dirname, '..')
 const integrationDir = path.join(repoRoot, 'test', 'integration')
 const mobileDir = path.join(repoRoot, 'test', 'mobile')
 const outputFile = path.join(mobileDir, 'integration.auto.cjs')
-const groupsFile = path.join(mobileDir, 'test-groups.json')
 
 function getIntegrationFiles() {
   if (!fs.existsSync(integrationDir)) {
@@ -64,50 +63,21 @@ function buildFileContents(files) {
   return `${lines.join('\n')}\n`
 }
 
-function validateGroups(functionNames) {
-  if (!fs.existsSync(groupsFile)) {
-    console.warn('[warn] test-groups.json not found — skipping split validation')
-    return
-  }
-  const groups = JSON.parse(fs.readFileSync(groupsFile, 'utf-8'))
-  const nameSet = new Set(functionNames)
-  for (const [platform, splits] of Object.entries(groups)) {
-    const covered = new Set(Object.values(splits).flat())
-    const missing = functionNames.filter((n) => !covered.has(n))
-    const extra = [...covered].filter((n) => !nameSet.has(n))
-    if (missing.length) {
-      throw new Error(
-        '[' +
-          platform +
-          '] Tests not assigned to any group in test-groups.json:\n  ' +
-          missing.join('\n  ') +
-          '\nAdd them to a group in test/mobile/test-groups.json.'
-      )
-    }
-    if (extra.length) {
-      throw new Error(
-        '[' +
-          platform +
-          '] test-groups.json references non-existent tests:\n  ' +
-          extra.join('\n  ') +
-          '\nRemove them or check for typos.'
-      )
-    }
-  }
-  console.log('Group coverage validated — all tests assigned for every platform.')
-}
-
+// NOTE: this generator deliberately performs no test-groups.json validation.
+// `npm run test:integration` chains it (so the committed integration.auto.cjs
+// can never go stale), which means anything that throws here takes desktop
+// integration tests down on every platform. Group coverage is a mobile
+// scheduling concern, so it lives in `npm run test:mobile:validate`
+// (scripts/validate-mobile-tests.js) and runs in the ungated ts-checks job.
 function main() {
   const files = getIntegrationFiles()
   if (files.length === 0) {
     throw new Error(`No integration test files found inside ${integrationDir}`)
   }
 
-  const functionNames = files.map(toFunctionName)
   const content = buildFileContents(files)
   fs.writeFileSync(outputFile, content, 'utf8')
   console.log(`Generated ${outputFile} with ${files.length} integration runners.`)
-  validateGroups(functionNames)
 }
 
 if (require.main === module) {

packages/vla-ggml/scripts/lib/validate-test-groups.js

@@ -0,0 +1,131 @@
+'use strict'
+
+// Group-coverage rules for test/mobile/test-groups.json.
+//
+// Deliberately dependency-free and side-effect-free (no fs, no process.exit) so
+// the same rules run under `node` from validate-mobile-tests.js and are unit
+// testable from scripts/__tests__/mobile-test-groups.test.js.
+//
+// This check lives OUTSIDE the generator on purpose. It answers a mobile
+// scheduling question — "is every on-device runner assigned to a Device Farm
+// shard?" — which has no bearing on whether integration.auto.cjs was written
+// correctly. Bundling it into the generator once let a Device Farm scheduling
+// edit abort `npm run test:integration`, taking desktop CI down on all seven
+// platforms (PR #4006).
+
+// Runners deliberately not scheduled on Device Farm are listed under this
+// top-level key. It sits beside the platform maps rather than inside one
+// because the CI composites consume only `.<platform>` and ignore every other
+// top-level key (see .github/actions/run-mobile-integration-tests/
+// upload-to-devicefarm/action.yml). Nesting it under `ios`/`android` would
+// instead schedule it as a real shard.
+const DEFERRED_KEY = 'deferred'
+
+// A platform entry is a `{ groupName: [runner, ...] }` map. Anything else at the
+// top level is metadata for another consumer — `deferred` here, OCR's
+// `perf_report_filter` — and is not a platform.
+function isPlatformEntry(value) {
+  return typeof value === 'object' && value !== null && !Array.isArray(value)
+}
+
+function platformNames(groups) {
+  return Object.keys(groups).filter((key) => isPlatformEntry(groups[key]))
+}
+
+function coveredRunners(platformEntry) {
+  return Object.values(platformEntry).filter(Array.isArray).flat()
+}
+
+function deferredRunners(groups) {
+  const deferred = groups[DEFERRED_KEY]
+  return Array.isArray(deferred) ? deferred : []
+}
+
+// Returns a list of human-readable problem strings; empty means valid.
+// `runners` is the authoritative runner-name list, derived from the generated
+// integration.auto.cjs by the caller.
+function validateTestGroups(groups, runners) {
+  const problems = []
+  const known = new Set(runners)
+  const deferred = deferredRunners(groups)
+
+  // A stale `deferred` entry is worse than a noisy one: it would silently
+  // excuse a runner that no longer exists, and mask a real gap if the name is
+  // ever reused.
+  const unknownDeferred = deferred.filter((name) => !known.has(name))
+  if (unknownDeferred.length) {
+    problems.push(
+      `[${DEFERRED_KEY}] lists runners that do not exist:\n  ` +
+        unknownDeferred.join('\n  ') +
+        '\nRemove them or check for typos.'
+    )
+  }
+
+  const platforms = platformNames(groups)
+  if (platforms.length === 0) {
+    problems.push(
+      'test-groups.json declares no platform maps.\n' +
+        'Expected at least one top-level `{ "<platform>": { "<group>": [runners] } }` entry.'
+    )
+    return problems
+  }
+
+  const deferredSet = new Set(deferred)
+
+  for (const platform of platforms) {
+    // `deferred` nested inside a platform is indistinguishable from a shard: to
+    // `coveredRunners` below it is just another array of runner names, so the
+    // whole file would validate clean — and `upload-to-devicefarm` turns every
+    // `{ groupName: [runners] }` entry into a Device Farm spec, so those runners
+    // would be scheduled (and billed) under a shard literally named "deferred".
+    // Reserving the name here is what makes the top-level rule enforceable
+    // rather than merely documented.
+    if (Object.prototype.hasOwnProperty.call(groups[platform], DEFERRED_KEY)) {
+      problems.push(
+        `[${platform}] "${DEFERRED_KEY}" is nested inside the platform map.\n` +
+          `It must be a top-level key: nested here it is scheduled as a real Device Farm shard.`
+      )
+    }
+
+    const covered = new Set(coveredRunners(groups[platform]))
+
+    const missing = runners.filter((name) => !covered.has(name) && !deferredSet.has(name))
+    if (missing.length) {
+      problems.push(
+        `[${platform}] runners not assigned to any group:\n  ` +
+          missing.join('\n  ') +
+          `\nAdd them to a group in test/mobile/test-groups.json, or to the ` +
+          `top-level "${DEFERRED_KEY}" list if they are intentionally not run on device.`
+      )
+    }
+
+    const extra = [...covered].filter((name) => !known.has(name))
+    if (extra.length) {
+      problems.push(
+        `[${platform}] groups reference runners that do not exist:\n  ` +
+          extra.join('\n  ') +
+          '\nRemove them or check for typos.'
+      )
+    }
+
+    // A runner in both a shard and `deferred` is contradictory: it would run on
+    // device while claiming to be deferred.
+    const contradictory = [...covered].filter((name) => deferredSet.has(name))
+    if (contradictory.length) {
+      problems.push(
+        `[${platform}] runners are both scheduled and listed as "${DEFERRED_KEY}":\n  ` +
+          contradictory.join('\n  ') +
+          `\nRemove them from one or the other.`
+      )
+    }
+  }
+
+  return problems
+}
+
+module.exports = {
+  DEFERRED_KEY,
+  validateTestGroups,
+  platformNames,
+  deferredRunners
+}

packages/vla-ggml/scripts/validate-mobile-tests.js

@@ -4,9 +4,12 @@
 const fs = require('fs')
 const path = require('path')
 
+const { validateTestGroups } = require('./lib/validate-test-groups.js')
+
 const repoRoot = path.resolve(__dirname, '..')
 const integrationDir = path.join(repoRoot, 'test', 'integration')
 const mobileAutoFile = path.join(repoRoot, 'test', 'mobile', 'integration.auto.cjs')
+const groupsFile = path.join(repoRoot, 'test', 'mobile', 'test-groups.json')
 
 function getIntegrationTestFiles() {
   if (!fs.existsSync(integrationDir)) {
@@ -32,6 +35,14 @@ function getGeneratedIntegrationRefs(content) {
   return references
 }
 
+// integration.auto.cjs declares one `async function run<Name>` per on-device
+// test. Once it is confirmed in sync with test/integration (above), it is the
+// authoritative runner-name list — the same source .github/actions/
+// run-mobile-integration-tests/validate-devices uses.
+function getGeneratedRunnerNames(content) {
+  return Array.from(content.matchAll(/^async function (run[A-Za-z0-9_]+)\s*\(/gm), (m) => m[1])
+}
+
 function setDiff(left, right) {
   return [...left].filter((item) => !right.has(item)).sort()
 }
@@ -73,20 +84,37 @@ try {
     process.exit(0)
   }
 
-  // Keep timestamp validation as a fast stale-content signal for edited tests.
-  const latestIntegrationTime = Math.max(
-    ...integrationFiles.map((f) => fs.statSync(path.join(integrationDir, f)).mtimeMs)
-  )
-  const mobileAutoTime = fs.statSync(mobileAutoFile).mtimeMs
+  // There is deliberately no mtime comparison here. `buildFileContents`
+  // (generate-mobile-integration-tests.js) derives integration.auto.cjs from the
+  // sorted *filenames* under test/integration/ and never opens a test file, so
+  // editing a test's body cannot make the generated file stale. A timestamp
+  // check can therefore only produce false positives — and since this script now
+  // runs as part of `npm run test:unit`, each one would be a hard failure telling
+  // the author to regenerate a byte-identical file. Every staleness it could
+  // legitimately catch (a test added, renamed or removed) is already caught by
+  // the content-based reference diff above.
+
+  // Device Farm shard coverage. This lives here rather than in the generator so
+  // that a mobile scheduling mistake can never abort `npm run test:integration`
+  // and take desktop CI down with it.
+  if (!fs.existsSync(groupsFile)) {
+    console.log('✅ Mobile integration tests are up to date (no test-groups.json — single-spec)')
+    process.exit(0)
+  }
 
-  if (latestIntegrationTime > mobileAutoTime) {
-    console.error('❌ Mobile integration tests are out of date!')
-    console.error('   Integration tests modified after mobile tests were generated.')
-    console.error('   Run: npm run test:mobile:generate')
+  const groups = JSON.parse(fs.readFileSync(groupsFile, 'utf8'))
+  const runners = getGeneratedRunnerNames(mobileAutoContent)
+  const problems = validateTestGroups(groups, runners)
+
+  if (problems.length > 0) {
+    console.error('❌ test-groups.json does not cover every mobile runner\n')
+    problems.forEach((problem) => console.error(`   ${problem}\n`))
     process.exit(1)
   }
 
-  console.log('✅ Mobile integration tests are up to date')
+  console.log(
+    `✅ Mobile integration tests are up to date (${runners.length} runner(s), group coverage OK)`
+  )
   process.exit(0)
 } catch (error) {
   console.error('Error validating mobile tests:', error.message)