Skip to content

Commit 0136bfa

Browse files
committed
refactor: rewritting analytics test to be more hermetic and stable
This fixes a few issues with the test: 1. The test accidentally used the real user's `$HOME` directory which led to confusing, non-hermetic failures. 2. The test had timeouts of 5 milliseconds, rather than the presumably intended 5 seconds. 3. `ng update` would always fail because there's no project. Changed to `ng update --help` which still skips the analytics prompt but doesn't require a complicated setup. 4. The test would end after seeing the analytics prompt and wouldn't bother accepting it. I added functionality to send data to stdin to accept the prompt which makes test logic simpler (no need to manually kill all processes or wait for a timeout), though I didn't add any new assertions that the CLI actually tracks / doesn't track correctly. Refs #23003.
1 parent 8da9269 commit 0136bfa

File tree

2 files changed

+64
-34
lines changed

2 files changed

+64
-34
lines changed
Lines changed: 46 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -1,49 +1,63 @@
1-
import { execWithEnv, killAllProcesses, waitForAnyProcessOutputToMatch } from '../../utils/process';
2-
import { expectToFail } from '../../utils/utils';
1+
import { promises as fs } from 'fs';
2+
import { execWithEnv } from '../../utils/process';
3+
4+
const ANALYTICS_PROMPT = /Would you like to share anonymous usage data/;
35

46
export default async function () {
5-
try {
6-
// Execute a command with TTY force enabled
7-
execWithEnv('ng', ['version'], {
8-
...process.env,
9-
NG_FORCE_TTY: '1',
10-
});
7+
// CLI should prompt for analytics permissions.
8+
await mockHome(async (home) => {
9+
const { stdout } = await execWithEnv(
10+
'ng',
11+
['version'],
12+
{
13+
...process.env,
14+
HOME: home,
15+
NG_FORCE_TTY: '1',
16+
},
17+
'y' /* stdin */,
18+
);
1119

12-
// Check if the prompt is shown
13-
await waitForAnyProcessOutputToMatch(/Would you like to share anonymous usage data/);
14-
} finally {
15-
killAllProcesses();
16-
}
20+
if (!ANALYTICS_PROMPT.test(stdout)) {
21+
throw new Error('CLI did not prompt for analytics permission.');
22+
}
23+
});
1724

18-
try {
19-
// Execute a command with TTY force enabled
20-
execWithEnv('ng', ['version'], {
25+
// CLI should skip analytics prompt with `NG_CLI_ANALYTICS=false`.
26+
await mockHome(async (home) => {
27+
const { stdout } = await execWithEnv('ng', ['version'], {
2128
...process.env,
29+
HOME: home,
2230
NG_FORCE_TTY: '1',
2331
NG_CLI_ANALYTICS: 'false',
2432
});
2533

26-
// Check if the prompt is shown
27-
await expectToFail(() =>
28-
waitForAnyProcessOutputToMatch(/Would you like to share anonymous usage data/, 5),
29-
);
30-
} finally {
31-
killAllProcesses();
32-
}
34+
if (ANALYTICS_PROMPT.test(stdout)) {
35+
throw new Error('CLI prompted for analytics permission when it should be forced off.');
36+
}
37+
});
3338

34-
// Should not show a prompt when using update
35-
try {
36-
// Execute a command with TTY force enabled
37-
execWithEnv('ng', ['update'], {
39+
// CLI should skip analytics prompt during `ng update`.
40+
await mockHome(async (home) => {
41+
const { stdout } = await execWithEnv('ng', ['update', '--help'], {
3842
...process.env,
43+
HOME: home,
3944
NG_FORCE_TTY: '1',
4045
});
4146

42-
// Check if the prompt is shown
43-
await expectToFail(() =>
44-
waitForAnyProcessOutputToMatch(/Would you like to share anonymous usage data/, 5),
45-
);
47+
if (ANALYTICS_PROMPT.test(stdout)) {
48+
throw new Error(
49+
'CLI prompted for analytics permission during an update where it should not' + ' have.',
50+
);
51+
}
52+
});
53+
}
54+
55+
async function mockHome(cb: (home: string) => Promise<void>): Promise<void> {
56+
const tempHome = await fs.mkdtemp('angular-cli-e2e-home-');
57+
58+
try {
59+
await cb(tempHome);
4660
} finally {
47-
killAllProcesses();
61+
await fs.rm(tempHome, { recursive: true, force: true });
4862
}
4963
}

tests/legacy-cli/e2e/utils/process.ts

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ interface ExecOptions {
1212
silent?: boolean;
1313
waitForMatch?: RegExp;
1414
env?: { [varname: string]: string };
15+
stdin?: string;
1516
}
1617

1718

@@ -96,6 +97,10 @@ function _exec(options: ExecOptions, cmd: string, args: string[]): Promise<Proc
9697
reject(err);
9798
}
9899
});
100+
childProcess.on('error', (error) => {
101+
err.message += `${error}...\n\nSTDOUT:\n${stdout}\n\nSTDERR:\n${stderr}\n`;
102+
reject(err);
103+
});
99104

100105
if (options.waitForMatch) {
101106
const match = options.waitForMatch;
@@ -110,6 +115,12 @@ function _exec(options: ExecOptions, cmd: string, args: string[]): Promise<Proc
110115
}
111116
});
112117
}
118+
119+
// Provide input to stdin if given.
120+
if (options.stdin) {
121+
childProcess.stdin.write(options.stdin);
122+
childProcess.stdin.end();
123+
}
113124
});
114125
}
115126

@@ -157,8 +168,13 @@ export function silentExec(cmd: string, ...args: string[]) {
157168
return _exec({ silent: true }, cmd, args);
158169
}
159170

160-
export function execWithEnv(cmd: string, args: string[], env: { [varname: string]: string }) {
161-
return _exec({ env }, cmd, args);
171+
export function execWithEnv(
172+
cmd: string,
173+
args: string[],
174+
env: { [varname: string]: string },
175+
stdin?: string,
176+
) {
177+
return _exec({ env, stdin }, cmd, args);
162178
}
163179

164180
export function execAndWaitForOutputToMatch(cmd: string, args: string[], match: RegExp) {

0 commit comments

Comments
 (0)