From 5f4870060c88401c69908f0ba7d76ec0745d02fc Mon Sep 17 00:00:00 2001 From: Leena <81589006+ShadowCat567@users.noreply.github.com> Date: Fri, 30 Aug 2024 15:25:21 -0400 Subject: [PATCH] feat: Add option to for dependency validator to allow list dependencies that can violate consistency check (#1920) * chore: created param to track inconsistent dependencies * chore: changed knownInconsistentDependencyViolations to record for easier lookup * fix: updated dependency validator to be more general in testing for known inconsistent dependencies * chore: updated error message for incorrect verions * chore: removed unused code * fix: updated tests to include array for new parameter in dependendies validator * chore: removed unnecessary comment * chore: minor adjustment to test * Update scripts/components/dependencies_validator.ts Co-authored-by: Kamil Sobol * chore: changed variable name, moved find statement * chore: added test for inconsistent dependency that has multiple versions in use * chore: test added to check multiple dependencies that are marked as inconsistent yield correct results * chore: added test for known vs unknown inconsistent dependencies * chore: moved execa comment to check_dependencies.ts * Revert "chore: moved execa comment to check_dependencies.ts" This reverts commit 44a9051c305c2ecafde0366b1f65d7807b11add3. * chore: moved execa comment to check_dependencies from dependencies_validator --------- Co-authored-by: Vieltojarvi Co-authored-by: Kamil Sobol --- scripts/check_dependencies.ts | 17 ++- .../components/dependencies_validator.test.ts | 109 ++++++++++++++++++ scripts/components/dependencies_validator.ts | 40 +++++-- .../package1/package.json | 9 ++ .../package2/package.json | 9 ++ .../package3/package.json | 9 ++ .../package1/package.json | 12 ++ .../package2/package.json | 12 ++ .../package3/package.json | 12 ++ 9 files changed, 216 insertions(+), 13 deletions(-) create mode 100644 scripts/components/test-resources/dependency-version-inconsistent-test-packages/package1/package.json create mode 100644 scripts/components/test-resources/dependency-version-inconsistent-test-packages/package2/package.json create mode 100644 scripts/components/test-resources/dependency-version-inconsistent-test-packages/package3/package.json create mode 100644 scripts/components/test-resources/dependency-version-multiple-inconsistencies-test-packages/package1/package.json create mode 100644 scripts/components/test-resources/dependency-version-multiple-inconsistencies-test-packages/package2/package.json create mode 100644 scripts/components/test-resources/dependency-version-multiple-inconsistencies-test-packages/package3/package.json diff --git a/scripts/check_dependencies.ts b/scripts/check_dependencies.ts index 53062b8f28..80d26889d4 100644 --- a/scripts/check_dependencies.ts +++ b/scripts/check_dependencies.ts @@ -21,5 +21,20 @@ await new DependenciesValidator( ], }, }, - [['aws-cdk', 'aws-cdk-lib']] + [['aws-cdk', 'aws-cdk-lib']], + [ + { + // @aws-amplify/plugin-types can depend on execa@^5.1.1 as a workaround for https://github.com/aws-amplify/amplify-backend/issues/962 + // all other packages must depend on execa@^8.0.1 + // this can be removed once execa is patched + dependencyName: 'execa', + globalDependencyVersion: '^8.0.1', + exceptions: [ + { + packageName: '@aws-amplify/plugin-types', + dependencyVersion: '^5.1.1', + }, + ], + }, + ] ).validate(); diff --git a/scripts/components/dependencies_validator.test.ts b/scripts/components/dependencies_validator.test.ts index fe93f5488d..9f2682dbb2 100644 --- a/scripts/components/dependencies_validator.test.ts +++ b/scripts/components/dependencies_validator.test.ts @@ -41,6 +41,7 @@ void describe('Dependency validator', () => { }, }, [], + [], execaMock as never ).validate(), (err: Error) => { @@ -65,6 +66,7 @@ void describe('Dependency validator', () => { }, }, [], + [], execaMock as never ).validate(), (err: Error) => { @@ -88,6 +90,7 @@ void describe('Dependency validator', () => { }, }, [], + [], execaMock as never ).validate(); }); @@ -106,6 +109,7 @@ void describe('Dependency validator', () => { }, }, [], + [], execaMock as never ).validate(), (err: Error) => { @@ -129,6 +133,7 @@ void describe('Dependency validator', () => { packagePaths, {}, [], + [], execaMock as never ).validate(); }, @@ -148,6 +153,108 @@ void describe('Dependency validator', () => { ); }); + void it('passes if dependency declaration that is known to be inconsistent uses multiple versions', async () => { + const packagePaths = await glob( + 'scripts/components/test-resources/dependency-version-inconsistent-test-packages/*' + ); + await new DependenciesValidator( + packagePaths, + {}, + [], + [ + { + dependencyName: 'glob', + globalDependencyVersion: '^7.2.0', + exceptions: [ + { + packageName: 'package2', + dependencyVersion: '^3.4.0', + }, + { + packageName: 'package3', + dependencyVersion: '^1.6.0', + }, + ], + }, + ], + execaMock as never + ).validate(); + }); + + void it('passes if multiple dependency declarations are known to be inconsistent', async () => { + const packagePaths = await glob( + 'scripts/components/test-resources/dependency-version-multiple-inconsistencies-test-packages/*' + ); + await new DependenciesValidator( + packagePaths, + {}, + [], + [ + { + dependencyName: 'glob', + globalDependencyVersion: '^7.2.0', + exceptions: [ + { + packageName: 'package3', + dependencyVersion: '^5.3.0', + }, + ], + }, + { + dependencyName: 'zod', + globalDependencyVersion: '^3.8.2-alpha.6', + exceptions: [ + { + packageName: 'package2', + dependencyVersion: '^2.0.0', + }, + ], + }, + ], + execaMock as never + ).validate(); + }); + + void it('can detect unknown inconsistent dependency delcarations when known inconsistent dependency delcarations are present', async () => { + await assert.rejects( + async () => { + const packagePaths = await glob( + 'scripts/components/test-resources/dependency-version-multiple-inconsistencies-test-packages/*' + ); + await new DependenciesValidator( + packagePaths, + {}, + [], + [ + { + dependencyName: 'glob', + globalDependencyVersion: '^7.2.0', + exceptions: [ + { + packageName: 'package3', + dependencyVersion: '^5.3.0', + }, + ], + }, + ], + execaMock as never + ).validate(); + }, + (err: Error) => { + assert.ok( + err.message.includes( + 'dependency declarations must all the on the same semver range' + ) + ); + assert.ok(err.message.includes('zod')); + assert.ok(err.message.includes('package1')); + assert.ok(err.message.includes('package2')); + assert.ok(err.message.includes('package3')); + return true; + } + ); + }); + void it('can detect inconsistent major versions of repo packages', async () => { const packagePaths = await glob( 'scripts/components/test-resources/inter-repo-dependency-version-consistency-test-packages/*' @@ -156,6 +263,7 @@ void describe('Dependency validator', () => { packagePaths, {}, [], + [], execaMock as never ); @@ -185,6 +293,7 @@ void describe('Dependency validator', () => { packagePaths, {}, [['aws-cdk', 'aws-cdk-lib']], + [], execaMock as never ).validate(); }, diff --git a/scripts/components/dependencies_validator.ts b/scripts/components/dependencies_validator.ts index 980bfdde80..caa9e50bbc 100644 --- a/scripts/components/dependencies_validator.ts +++ b/scripts/components/dependencies_validator.ts @@ -12,6 +12,12 @@ export type DependencyRule = allowList: Array; }; +export type DependencyWithKnownVersionConsistencyException = { + dependencyName: string; + globalDependencyVersion: string; + exceptions: Array<{ packageName: string; dependencyVersion: string }>; +}; + type NpmListOutputItem = { name?: string; dependencies?: Record; @@ -47,14 +53,16 @@ export class DependenciesValidator { /** * Creates dependency validator * @param packagePaths paths of packages to validate - * @param dependencyRules dependency exclusion and inclusion rules + * @param disallowedDependencies dependency exclusion and inclusion rules * @param linkedDependencies dependencies that should be versioned with the same version + * @param knownInconsistentDependencyVersions dependencies that are known to violate the consistency check * @param execa in order to inject execa mock in tests */ constructor( private packagePaths: Array, - private dependencyRules: Record, + private disallowedDependencies: Record, private linkedDependencies: Array>, + private knownInconsistentDependencyVersions: Array, private execa = _execa ) {} @@ -202,9 +210,9 @@ export class DependenciesValidator { // skip if self referencing continue; } - if (dependencyName in this.dependencyRules) { + if (dependencyName in this.disallowedDependencies) { const dependencyRule: DependencyRule = - this.dependencyRules[dependencyName]; + this.disallowedDependencies[dependencyName]; const isViolating = dependencyRule.denyAll || !dependencyRule.allowList.includes(packageName); @@ -243,20 +251,28 @@ export class DependenciesValidator { private getPackageVersionDeclarationPredicate = async ( packageName: string ): Promise => { - if (packageName === 'execa') { - // @aws-amplify/plugin-types can depend on execa@^5.1.1 as a workaround for https://github.com/aws-amplify/amplify-backend/issues/962 - // all other packages must depend on execa@^8.0.1 - // this can be removed once execa is patched + const inconsistentDependency = + this.knownInconsistentDependencyVersions.find( + (x) => x.dependencyName === packageName + ); + if (inconsistentDependency) { return (declarations) => { const validationResult = declarations.every( ({ dependentPackageName, version }) => - (dependentPackageName === '@aws-amplify/plugin-types' && - version === '^5.1.1') || - version === '^8.0.1' + inconsistentDependency!.exceptions.find( + (a) => a.packageName === dependentPackageName + )?.dependencyVersion || + version === inconsistentDependency!.globalDependencyVersion ); return ( validationResult || - `${packageName} dependency declarations must depend on version ^8.0.1 except in @aws-amplify/plugin-types where it must depend on ^5.1.1.` + `${packageName} dependency declarations must depend on version ${ + inconsistentDependency!.globalDependencyVersion + } except in the following packages` + + inconsistentDependency!.exceptions.forEach( + (exception) => + `, ${exception.packageName} where it must depend on ${exception.dependencyVersion}` + ) ); }; } else if ((await this.getRepoPackageNames()).includes(packageName)) { diff --git a/scripts/components/test-resources/dependency-version-inconsistent-test-packages/package1/package.json b/scripts/components/test-resources/dependency-version-inconsistent-test-packages/package1/package.json new file mode 100644 index 0000000000..04c57c8144 --- /dev/null +++ b/scripts/components/test-resources/dependency-version-inconsistent-test-packages/package1/package.json @@ -0,0 +1,9 @@ +{ + "name": "package1", + "dependencies": { + "glob": "^7.2.0" + }, + "devDependencies": { + "zod": "^3.8.2-alpha.6" + } +} diff --git a/scripts/components/test-resources/dependency-version-inconsistent-test-packages/package2/package.json b/scripts/components/test-resources/dependency-version-inconsistent-test-packages/package2/package.json new file mode 100644 index 0000000000..b8c74c3e16 --- /dev/null +++ b/scripts/components/test-resources/dependency-version-inconsistent-test-packages/package2/package.json @@ -0,0 +1,9 @@ +{ + "name": "package2", + "dependencies": { + "glob": "^3.4.0" + }, + "devDependencies": { + "zod": "^3.8.2-alpha.6" + } +} diff --git a/scripts/components/test-resources/dependency-version-inconsistent-test-packages/package3/package.json b/scripts/components/test-resources/dependency-version-inconsistent-test-packages/package3/package.json new file mode 100644 index 0000000000..c20dc3d34d --- /dev/null +++ b/scripts/components/test-resources/dependency-version-inconsistent-test-packages/package3/package.json @@ -0,0 +1,9 @@ +{ + "name": "package2", + "dependencies": { + "glob": "^1.6.0" + }, + "devDependencies": { + "zod": "^3.8.2-alpha.6" + } +} diff --git a/scripts/components/test-resources/dependency-version-multiple-inconsistencies-test-packages/package1/package.json b/scripts/components/test-resources/dependency-version-multiple-inconsistencies-test-packages/package1/package.json new file mode 100644 index 0000000000..64ec0f74ce --- /dev/null +++ b/scripts/components/test-resources/dependency-version-multiple-inconsistencies-test-packages/package1/package.json @@ -0,0 +1,12 @@ +{ + "name": "package1", + "dependencies": { + "glob": "^7.2.0" + }, + "devDependencies": { + "zod": "^3.8.2-alpha.6" + }, + "peerDependencies": { + "yargs": "~14.2.3" + } +} diff --git a/scripts/components/test-resources/dependency-version-multiple-inconsistencies-test-packages/package2/package.json b/scripts/components/test-resources/dependency-version-multiple-inconsistencies-test-packages/package2/package.json new file mode 100644 index 0000000000..0d7f2c8dcc --- /dev/null +++ b/scripts/components/test-resources/dependency-version-multiple-inconsistencies-test-packages/package2/package.json @@ -0,0 +1,12 @@ +{ + "name": "package2", + "dependencies": { + "glob": "^7.2.0" + }, + "devDependencies": { + "zod": "^2.0.0" + }, + "peerDependencies": { + "yargs": "~14.2.3" + } +} diff --git a/scripts/components/test-resources/dependency-version-multiple-inconsistencies-test-packages/package3/package.json b/scripts/components/test-resources/dependency-version-multiple-inconsistencies-test-packages/package3/package.json new file mode 100644 index 0000000000..315990eda0 --- /dev/null +++ b/scripts/components/test-resources/dependency-version-multiple-inconsistencies-test-packages/package3/package.json @@ -0,0 +1,12 @@ +{ + "name": "package3", + "dependencies": { + "glob": "^5.3.0" + }, + "devDependencies": { + "zod": "^3.8.2-alpha.6" + }, + "peerDependencies": { + "yargs": "~14.2.3" + } +}