Skip to content

Commit b5bbe7c

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 3fa38b0 commit b5bbe7c

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
@@ -11,6 +11,7 @@ interface ExecOptions {
1111
silent?: boolean;
1212
waitForMatch?: RegExp;
1313
env?: { [varname: string]: string };
14+
stdin?: string;
1415
}
1516

1617
let _processes: child_process.ChildProcess[] = [];
@@ -107,6 +108,10 @@ function _exec(options: ExecOptions, cmd: string, args: string[]): Promise<Proce
107108
),
108109
);
109110
});
111+
childProcess.on('error', (error) => {
112+
err.message += `${error}...\n\nSTDOUT:\n${stdout}\n\nSTDERR:\n${stderr}\n`;
113+
reject(err);
114+
});
110115

111116
if (options.waitForMatch) {
112117
const match = options.waitForMatch;
@@ -123,6 +128,12 @@ function _exec(options: ExecOptions, cmd: string, args: string[]): Promise<Proce
123128
}
124129
});
125130
}
131+
132+
// Provide input to stdin if given.
133+
if (options.stdin) {
134+
childProcess.stdin.write(options.stdin);
135+
childProcess.stdin.end();
136+
}
126137
});
127138
}
128139

@@ -174,8 +185,13 @@ export function silentExec(cmd: string, ...args: string[]) {
174185
return _exec({ silent: true }, cmd, args);
175186
}
176187

177-
export function execWithEnv(cmd: string, args: string[], env: { [varname: string]: string }) {
178-
return _exec({ env }, cmd, args);
188+
export function execWithEnv(
189+
cmd: string,
190+
args: string[],
191+
env: { [varname: string]: string },
192+
stdin?: string,
193+
) {
194+
return _exec({ env, stdin }, cmd, args);
179195
}
180196

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

0 commit comments

Comments
 (0)