Skip to content

Commit ec79ab1

Browse files
alan-agius4alan-agius4
committed
test(@angular/cli): improve environment variables clean up during E2E's
(cherry picked from commit 99bd478)
1 parent eaa2378 commit ec79ab1

File tree

6 files changed

+61
-101
lines changed

6 files changed

+61
-101
lines changed

‎tests/legacy-cli/e2e/tests/commands/add/npm-env-vars.ts

Lines changed: 14 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,33 +1,26 @@
11
import { expectFileNotToExist, expectFileToExist } from '../../../utils/fs';
2-
import { getActivePackageManager } from '../../../utils/packages';
32
import { git, ng } from '../../../utils/process';
43
import {
54
createNpmConfigForAuthentication,
65
setNpmEnvVarsForAuthentication,
76
} from '../../../utils/registry';
87

98
export default async function () {
10-
const packageManager = getActivePackageManager();
9+
// Yarn also supports NPM_CONFIG env variables.
10+
// https://classic.yarnpkg.com/en/docs/envvars/
1111

12-
if (packageManager === 'npm') {
13-
const originalEnvironment = { ...process.env };
14-
try {
15-
const command = ['add', '@angular/pwa', '--skip-confirmation'];
12+
const command = ['add', '@angular/pwa', '--skip-confirmation'];
1613

17-
// Environment variables only
18-
await expectFileNotToExist('src/manifest.webmanifest');
19-
setNpmEnvVarsForAuthentication();
20-
await ng(...command);
21-
await expectFileToExist('src/manifest.webmanifest');
22-
await git('clean', '-dxf');
14+
// Environment variables only
15+
await expectFileNotToExist('src/manifest.webmanifest');
16+
setNpmEnvVarsForAuthentication();
17+
await ng(...command);
18+
await expectFileToExist('src/manifest.webmanifest');
19+
await git('clean', '-dxf');
2320

24-
// Mix of config file and env vars works
25-
await expectFileNotToExist('src/manifest.webmanifest');
26-
await createNpmConfigForAuthentication(false, true);
27-
await ng(...command);
28-
await expectFileToExist('src/manifest.webmanifest');
29-
} finally {
30-
process.env = originalEnvironment;
31-
}
32-
}
21+
// Mix of config file and env vars works
22+
await expectFileNotToExist('src/manifest.webmanifest');
23+
await createNpmConfigForAuthentication(false, true);
24+
await ng(...command);
25+
await expectFileToExist('src/manifest.webmanifest');
3326
}

‎tests/legacy-cli/e2e/tests/commands/add/registry-option.ts

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -10,21 +10,12 @@ export default async function () {
1010
await writeMultipleFiles({
1111
'.npmrc': 'registry=http://127.0.0.1:9999',
1212
});
13+
1314
// The environment variable has priority over the .npmrc
14-
let originalRegistryVariable;
15-
if (process.env['NPM_CONFIG_REGISTRY']) {
16-
originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY'];
17-
delete process.env['NPM_CONFIG_REGISTRY'];
18-
}
15+
delete process.env['NPM_CONFIG_REGISTRY'];
1916

20-
try {
21-
await expectToFail(() => ng('add', '@angular/pwa', '--skip-confirmation'));
17+
await expectToFail(() => ng('add', '@angular/pwa', '--skip-confirmation'));
2218

23-
await ng('add', `--registry=${testRegistry}`, '@angular/pwa', '--skip-confirmation');
24-
await expectFileToExist('src/manifest.webmanifest');
25-
} finally {
26-
if (originalRegistryVariable) {
27-
process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable;
28-
}
29-
}
19+
await ng('add', `--registry=${testRegistry}`, '@angular/pwa', '--skip-confirmation');
20+
await expectFileToExist('src/manifest.webmanifest');
3021
}

‎tests/legacy-cli/e2e/tests/commands/add/secure-registry.ts

Lines changed: 18 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -5,38 +5,28 @@ import { expectToFail } from '../../../utils/utils';
55

66
export default async function () {
77
// The environment variable has priority over the .npmrc
8-
let originalRegistryVariable;
9-
if (process.env['NPM_CONFIG_REGISTRY']) {
10-
originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY'];
11-
delete process.env['NPM_CONFIG_REGISTRY'];
12-
}
8+
delete process.env['NPM_CONFIG_REGISTRY'];
139

14-
try {
15-
const command = ['add', '@angular/pwa', '--skip-confirmation'];
16-
await expectFileNotToExist('src/manifest.webmanifest');
10+
const command = ['add', '@angular/pwa', '--skip-confirmation'];
11+
await expectFileNotToExist('src/manifest.webmanifest');
1712

18-
// Works with unscoped registry authentication details
19-
await createNpmConfigForAuthentication(false);
20-
await ng(...command);
21-
await expectFileToExist('src/manifest.webmanifest');
22-
await git('clean', '-dxf');
13+
// Works with unscoped registry authentication details
14+
await createNpmConfigForAuthentication(false);
15+
await ng(...command);
16+
await expectFileToExist('src/manifest.webmanifest');
17+
await git('clean', '-dxf');
2318

24-
// Works with scoped registry authentication details
25-
await expectFileNotToExist('src/manifest.webmanifest');
19+
// Works with scoped registry authentication details
20+
await expectFileNotToExist('src/manifest.webmanifest');
2621

27-
await createNpmConfigForAuthentication(true);
28-
await ng(...command);
29-
await expectFileToExist('src/manifest.webmanifest');
22+
await createNpmConfigForAuthentication(true);
23+
await ng(...command);
24+
await expectFileToExist('src/manifest.webmanifest');
3025

31-
// Invalid authentication token
32-
await createNpmConfigForAuthentication(false, true);
33-
await expectToFail(() => ng(...command));
26+
// Invalid authentication token
27+
await createNpmConfigForAuthentication(false, true);
28+
await expectToFail(() => ng(...command));
3429

35-
await createNpmConfigForAuthentication(true, true);
36-
await expectToFail(() => ng(...command));
37-
} finally {
38-
if (originalRegistryVariable) {
39-
process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable;
40-
}
41-
}
30+
await createNpmConfigForAuthentication(true, true);
31+
await expectToFail(() => ng(...command));
4232
}

‎tests/legacy-cli/e2e/tests/update/update-secure-registry.ts

Lines changed: 17 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -4,37 +4,26 @@ import { expectToFail } from '../../utils/utils';
44

55
export default async function () {
66
// The environment variable has priority over the .npmrc
7-
let originalRegistryVariable;
8-
if (process.env['NPM_CONFIG_REGISTRY']) {
9-
originalRegistryVariable = process.env['NPM_CONFIG_REGISTRY'];
10-
delete process.env['NPM_CONFIG_REGISTRY'];
11-
}
12-
7+
delete process.env['NPM_CONFIG_REGISTRY'];
138
const worksMessage = 'We analyzed your package.json';
149

15-
try {
16-
// Valid authentication token
17-
await createNpmConfigForAuthentication(false);
18-
const { stdout: stdout1 } = await ng('update');
19-
if (!stdout1.includes(worksMessage)) {
20-
throw new Error(`Expected stdout to contain "${worksMessage}"`);
21-
}
10+
// Valid authentication token
11+
await createNpmConfigForAuthentication(false);
12+
const { stdout: stdout1 } = await ng('update');
13+
if (!stdout1.includes(worksMessage)) {
14+
throw new Error(`Expected stdout to contain "${worksMessage}"`);
15+
}
2216

23-
await createNpmConfigForAuthentication(true);
24-
const { stdout: stdout2 } = await ng('update');
25-
if (!stdout2.includes(worksMessage)) {
26-
throw new Error(`Expected stdout to contain "${worksMessage}"`);
27-
}
17+
await createNpmConfigForAuthentication(true);
18+
const { stdout: stdout2 } = await ng('update');
19+
if (!stdout2.includes(worksMessage)) {
20+
throw new Error(`Expected stdout to contain "${worksMessage}"`);
21+
}
2822

29-
// Invalid authentication token
30-
await createNpmConfigForAuthentication(false, true);
31-
await expectToFail(() => ng('update'));
23+
// Invalid authentication token
24+
await createNpmConfigForAuthentication(false, true);
25+
await expectToFail(() => ng('update'));
3226

33-
await createNpmConfigForAuthentication(true, true);
34-
await expectToFail(() => ng('update'));
35-
} finally {
36-
if (originalRegistryVariable) {
37-
process.env['NPM_CONFIG_REGISTRY'] = originalRegistryVariable;
38-
}
39-
}
27+
await createNpmConfigForAuthentication(true, true);
28+
await expectToFail(() => ng('update'));
4029
}

‎tests/legacy-cli/e2e/utils/packages.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,7 @@ export async function setRegistry(useTestRegistry: boolean): Promise<void> {
5353
// Safe to set a user configuration on CI
5454
await npm('config', 'set', 'registry', url);
5555
} else {
56-
// Yarn does not use the environment variable so an .npmrc file is also required
57-
await writeFile('.npmrc', `registry=${url}`);
56+
// Yarn supports both `NPM_CONFIG_REGISTRY` and `YARN_REGISTRY`.
5857
process.env['NPM_CONFIG_REGISTRY'] = url;
5958
}
6059
}

‎tests/legacy-cli/e2e_runner.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -173,16 +173,19 @@ testsToRun
173173
.then(() => {
174174
// If we're not in a setup, change the directory back to where it was before the test.
175175
// This allows tests to chdir without worrying about keeping the original directory.
176-
if (allSetups.indexOf(relativeName) == -1 && previousDir) {
176+
if (!allSetups.includes(relativeName) && previousDir) {
177177
process.chdir(previousDir);
178+
179+
// Restore env variables before each test.
180+
console.log(' Restoring original environment variables...');
181+
process.env = originalEnvVariables;
178182
}
179183
})
180184
.then(() => {
181185
// Only clean after a real test, not a setup step. Also skip cleaning if the test
182186
// requested an exception.
183-
if (allSetups.indexOf(relativeName) == -1 && clean) {
187+
if (!allSetups.includes(relativeName) && clean) {
184188
logStack.push(new logging.NullLogger());
185-
186189
return gitClean().then(
187190
() => logStack.pop(),
188191
(err) => {
@@ -192,11 +195,6 @@ testsToRun
192195
);
193196
}
194197
})
195-
.finally(() => {
196-
// Restore env variables after each test.
197-
console.log(' Restoring original environment variables...');
198-
process.env = originalEnvVariables;
199-
})
200198
.then(
201199
() => printFooter(currentFileName, start),
202200
(err) => {

0 commit comments

Comments
 (0)