Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Add option to for dependency validator to allow list dependencies that can violate consistency check #1920

Merged
merged 20 commits into from
Aug 30, 2024
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
948f909
chore: created param to track inconsistent dependencies
Aug 26, 2024
7d654af
chore: changed knownInconsistentDependencyViolations to record for ea…
Aug 26, 2024
e02a47d
fix: updated dependency validator to be more general in testing for k…
Aug 26, 2024
826327c
chore: updated error message for incorrect verions
Aug 26, 2024
8c4f299
chore: removed unused code
Aug 26, 2024
09a4bdb
Merge branch 'main' into Dependency-validator
Aug 26, 2024
bae9d88
Merge branch 'main' into Dependency-validator
Aug 27, 2024
3a6b0a5
fix: updated tests to include array for new parameter in dependendies…
Aug 27, 2024
a8a1c77
chore: removed unnecessary comment
Aug 27, 2024
366f31c
Merge branch 'main' into Dependency-validator
Aug 27, 2024
666e95b
chore: minor adjustment to test
Aug 28, 2024
f857ca9
Merge branch 'main' into Dependency-validator
Aug 28, 2024
5ff0650
Update scripts/components/dependencies_validator.ts
ShadowCat567 Aug 28, 2024
628e02c
chore: changed variable name, moved find statement
Aug 28, 2024
c301064
chore: added test for inconsistent dependency that has multiple versi…
Aug 29, 2024
62c579e
chore: test added to check multiple dependencies that are marked as i…
Aug 29, 2024
b43473a
chore: added test for known vs unknown inconsistent dependencies
Aug 29, 2024
44a9051
chore: moved execa comment to check_dependencies.ts
Aug 29, 2024
5d5439e
Revert "chore: moved execa comment to check_dependencies.ts"
Aug 29, 2024
9a0f60b
chore: moved execa comment to check_dependencies from dependencies_va…
Aug 29, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion scripts/check_dependencies.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,17 @@ await new DependenciesValidator(
],
},
},
[['aws-cdk', 'aws-cdk-lib']]
[['aws-cdk', 'aws-cdk-lib']],
[
{
dependencyName: 'execa',
globalDependencyVersion: '^8.0.1',
exceptions: [
{
packageName: '@aws-amplify/plugin-types',
dependencyVersion: '^5.1.1',
},
],
},
]
).validate();
7 changes: 7 additions & 0 deletions scripts/components/dependencies_validator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ void describe('Dependency validator', () => {
},
},
[],
[],
execaMock as never
).validate(),
(err: Error) => {
Expand All @@ -65,6 +66,7 @@ void describe('Dependency validator', () => {
},
},
[],
[],
execaMock as never
).validate(),
(err: Error) => {
Expand All @@ -88,6 +90,7 @@ void describe('Dependency validator', () => {
},
},
[],
[],
execaMock as never
).validate();
});
Expand All @@ -106,6 +109,7 @@ void describe('Dependency validator', () => {
},
},
[],
[],
execaMock as never
).validate(),
(err: Error) => {
Expand All @@ -129,6 +133,7 @@ void describe('Dependency validator', () => {
packagePaths,
{},
[],
[],
execaMock as never
).validate();
},
Expand Down Expand Up @@ -156,6 +161,7 @@ void describe('Dependency validator', () => {
packagePaths,
{},
[],
[],
execaMock as never
);

Expand Down Expand Up @@ -185,6 +191,7 @@ void describe('Dependency validator', () => {
packagePaths,
{},
[['aws-cdk', 'aws-cdk-lib']],
[],
execaMock as never
).validate();
},
Expand Down
40 changes: 31 additions & 9 deletions scripts/components/dependencies_validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,12 @@ export type DependencyRule =
allowList: Array<string>;
};

export type DependencyWithKnownException = {
dependencyName: string;
globalDependencyVersion: string;
exceptions: Array<{ packageName: string; dependencyVersion: string }>;
};

type NpmListOutputItem = {
name?: string;
dependencies?: Record<string, NpmListOutputItem>;
Expand Down Expand Up @@ -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 knownInconsistentDependencies dependencies that are known to violate the consistency check
* @param execa in order to inject execa mock in tests
*/
constructor(
private packagePaths: Array<string>,
private dependencyRules: Record<string, DependencyRule>,
private disallowedDependencies: Record<string, DependencyRule>,
private linkedDependencies: Array<Array<string>>,
private knownInconsistentDependencies: Array<DependencyWithKnownException>,
ShadowCat567 marked this conversation as resolved.
Show resolved Hide resolved
private execa = _execa
) {}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -243,20 +251,34 @@ export class DependenciesValidator {
private getPackageVersionDeclarationPredicate = async (
packageName: string
): Promise<DependencyVersionPredicate> => {
if (packageName === 'execa') {
if (
this.knownInconsistentDependencies.find(
(a) => a.dependencyName === packageName
) !== undefined
ShadowCat567 marked this conversation as resolved.
Show resolved Hide resolved
) {
// @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.knownInconsistentDependencies.find(
(x) => x.dependencyName === packageName
);
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)) {
Expand Down