diff --git a/.github/scripts/trunk-impacted-targets.js b/.github/scripts/trunk-impacted-targets.js index 7df9c9dcd64e..27b6d6ac2133 100644 --- a/.github/scripts/trunk-impacted-targets.js +++ b/.github/scripts/trunk-impacted-targets.js @@ -577,9 +577,13 @@ function loadContractSurfaces(repoRoot, products) { // exactly when the dependents most need to be tested alongside it. const CONTRACT_DECLARATIONS = ['turbo.json', 'package.json'] +function isContractDeclaration(product, file) { + return CONTRACT_DECLARATIONS.includes(file.slice(`products/${product}/`.length)) +} + function touchesContractSurface(product, file, contractSurfaces) { const relativePath = file.slice(`products/${product}/`.length) - if (CONTRACT_DECLARATIONS.includes(relativePath)) { + if (isContractDeclaration(product, file)) { return true } const matcher = contractSurfaces.get(product) @@ -791,7 +795,7 @@ function computeTargets(changedFiles, context) { } } - const changedIsolatedProducts = new Set() + const cascadeSeeds = new Set() let inertFiles = 0 for (const file of changedFiles) { @@ -930,12 +934,24 @@ function computeTargets(changedFiles, context) { if (isolatedProducts.has(product)) { targets.add(pyProduct(product)) if (touchesContractSurface(product, file, contractSurfaces)) { - changedIsolatedProducts.add(product) + cascadeSeeds.add(product) } } else if (backendDetachedProducts.has(product)) { // No backend suite covers this product and no declared - // module imports it, so the lane it keeps is its own. + // module imports it, so the lane it keeps is its own. This + // case comes first because it also answers the declaration + // case below: a product absent from the module graph has no + // importers for the cascade to name. + targets.add(pyProduct(product)) + } else if (isContractDeclaration(product, file)) { + // A non-isolated product's backend code has no declared + // boundary, so it keeps widening below. Its declarations + // are a different kind of file: they configure this + // product's own tasks, including the backend:test command + // most of them carry, and every importer that a change to + // them can reach is named by the cascade instead. targets.add(pyProduct(product)) + cascadeSeeds.add(product) } else { allPyProducts() } @@ -956,11 +972,12 @@ function computeTargets(changedFiles, context) { // backend target. Only 14 of the products declare a contract check, so // widening on each of them would collapse every cascade to the full set. // - // The seeds are the products whose contract surface changed, and the - // dependents are one hop deep. See the two numbered narrowings at the top - // of this file for what that gives up. - if (changedIsolatedProducts.size > 0) { - const dependents = tachDependentProducts([...changedIsolatedProducts], tachGraph) + // The seeds are the products whose contract surface changed, plus any + // product whose own declarations changed, and the dependents are one hop + // deep. See the two numbered narrowings at the top of this file for what + // that gives up. + if (cascadeSeeds.size > 0) { + const dependents = tachDependentProducts([...cascadeSeeds], tachGraph) if (dependents === null) { allPyProducts() } else { diff --git a/.github/scripts/trunk-impacted-targets.test.js b/.github/scripts/trunk-impacted-targets.test.js index d55f976b8f6b..de648bc09add 100644 --- a/.github/scripts/trunk-impacted-targets.test.js +++ b/.github/scripts/trunk-impacted-targets.test.js @@ -364,6 +364,41 @@ test('a non-isolated product change widens to every backend target', () => { } }) +// 63 of the products are non-isolated, and their package.json carries the +// backend:test command rather than being a bare JS manifest. It still claims a +// backend lane, just this product's own plus its importers, instead of dragging +// in every other product's. +test("a non-isolated product's declarations claim its own lane, not every backend lane", () => { + for (const file of ['products/gamma/package.json', 'products/gamma/turbo.json']) { + assert.deepEqual(computeTargets([file], CONTEXT), ['fe:product:gamma', 'py:product:gamma'], file) + } +}) + +test("a non-isolated product's declarations still seed the dependent cascade", () => { + const context = { + ...CONTEXT, + tachGraph: { graph: new Map(), tachDependents: (changed) => (changed.includes('gamma') ? ['alpha'] : []) }, + } + assert.deepEqual(computeTargets(['products/gamma/package.json'], context), [ + 'fe:product:gamma', + 'py:product:alpha', + 'py:product:gamma', + ]) +}) + +// The narrowing is scoped to the declarations. Backend code in a non-isolated +// product has no declared boundary, which is the whole reason it widens. +test('a non-isolated product keeps widening on everything that is not a declaration', () => { + for (const file of ['products/gamma/backend/api.py', 'products/gamma/mcp/tools.yaml']) { + assert.equal(computeTargets([file], CONTEXT).includes('py:product:alpha'), true, file) + } +}) + +test('an unavailable tach graph widens a declaration change too', () => { + const noTach = { ...CONTEXT, tachGraph: null } + assert.equal(computeTargets(['products/gamma/package.json'], noTach).includes('py:core'), true) +}) + test('an unavailable tach graph widens backend changes instead of narrowing', () => { const noTach = { ...CONTEXT, tachGraph: null } const targets = computeTargets(['products/alpha/backend/api.py'], noTach) @@ -432,10 +467,12 @@ test('python inside a declared workspace package still claims the backend lanes' // Only the declared package subtrees narrow. The product root holds the files // that decide isolation and contract surface, and anything else under the // product is unclassified in the same way it was before. -test('files outside the declared workspace packages keep widening', () => { - for (const file of ['products/gamma/package.json', 'products/gamma/scripts/release.mjs']) { - assert.equal(computeTargets([file], WORKSPACE_CONTEXT).includes('py:core'), true, file) - } +test('files outside the declared workspace packages keep their backend claim', () => { + // The root declarations get the narrower per-product treatment below rather + // than the workspace one, so what matters here is that they still claim a + // backend lane instead of being read as a JS manifest. + assert.equal(computeTargets(['products/gamma/package.json'], WORKSPACE_CONTEXT).includes('py:product:gamma'), true) + assert.equal(computeTargets(['products/gamma/scripts/release.mjs'], WORKSPACE_CONTEXT).includes('py:core'), true) }) // The workspace declaration and its lockfile sit at the product root, so the