Steve-Dee-Designs
/
woocommerce
mirror of https://github.com/woocommerce/woocommerce.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Enhance CI Change Detection (#43596) 

Realistically speaking, if a `package.json` file is changed we should consider it to be a change
for the entire package. This helps because it might impact the config itself. The same is true
of changes to `pnpm-lock.yaml` since those may update dependencies for any package. I
have also added a `-f` option to the CI Jobs CLI command to trigger all jobs.
This commit is contained in:
Christopher Allford 2024-01-15 13:30:31 -08:00 committed by GitHub
parent 43be71a52f
commit 5425a9536a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 237 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
tools/monorepo-utils/dist/index.js vendored
View File

File diff suppressed because one or more lines are too long

19
tools/monorepo-utils/src/ci-jobs/index.ts
View File

@ -21,14 +21,25 @@ const program = new Command( 'ci-jobs' )
'<base-ref>',
'Base ref to compare the current ref against for change detection.'
)
.action( async ( baseRef: string ) => {
.option(
'-f --force',
'Forces all projects to be marked as changed.',
false
)
.action( async ( baseRef: string, options ) => {
Logger.startTask( 'Parsing Project Graph', true );
const projectGraph = buildProjectGraph();
Logger.endTask( true );
Logger.startTask( 'Pulling File Changes', true );
const fileChanges = getFileChanges( projectGraph, baseRef );
Logger.endTask( true );
let fileChanges;
if ( options.force ) {
Logger.warn( 'Forcing all projects to be marked as changed.' );
fileChanges = true;
} else {
Logger.startTask( 'Pulling File Changes', true );
fileChanges = getFileChanges( projectGraph, baseRef );
Logger.endTask( true );
}
Logger.startTask( 'Creating Jobs', true );
const jobs = await createJobsForChanges( projectGraph, fileChanges, {

26
tools/monorepo-utils/src/ci-jobs/lib/__tests__/config.spec.ts
View File

@ -33,7 +33,10 @@ describe( 'Config', () => {
jobs: [
{
type: JobType.Lint,
changes: [ makeRe( '/src/**/*.{js,jsx,ts,tsx}' ) ],
changes: [
/^package\.json$/,
makeRe( '/src/**/*.{js,jsx,ts,tsx}' ),
],
command: 'foo',
},
],
@ -57,7 +60,10 @@ describe( 'Config', () => {
jobs: [
{
type: JobType.Lint,
changes: [ makeRe( '/src/**/*.{js,jsx,ts,tsx}' ) ],
changes: [
/^package\.json$/,
makeRe( '/src/**/*.{js,jsx,ts,tsx}' ),
],
command: 'foo <baseRef>',
},
],
@ -100,6 +106,7 @@ describe( 'Config', () => {
{
type: JobType.Lint,
changes: [
/^package\.json$/,
makeRe( '/src/**/*.{js,jsx,ts,tsx}' ),
makeRe( '/test/**/*.{js,jsx,ts,tsx}' ),
],
@ -130,7 +137,10 @@ describe( 'Config', () => {
{
type: JobType.Test,
name: 'default',
changes: [ makeRe( '/src/**/*.{js,jsx,ts,tsx}' ) ],
changes: [
/^package\.json$/,
makeRe( '/src/**/*.{js,jsx,ts,tsx}' ),
],
command: 'foo',
},
],
@ -164,7 +174,10 @@ describe( 'Config', () => {
{
type: JobType.Test,
name: 'default',
changes: [ makeRe( '/src/**/*.{js,jsx,ts,tsx}' ) ],
changes: [
/^package\.json$/,
makeRe( '/src/**/*.{js,jsx,ts,tsx}' ),
],
command: 'foo',
testEnv: {
start: 'bar',
@ -199,7 +212,10 @@ describe( 'Config', () => {
{
type: JobType.Test,
name: 'default',
changes: [ makeRe( '/src/**/*.{js,jsx,ts,tsx}' ) ],
changes: [
/^package\.json$/,
makeRe( '/src/**/*.{js,jsx,ts,tsx}' ),
],
command: 'foo',
cascadeKeys: [ 'bar' ],
},

42
tools/monorepo-utils/src/ci-jobs/lib/__tests__/file-changes.spec.ts
View File

@ -57,4 +57,46 @@ baz/project-d/baz.js`;
} );
} );
} );
it( 'should see pnpm-lock.yaml file changes as universal changes', () => {
jest.mocked( execSync ).mockImplementation( ( command ) => {
if ( command === 'git diff --name-only origin/trunk' ) {
return `test/project-a/package.json
foo/project-b/foo.js
pnpm-lock.yaml
bar/project-c/bar.js
baz/project-d/baz.js`;
}
throw new Error( 'Invalid command' );
} );
const fileChanges = getFileChanges(
{
name: 'project-a',
path: 'test/project-a',
dependencies: [
{
name: 'project-b',
path: 'foo/project-b',
dependencies: [
{
name: 'project-c',
path: 'bar/project-c',
dependencies: [],
},
],
},
{
name: 'project-c',
path: 'bar/project-c',
dependencies: [],
},
],
},
'origin/trunk'
);
expect( fileChanges ).toStrictEqual( true );
} );
} );

45
tools/monorepo-utils/src/ci-jobs/lib/__tests__/job-processing.spec.ts
View File

@ -8,7 +8,7 @@ import { parseTestEnvConfig } from '../test-environment';
jest.mock( '../test-environment' );
describe( 'Job Processing', () => {
describe( 'getFileChanges', () => {
describe( 'createJobsForChanges', () => {
it( 'should do nothing with no CI configs', async () => {
const jobs = await createJobsForChanges(
{
@ -686,5 +686,48 @@ describe( 'Job Processing', () => {
},
} );
} );
it( 'should trigger all jobs for a single node with changes set to "true"', async () => {
const jobs = await createJobsForChanges(
{
name: 'test',
path: 'test',
ciConfig: {
jobs: [
{
type: JobType.Lint,
changes: [ /test.js$/ ],
command: 'test-lint',
},
{
type: JobType.Test,
name: 'Default',
changes: [ /test.js$/ ],
command: 'test-cmd',
},
],
},
dependencies: [],
},
true,
{}
);
expect( jobs.lint ).toHaveLength( 1 );
expect( jobs.lint ).toContainEqual( {
projectName: 'test',
command: 'test-lint',
} );
expect( jobs.test ).toHaveLength( 1 );
expect( jobs.test ).toContainEqual( {
projectName: 'test',
name: 'Default',
command: 'test-cmd',
testEnv: {
shouldCreate: false,
envVars: {},
},
} );
} );
} );
} );

1
tools/monorepo-utils/src/ci-jobs/lib/__tests__/project-graph.spec.ts
View File

@ -80,6 +80,7 @@ describe( 'Project Graph', () => {
command: 'foo',
type: 'lint',
changes: [
/^package\.json$/,
/^(?:src(?:\/|\/(?:(?!(?:\/|^)\.).)*?\/)(?!\.)[^/]*?\.js|src(?:\/|\/(?:(?!(?:\/|^)\.).)*?\/)(?!\.)[^/]*?\.jsx|src(?:\/|\/(?:(?!(?:\/|^)\.).)*?\/)(?!\.)[^/]*?\.ts|src(?:\/|\/(?:(?!(?:\/|^)\.).)*?\/)(?!\.)[^/]*?\.tsx)$/,
],
},

31
tools/monorepo-utils/src/ci-jobs/lib/config.ts
View File

@ -57,9 +57,28 @@ interface BaseJobConfig {
/**
* Parses and validates a raw change config entry.
*
* @param {string|string[]} raw The raw config to parse.
* @param {string|string[]} raw The raw config to parse.
* @param {string[]} extraGlobs Any extra globs that should be added to the configuration.
*/
function parseChangesConfig( raw: unknown ): RegExp[] {
function parseChangesConfig(
raw: unknown,
extraGlobs: string[] = []
): RegExp[] {
const changes: RegExp[] = [];
// Make sure to include any extra glob patterns that were passed in.
// This allows us to make sure we're watching for changes in files
// that may implicitly be impactful but shouldn't need to be
// stated explicitly in the list of file changes.
for ( const entry of extraGlobs ) {
const regex = makeRe( entry );
if ( ! regex ) {
throw new Error( 'Invalid extra glob pattern.' );
}
changes.push( regex );
}
if ( typeof raw === 'string' ) {
const regex = makeRe( raw );
if ( ! regex ) {
@ -68,7 +87,8 @@ function parseChangesConfig( raw: unknown ): RegExp[] {
);
}
return [ regex ];
changes.push( regex );
return changes;
}
if ( ! Array.isArray( raw ) ) {
@ -77,7 +97,6 @@ function parseChangesConfig( raw: unknown ): RegExp[] {
);
}
const changes: RegExp[] = [];
for ( const entry of raw ) {
if ( typeof entry !== 'string' ) {
throw new ConfigError(
@ -156,7 +175,7 @@ function parseLintJobConfig( raw: any ): LintJobConfig {
return {
type: JobType.Lint,
changes: parseChangesConfig( raw.changes ),
changes: parseChangesConfig( raw.changes, [ 'package.json' ] ),
command: raw.command,
};
}
@ -306,7 +325,7 @@ function parseTestJobConfig( raw: any ): TestJobConfig {
const config: TestJobConfig = {
type: JobType.Test,
name: raw.name,
changes: parseChangesConfig( raw.changes ),
changes: parseChangesConfig( raw.changes, [ 'package.json' ] ),
command: raw.command,
};

14
tools/monorepo-utils/src/ci-jobs/lib/file-changes.ts
View File

@ -77,20 +77,26 @@ function getChangedFilesForProject(
*
* @param {Object} projectGraph The project graph to assign changes for.
* @param {string} baseRef The git ref to compare against for changes.
* @return {Object} A map of changed files keyed by the project name.
* @return {Object|true} A map of changed files keyed by the project name or true if all projects should be marked as changed.
*/
export function getFileChanges(
projectGraph: ProjectNode,
baseRef: string
): ProjectFileChanges {
const projectPaths = getProjectPaths( projectGraph );
): ProjectFileChanges | true {
// We're going to use git to figure out what files have changed.
const output = execSync( `git diff --name-only ${ baseRef }`, {
encoding: 'utf8',
} );
const changedFilePaths = output.split( '\n' );
// If the root lockfile has been changed we have no easy way
// of knowing which projects have been impacted. We want
// to re-run all jobs in all projects for safety.
if ( changedFilePaths.includes( 'pnpm-lock.yaml' ) ) {
return true;
}
const projectPaths = getProjectPaths( projectGraph );
const changes: ProjectFileChanges = {};
for ( const projectName in projectPaths ) {
// Projects with no paths have no changed files for us to identify.

131
tools/monorepo-utils/src/ci-jobs/lib/job-processing.ts
View File

@ -73,33 +73,39 @@ function replaceCommandVars( command: string, options: CreateOptions ): string {
/**
* Checks the config against the changes and creates one if it should be run.
*
* @param {string} projectName The name of the project that the job is for.
* @param {Object} config The config object for the lint job.
* @param {Array.<string>} changes The file changes that have occurred for the project.
* @param {Object} options The options to use when creating the job.
* @param {string} projectName The name of the project that the job is for.
* @param {Object} config The config object for the lint job.
* @param {Array.<string>|true} changes The file changes that have occurred for the project or true if all projects should be marked as changed.
* @param {Object} options The options to use when creating the job.
* @return {Object|null} The job that should be run or null if no job should be run.
*/
function createLintJob(
projectName: string,
config: LintJobConfig,
changes: string[],
changes: string[] | true,
options: CreateOptions
): LintJob | null {
let triggered = false;
// Projects can configure jobs to be triggered when a
// changed file matches a path regex.
for ( const file of changes ) {
for ( const change of config.changes ) {
if ( change.test( file ) ) {
triggered = true;
// When we're forcing changes for all projects we don't need to check
// for any changed files before triggering the job.
if ( changes === true ) {
triggered = true;
} else {
// Projects can configure jobs to be triggered when a
// changed file matches a path regex.
for ( const file of changes ) {
for ( const change of config.changes ) {
if ( change.test( file ) ) {
triggered = true;
break;
}
}
if ( triggered ) {
break;
}
}
if ( triggered ) {
break;
}
}
if ( ! triggered ) {
@ -115,47 +121,55 @@ function createLintJob(
/**
* Checks the config against the changes and creates one if it should be run.
*
* @param {string} projectName The name of the project that the job is for.
* @param {Object} config The config object for the test job.
* @param {Array.<string>} changes The file changes that have occurred for the project.
* @param {Object} options The options to use when creating the job.
* @param {Array.<string>} cascadeKeys The cascade keys that have been triggered in dependencies.
* @param {string} projectName The name of the project that the job is for.
* @param {Object} config The config object for the test job.
* @param {Array.<string>|true} changes The file changes that have occurred for the project or true if all projects should be marked as changed.
* @param {Object} options The options to use when creating the job.
* @param {Array.<string>} cascadeKeys The cascade keys that have been triggered in dependencies.
* @return {Promise.<Object|null>} The job that should be run or null if no job should be run.
*/
async function createTestJob(
projectName: string,
config: TestJobConfig,
changes: string[],
changes: string[] | true,
options: CreateOptions,
cascadeKeys: string[]
): Promise< TestJob | null > {
let triggered = false;
// Some jobs can be configured to trigger when a dependency has a job that
// was triggered. For example, a code change in a dependency might mean
// that code is impacted in the current project even if no files were
// actually changed in this project.
if (
config.cascadeKeys &&
config.cascadeKeys.some( ( value ) => cascadeKeys.includes( value ) )
) {
// When we're forcing changes for all projects we don't need to check
// for any changed files before triggering the job.
if ( changes === true ) {
triggered = true;
}
} else {
// Some jobs can be configured to trigger when a dependency has a job that
// was triggered. For example, a code change in a dependency might mean
// that code is impacted in the current project even if no files were
// actually changed in this project.
if (
config.cascadeKeys &&
config.cascadeKeys.some( ( value ) =>
cascadeKeys.includes( value )
)
) {
triggered = true;
}
// Projects can configure jobs to be triggered when a
// changed file matches a path regex.
if ( ! triggered ) {
for ( const file of changes ) {
for ( const change of config.changes ) {
if ( change.test( file ) ) {
triggered = true;
// Projects can configure jobs to be triggered when a
// changed file matches a path regex.
if ( ! triggered ) {
for ( const file of changes ) {
for ( const change of config.changes ) {
if ( change.test( file ) ) {
triggered = true;
break;
}
}
if ( triggered ) {
break;
}
}
if ( triggered ) {
break;
}
}
}
@ -189,15 +203,15 @@ async function createTestJob(
/**
* Recursively checks the project for any jobs that should be executed and returns them.
*
* @param {Object} node The current project node to examine.
* @param {Object} changedFiles The files that have changed for the project.
* @param {Object} options The options to use when creating the job.
* @param {Array.<string>} cascadeKeys The cascade keys that have been triggered in dependencies.
* @param {Object} node The current project node to examine.
* @param {Object|true} changes The changed files keyed by their project or true if all projects should be marked as changed.
* @param {Object} options The options to use when creating the job.
* @param {Array.<string>} cascadeKeys The cascade keys that have been triggered in dependencies.
* @return {Promise.<Object>} The jobs that have been created for the project.
*/
async function createJobsForProject(
node: ProjectNode,
changedFiles: ProjectFileChanges,
changes: ProjectFileChanges | true,
options: CreateOptions,
cascadeKeys: string[]
): Promise< Jobs > {
@ -221,7 +235,7 @@ async function createJobsForProject(
const dependencyJobs = await createJobsForProject(
dependency,
changedFiles,
changes,
options,
dependencyCascade
);
@ -256,12 +270,23 @@ async function createJobsForProject(
continue;
}
// Jobs will check to see whether or not they should trigger based on the files
// that have been changed in the project. When "true" is given, however, it
// means that we should consider ALL files to have been changed and
// trigger any jobs for the project.
let projectChanges;
if ( changes === true ) {
projectChanges = true;
} else {
projectChanges = changes[ node.name ] ?? [];
}
switch ( jobConfig.type ) {
case JobType.Lint: {
const created = createLintJob(
node.name,
jobConfig,
changedFiles[ node.name ] ?? [],
projectChanges,
options
);
if ( ! created ) {
@ -277,7 +302,7 @@ async function createJobsForProject(
const created = await createTestJob(
node.name,
jobConfig,
changedFiles[ node.name ] ?? [],
projectChanges,
options,
cascadeKeys
);
@ -306,14 +331,14 @@ async function createJobsForProject(
/**
* Creates jobs to run for the given project graph and file changes.
*
* @param {Object} root The root node for the project graph.
* @param {Object} changes The file changes that have occurred.
* @param {Object} options The options to use when creating the job.
* @param {Object} root The root node for the project graph.
* @param {Object|true} changes The changed files keyed by their project or true if all projects should be marked as changed.
* @param {Object} options The options to use when creating the job.
* @return {Promise.<Object>} The jobs that should be run.
*/
export function createJobsForChanges(
root: ProjectNode,
changes: ProjectFileChanges,
changes: ProjectFileChanges | true,
options: CreateOptions
): Promise< Jobs > {
return createJobsForProject( root, changes, options, [] );