Skip to content

Commit

Permalink
chore(integtests): make installing NPM7 safe against concurrent calls (
Browse files Browse the repository at this point in the history
…aws#12642)

Do this by memoizing the Promise of the async call; each call gets the
same promise. This is fine because it is safe to await a Promise
multiple times.

Effectively the function will only get invoked once and each caller
waits for the one copy to finish.

We lose "debugging" output here, where it would write the progress
bar of NPM to the output log of the first test to install NPM7. I did
not think maintaining that was worth breaking the clear contract of the
memoize implementation (0 arguments clearly implies that there is no
argument-value based cache here).

(Failure to install will still bubble up as an exception including the shell
output)

----

*By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
  • Loading branch information
rix0rrr authored Jan 21, 2021
1 parent 8fe32b4 commit 01169c1
Show file tree
Hide file tree
Showing 2 changed files with 25 additions and 16 deletions.
27 changes: 11 additions & 16 deletions packages/aws-cdk/test/integ/helpers/cdk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as fs from 'fs';
import * as os from 'os';
import * as path from 'path';
import { outputFromStack, AwsClients } from './aws';
import { memoize0 } from './memoize';
import { findYarnPackages } from './monorepo';
import { ResourcePool } from './resource-pool';
import { TestContext } from './test-helpers';
Expand Down Expand Up @@ -489,7 +490,7 @@ async function installNpmPackages(fixture: TestFixture, packages: Record<string,
}, undefined, 2), { encoding: 'utf-8' });

// Now install that `package.json` using NPM7
const npm7 = await installNpm7(fixture.output);
const npm7 = await installNpm7();
await fixture.shell([npm7, 'install']);
}

Expand All @@ -500,20 +501,14 @@ async function installNpmPackages(fixture: TestFixture, packages: Record<string,
* - The install is cached so we don't have to install it over and over again
* for every test.
*/
async function installNpm7(output?: NodeJS.WritableStream): Promise<string> {
if (NPM7_INSTALL_LOCATION === undefined) {
const installDir = path.join(os.tmpdir(), 'cdk-integ-npm7');
await shell(['rm', '-rf', installDir], { output });
await shell(['mkdir', '-p', installDir], { output });
const installNpm7 = memoize0(async (): Promise<string> => {
const installDir = path.join(os.tmpdir(), 'cdk-integ-npm7');
await shell(['rm', '-rf', installDir]);
await shell(['mkdir', '-p', installDir]);

await shell(['npm', 'install',
'--prefix', installDir,
'npm@7'], { output });
await shell(['npm', 'install',
'--prefix', installDir,
'npm@7']);

NPM7_INSTALL_LOCATION = path.join(installDir, 'node_modules', '.bin', 'npm');
}

return NPM7_INSTALL_LOCATION;
}

let NPM7_INSTALL_LOCATION: string | undefined;
return path.join(installDir, 'node_modules', '.bin', 'npm');
});
14 changes: 14 additions & 0 deletions packages/aws-cdk/test/integ/helpers/memoize.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
/**
* Return a memoized version of an function with 0 arguments.
*
* Async-safe.
*/
export function memoize0<A>(fn: () => Promise<A>): () => Promise<A> {
let promise: Promise<A> | undefined;
return () => {
if (!promise) {
promise = fn();
}
return promise;
};
}

0 comments on commit 01169c1

Please sign in to comment.