Skip to content

Commit

Permalink
fix: multiple breakages due to jest version upgrade (#7667)
Browse files Browse the repository at this point in the history 
Couple of things here:

**require.cache**

In version `v25.5.0`, jest introduced an [implementation](jestjs/jest#9841) of their own to the `require.cache` object. It seems that it doesn't handle caching `json` modules properly, which causes our code to fail because `mod.paths` is `undefined` when we are querying for `json` files that were required (for example `package.json` or `cloud-assembly.schema.json`).

https://github.com/aws/aws-cdk/blob/07fe642e38118e24837b492fa182737fc41bb429/packages/%40aws-cdk/core/lib/private/runtime-info.ts#L66

```console
TypeError: Cannot read property 'map' of undefined
```

The "fix" was to add a null check to prevent failure when looking up modules who don't have the `paths` property defined.

Note that this was only observed in test environments using `jest > v25.5.0`, not during actual runtime of `cdk synth`. This is because `jest` manipulates the built-in `require` module of `nodejs`.

**graceful-fs**

In version `v25.5.0`, jest [added](jestjs/jest#9443) a dependency on the `graceful-fs` package. The version they depend on (`4.2.4`) differs from the version that we bring (`4.2.3`). This caused the `graceful-fs` dependency that comes from `jest` no to be deduped by node at installation. In turn, this caused multiple copies of the library to be installed on disk.

The `graceful-fs` package monkey patches the `process.chdir` and `process.cwd` functions with a cached version for better performance.

For reasons not completely clear to us, the existence of multiple copies of the module, caused `process.cwd()` to return incorrect cached results, essentially always returning the directory that the process was started from, without consideration to calls to `process.chdir`. This broke `decdk` (and possibly many more) tests which rely on `process.chdir`.

Fixes #7657
  • Loading branch information
@iliapolo
iliapolo committed Apr 29, 2020
1 parent dea10e8 commit 6eaff91
Show file tree
Hide file tree
Showing 42 changed files with 560 additions and 1,046 deletions.
1 change: 1 addition & 0 deletions package.json
View file
Expand Up @@ -21,6 +21,7 @@
"jsii-rosetta": "^1.4.1",
"lerna": "^3.20.2",
"standard-version": "^7.1.0",
"graceful-fs": "^4.2.4",
"typescript": "~3.8.3"
},
"repository": {
Expand Down
4 changes: 2 additions & 2 deletions packages/@aws-cdk/assert/package.json
View file
Expand Up @@ -31,7 +31,7 @@
"devDependencies": {
"@types/jest": "^25.2.1",
"cdk-build-tools": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0",
"ts-jest": "^25.4.0"
},
Expand All @@ -44,7 +44,7 @@
},
"peerDependencies": {
"@aws-cdk/core": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"constructs": "^3.0.2"
},
"repository": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-autoscaling-hooktargets/package.json
View file
Expand Up @@ -81,7 +81,7 @@
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"cfn2ts": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-batch/package.json
View file
Expand Up @@ -83,7 +83,7 @@
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"cfn2ts": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion ...cdk/aws-certificatemanager/lambda-packages/dns_validated_certificate_handler/package.json
View file
Expand Up @@ -35,7 +35,7 @@
"eslint-plugin-node": "^10.0.0",
"eslint-plugin-promise": "^4.2.1",
"eslint-plugin-standard": "^4.0.1",
"jest": "^25.3.0",
"jest": "^25.5.0",
"lambda-tester": "^3.6.0",
"nock": "^11.7.0",
"ts-jest": "^25.3.1"
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-cloudwatch-actions/package.json
View file
Expand Up @@ -81,7 +81,7 @@
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"cfn2ts": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-cognito/package.json
View file
Expand Up @@ -67,7 +67,7 @@
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"cfn2ts": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"nodeunit": "^0.11.3",
"pkglint": "0.0.0"
},
Expand Down
2 changes: 1 addition & 1 deletion ...es/@aws-cdk/aws-dynamodb-global/lambda-packages/aws-global-table-coordinator/package.json
View file
Expand Up @@ -35,7 +35,7 @@
"eslint-plugin-node": "^10.0.0",
"eslint-plugin-promise": "^4.2.1",
"eslint-plugin-standard": "^4.0.1",
"jest": "^25.3.0",
"jest": "^25.5.0",
"lambda-tester": "^3.6.0",
"nock": "^11.7.0"
}
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-dynamodb/package.json
View file
Expand Up @@ -69,7 +69,7 @@
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"cfn2ts": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0",
"sinon": "^9.0.2",
"ts-jest": "^25.4.0"
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-ecs-patterns/package.json
View file
Expand Up @@ -65,7 +65,7 @@
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"cfn2ts": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"nodeunit": "^0.11.3",
"pkglint": "0.0.0"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-elasticloadbalancingv2-targets/package.json
View file
Expand Up @@ -80,7 +80,7 @@
"@aws-cdk/assert": "0.0.0",
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-events-targets/package.json
View file
Expand Up @@ -90,7 +90,7 @@
"aws-sdk-mock": "^5.1.0",
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-iam/package.json
View file
Expand Up @@ -68,7 +68,7 @@
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"cfn2ts": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-lambda-destinations/package.json
View file
Expand Up @@ -80,7 +80,7 @@
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"cfn2ts": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-logs-destinations/package.json
View file
Expand Up @@ -80,7 +80,7 @@
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"cfn2ts": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-route53-patterns/package.json
View file
Expand Up @@ -80,7 +80,7 @@
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"cfn2ts": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-route53-targets/package.json
View file
Expand Up @@ -82,7 +82,7 @@
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"cfn2ts": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-s3-notifications/package.json
View file
Expand Up @@ -78,7 +78,7 @@
"@aws-cdk/assert": "0.0.0",
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-sam/package.json
View file
Expand Up @@ -67,7 +67,7 @@
"@types/jest": "^25.2.1",
"cdk-build-tools": "0.0.0",
"cfn2ts": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0",
"ts-jest": "^25.4.0"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-ses-actions/package.json
View file
Expand Up @@ -81,7 +81,7 @@
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"cfn2ts": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-sns-subscriptions/package.json
View file
Expand Up @@ -80,7 +80,7 @@
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"cfn2ts": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/aws-stepfunctions-tasks/package.json
View file
Expand Up @@ -83,7 +83,7 @@
"@aws-cdk/aws-s3-assets": "0.0.0",
"cdk-build-tools": "0.0.0",
"cdk-integ-tools": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cdk-assets-schema/package.json
View file
Expand Up @@ -68,7 +68,7 @@
"devDependencies": {
"@types/jest": "^25.2.1",
"cdk-build-tools": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0"
},
"repository": {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cloud-assembly-schema/package.json
View file
Expand Up @@ -58,7 +58,7 @@
"@types/jest": "^25.2.1",
"@types/mock-fs": "^4.10.0",
"cdk-build-tools": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"mock-fs": "^4.12.0",
"pkglint": "0.0.0",
"typescript-json-schema": "^0.42.0"
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cloudformation-diff/package.json
View file
Expand Up @@ -43,7 +43,7 @@
"@types/table": "^4.0.7",
"cdk-build-tools": "0.0.0",
"fast-check": "^1.24.2",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0",
"ts-jest": "^25.4.0"
},
Expand Down
9 changes: 9 additions & 0 deletions packages/@aws-cdk/core/lib/private/runtime-info.ts
View file
Expand Up @@ -63,6 +63,15 @@ export function collectRuntimeInformation(): cxschema.RuntimeInfo {
*/
function findNpmPackage(fileName: string): { name: string, version: string, private?: boolean } | undefined {
const mod = require.cache[fileName];

if (!mod.paths) {
// sometimes this can be undefined. for example when querying for .json modules
// inside a jest runtime environment.
// see https://github.com/aws/aws-cdk/issues/7657
// potentially we can remove this if it turns out to be a bug in how jest implemented the 'require' module.
return undefined;
}

const paths = mod.paths.map(stripNodeModules);

try {
Expand Down
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cx-api/package.json
View file
Expand Up @@ -74,7 +74,7 @@
"@types/mock-fs": "^4.10.0",
"@types/semver": "^7.1.0",
"cdk-build-tools": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"mock-fs": "^4.12.0",
"pkglint": "0.0.0"
},
Expand Down
4 changes: 2 additions & 2 deletions packages/@monocdk-experiment/assert/package.json
View file
Expand Up @@ -45,7 +45,7 @@
"devDependencies": {
"@types/jest": "^25.2.1",
"cdk-build-tools": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0",
"ts-jest": "^25.4.0",
"@monocdk-experiment/rewrite-imports": "0.0.0",
Expand All @@ -56,7 +56,7 @@
"@aws-cdk/cloudformation-diff": "0.0.0"
},
"peerDependencies": {
"jest": "^25.4.0",
"jest": "^25.5.0",
"monocdk-experiment": "^0.0.0",
"constructs": "^3.0.2"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/init-templates/app/javascript/package.template.json
View file
Expand Up @@ -12,7 +12,7 @@
"devDependencies": {
"@aws-cdk/assert": "%cdk-version%",
"aws-cdk": "%cdk-version%",
"jest": "^25.3.0"
"jest": "^25.5.0"
},
"dependencies": {
"@aws-cdk/core": "%cdk-version%"
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/init-templates/app/typescript/package.template.json
View file
Expand Up @@ -14,7 +14,7 @@
"@aws-cdk/assert": "%cdk-version%",
"@types/jest": "^25.2.1",
"@types/node": "10.17.5",
"jest": "^25.3.0",
"jest": "^25.5.0",
"ts-jest": "^25.3.1",
"aws-cdk": "%cdk-version%",
"ts-node": "^8.1.0",
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/init-templates/lib/typescript/package.template.json
View file
Expand Up @@ -12,7 +12,7 @@
"@aws-cdk/assert": "%cdk-version%",
"@types/jest": "^25.2.1",
"@types/node": "10.17.5",
"jest": "^25.3.0",
"jest": "^25.5.0",
"ts-jest": "^25.3.1",
"typescript": "~3.7.2"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/init-templates/sample-app/javascript/package.template.json
View file
Expand Up @@ -12,7 +12,7 @@
"devDependencies": {
"@aws-cdk/assert": "%cdk-version%",
"aws-cdk": "%cdk-version%",
"jest": "^25.3.0"
"jest": "^25.5.0"
},
"dependencies": {
"@aws-cdk/aws-sns": "%cdk-version%",
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/lib/init-templates/sample-app/typescript/package.template.json
View file
Expand Up @@ -15,7 +15,7 @@
"@aws-cdk/assert": "%cdk-version%",
"@types/jest": "^25.2.1",
"@types/node": "10.17.5",
"jest": "^25.3.0",
"jest": "^25.5.0",
"ts-jest": "^25.3.1",
"ts-node": "^8.1.0",
"typescript": "~3.7.2"
Expand Down
2 changes: 1 addition & 1 deletion packages/aws-cdk/package.json
View file
Expand Up @@ -54,7 +54,7 @@
"@types/yargs": "^15.0.4",
"aws-sdk-mock": "^5.1.0",
"cdk-build-tools": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"mockery": "^2.1.0",
"pkglint": "0.0.0",
"sinon": "^9.0.2",
Expand Down
2 changes: 1 addition & 1 deletion packages/cdk-assets/package.json
View file
Expand Up @@ -37,7 +37,7 @@
"@types/jszip": "^3.1.7",
"jszip": "^3.4.0",
"cdk-build-tools": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"mock-fs": "^4.12.0",
"pkglint": "0.0.0"
},
Expand Down
2 changes: 1 addition & 1 deletion packages/cdk-dasm/package.json
View file
Expand Up @@ -32,7 +32,7 @@
"devDependencies": {
"@types/jest": "^25.2.1",
"@types/yaml": "1.9.7",
"jest": "^25.4.0"
"jest": "^25.5.0"
},
"keywords": [
"aws",
Expand Down
2 changes: 1 addition & 1 deletion packages/decdk/package.json
View file
Expand Up @@ -183,7 +183,7 @@
"@types/jest": "^25.2.1",
"@types/yaml": "1.9.7",
"@types/yargs": "^15.0.4",
"jest": "^25.4.0",
"jest": "^25.5.0",
"jsii": "^1.4.1"
},
"keywords": [
Expand Down
22 changes: 22 additions & 0 deletions packages/decdk/test/sanity.test.ts
View file
@@ -0,0 +1,22 @@
import * as path from 'path';

test('path.resolve is sane', async () => {
// Reasons why this might not be true:
// graceful-fs, which is used by Jest, hooks into process.cwd() and
// process.chdir() and caches the values. Because... profit?

const targetDir = path.join(__dirname, 'fixture');

const cwd = process.cwd();

try {
process.chdir(targetDir);
expect(process.cwd()).toEqual(targetDir);

const resolved = path.resolve('.');
expect(resolved).toEqual(targetDir);

} finally {
process.chdir(cwd);
}
});
2 changes: 1 addition & 1 deletion tools/cdk-build-tools/package.json
View file
Expand Up @@ -48,7 +48,7 @@
"eslint-import-resolver-typescript": "^2.0.0",
"eslint-plugin-import": "^2.20.2",
"fs-extra": "^8.1.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"jsii": "^1.4.1",
"jsii-pacmak": "^1.4.1",
"nodeunit": "^0.11.3",
Expand Down
2 changes: 1 addition & 1 deletion tools/cfn2ts/package.json
View file
Expand Up @@ -40,7 +40,7 @@
"@types/jest": "^25.2.1",
"@types/yargs": "^15.0.4",
"cdk-build-tools": "0.0.0",
"jest": "^25.4.0",
"jest": "^25.5.0",
"pkglint": "0.0.0"
},
"cdk-build": {
Expand Down

0 comments on commit 6eaff91

Please sign in to comment.