Skip to content

Commit da3bb43

Browse files
lukastaegertlukastaegert
authored
More watch test stabilization (#4339)
* Make CLI tests repeatable * Add watcher to test * Make it more lightweight by using fs.watch * More logging * Use timeouts again * Remove repetitions
1 parent 205e861 commit da3bb43

File tree

4 files changed

+137
-123
lines changed

4 files changed

+137
-123
lines changed

‎test/cli/index.js‎

Lines changed: 110 additions & 101 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,8 @@ const sander = require('sander');
55
const {
66
normaliseOutput,
77
runTestSuiteWithSamples,
8-
assertDirectoriesAreEqual
8+
assertDirectoriesAreEqual,
9+
getFileNamesAndRemoveOutput
910
} = require('../utils.js');
1011

1112
const cwd = process.cwd();
@@ -17,123 +18,131 @@ runTestSuiteWithSamples(
1718
'cli',
1819
path.resolve(__dirname, 'samples'),
1920
(dir, config) => {
20-
(config.skip ? it.skip : config.solo ? it.only : it)(
21-
path.basename(dir) + ': ' + config.description,
22-
done => {
23-
process.chdir(config.cwd || dir);
24-
if (config.before) config.before();
21+
// allow to repeat flaky tests for debugging on CLI
22+
for (let pass = 0; pass < (config.repeat || 1); pass++) {
23+
runTest(dir, config, pass);
24+
}
25+
},
26+
() => process.chdir(cwd)
27+
);
2528

26-
const command = config.command.replace(
27-
/(^| )rollup($| )/g,
28-
`node ${path.resolve(__dirname, '../../dist/bin')}${path.sep}rollup `
29-
);
29+
function runTest(dir, config, pass) {
30+
const name = path.basename(dir) + ': ' + config.description;
31+
(config.skip ? it.skip : config.solo ? it.only : it)(
32+
pass > 0 ? `${name} (pass ${pass + 1})` : name,
33+
done => {
34+
process.chdir(config.cwd || dir);
35+
if (pass > 0) {
36+
getFileNamesAndRemoveOutput(dir);
37+
}
38+
if (config.before) config.before();
3039

31-
const childProcess = exec(
32-
command,
33-
{
34-
timeout: 40000,
35-
env: { ...process.env, FORCE_COLOR: '0', ...config.env }
36-
},
37-
(err, code, stderr) => {
38-
if (config.after) config.after(err, code, stderr);
39-
if (err && !err.killed) {
40-
if (config.error) {
41-
const shouldContinue = config.error(err);
42-
if (!shouldContinue) return done();
43-
} else {
44-
throw err;
45-
}
46-
}
40+
const command = config.command.replace(
41+
/(^| )rollup($| )/g,
42+
`node ${path.resolve(__dirname, '../../dist/bin')}${path.sep}rollup `
43+
);
4744

48-
if ('stderr' in config) {
49-
const shouldContinue = config.stderr(stderr);
45+
const childProcess = exec(
46+
command,
47+
{
48+
timeout: 40000,
49+
env: { ...process.env, FORCE_COLOR: '0', ...config.env }
50+
},
51+
(err, code, stderr) => {
52+
if (config.after) config.after(err, code, stderr);
53+
if (err && !err.killed) {
54+
if (config.error) {
55+
const shouldContinue = config.error(err);
5056
if (!shouldContinue) return done();
51-
} else if (stderr) {
52-
console.error(stderr);
57+
} else {
58+
throw err;
5359
}
60+
}
5461

55-
let unintendedError;
56-
57-
if (config.execute) {
58-
try {
59-
const fn = new Function('require', 'module', 'exports', 'assert', code);
60-
const module = {
61-
exports: {}
62-
};
63-
fn(require, module, module.exports, assert);
62+
if ('stderr' in config) {
63+
const shouldContinue = config.stderr(stderr);
64+
if (!shouldContinue) return done();
65+
} else if (stderr) {
66+
console.error(stderr);
67+
}
6468

65-
if (config.error) {
66-
unintendedError = new Error('Expected an error while executing output');
67-
}
69+
let unintendedError;
6870

69-
if (config.exports) {
70-
config.exports(module.exports);
71-
}
72-
} catch (err) {
73-
if (config.error) {
74-
config.error(err);
75-
} else {
76-
unintendedError = err;
77-
}
78-
}
71+
if (config.execute) {
72+
try {
73+
const fn = new Function('require', 'module', 'exports', 'assert', code);
74+
const module = {
75+
exports: {}
76+
};
77+
fn(require, module, module.exports, assert);
7978

80-
if (config.show || unintendedError) {
81-
console.log(code + '\n\n\n');
79+
if (config.error) {
80+
unintendedError = new Error('Expected an error while executing output');
8281
}
8382

84-
if (config.solo) console.groupEnd();
85-
86-
unintendedError ? done(unintendedError) : done();
87-
} else if (config.result) {
88-
try {
89-
config.result(code);
90-
done();
91-
} catch (err) {
92-
done(err);
93-
}
94-
} else if (config.test) {
95-
try {
96-
config.test();
97-
done();
98-
} catch (err) {
99-
done(err);
100-
}
101-
} else if (
102-
sander.existsSync('_expected') &&
103-
sander.statSync('_expected').isDirectory()
104-
) {
105-
try {
106-
assertDirectoriesAreEqual('_actual', '_expected');
107-
done();
108-
} catch (err) {
109-
done(err);
83+
if (config.exports) {
84+
config.exports(module.exports);
11085
}
111-
} else {
112-
const expected = sander.readFileSync('_expected.js').toString();
113-
try {
114-
assert.equal(normaliseOutput(code), normaliseOutput(expected));
115-
done();
116-
} catch (err) {
117-
done(err);
86+
} catch (err) {
87+
if (config.error) {
88+
config.error(err);
89+
} else {
90+
unintendedError = err;
11891
}
11992
}
120-
}
121-
);
12293

123-
childProcess.stderr.on('data', async data => {
124-
if (config.abortOnStderr) {
94+
if (config.show || unintendedError) {
95+
console.log(code + '\n\n\n');
96+
}
97+
98+
if (config.solo) console.groupEnd();
99+
100+
unintendedError ? done(unintendedError) : done();
101+
} else if (config.result) {
125102
try {
126-
if (await config.abortOnStderr(data)) {
127-
childProcess.kill('SIGTERM');
128-
}
103+
config.result(code);
104+
done();
105+
} catch (err) {
106+
done(err);
107+
}
108+
} else if (config.test) {
109+
try {
110+
config.test();
111+
done();
112+
} catch (err) {
113+
done(err);
114+
}
115+
} else if (sander.existsSync('_expected') && sander.statSync('_expected').isDirectory()) {
116+
try {
117+
assertDirectoriesAreEqual('_actual', '_expected');
118+
done();
119+
} catch (err) {
120+
done(err);
121+
}
122+
} else {
123+
const expected = sander.readFileSync('_expected.js').toString();
124+
try {
125+
assert.equal(normaliseOutput(code), normaliseOutput(expected));
126+
done();
129127
} catch (err) {
130-
childProcess.kill('SIGTERM');
131128
done(err);
132129
}
133130
}
134-
});
135-
}
136-
).timeout(50000);
137-
},
138-
() => process.chdir(cwd)
139-
);
131+
}
132+
);
133+
134+
childProcess.stderr.on('data', async data => {
135+
if (config.abortOnStderr) {
136+
try {
137+
if (await config.abortOnStderr(data)) {
138+
childProcess.kill('SIGTERM');
139+
}
140+
} catch (err) {
141+
childProcess.kill('SIGTERM');
142+
done(err);
143+
}
144+
}
145+
});
146+
}
147+
).timeout(50000);
148+
}

‎test/cli/samples/watch/watch-config-early-update/_config.js‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,9 @@ module.exports = {
88
description: 'immediately reloads the config file if a change happens while it is parsed',
99
command: 'rollup -cw',
1010
before() {
11-
fs.mkdirSync(path.resolve(__dirname, '_actual'));
11+
// This test writes a config file that prints a message to stderr but delays resolving to a
12+
// config. The stderr message is observed by the parent process and triggers overwriting the
13+
// config file. That way, we simulate a complicated config file being changed while it is parsed.
1214
configFile = path.resolve(__dirname, 'rollup.config.js');
1315
fs.writeFileSync(
1416
configFile,
@@ -18,16 +20,15 @@ module.exports = {
1820
setTimeout(
1921
() =>
2022
resolve({
21-
input: { output1: 'main.js' },
23+
input: 'main.js',
2224
output: {
23-
dir: '_actual',
25+
file: '_actual/output1.js',
2426
format: 'es'
2527
}
2628
}),
27-
1000
29+
3000
2830
);
29-
});
30-
`
31+
});`
3132
);
3233
},
3334
after() {
@@ -40,18 +41,18 @@ module.exports = {
4041
`
4142
console.error('updated');
4243
export default {
43-
input: {output2: "main.js"},
44+
input: 'main.js',
4445
output: {
45-
dir: "_actual",
46+
file: '_actual/output2.js',
4647
format: "es"
4748
}
4849
};
4950
`
5051
);
5152
return false;
5253
}
53-
if (data === 'updated\n') {
54-
return new Promise(resolve => setTimeout(() => resolve(true), 500));
54+
if (data.includes(`created _actual${path.sep}output2.js`)) {
55+
return new Promise(resolve => setTimeout(() => resolve(true), 600));
5556
}
5657
}
5758
};

‎test/cli/samples/watch/watch-config-error/_config.js‎

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -11,22 +11,25 @@ module.exports = {
1111
configFile = path.resolve(__dirname, 'rollup.config.js');
1212
fs.writeFileSync(
1313
configFile,
14-
'export default {\n' +
15-
'\tinput: "main.js",\n' +
16-
'\toutput: {\n' +
17-
'\t\tfile: "_actual/main1.js",\n' +
18-
'\t\tformat: "es"\n' +
19-
'\t}\n' +
20-
'};'
14+
`
15+
export default {
16+
input: "main.js",
17+
output: {
18+
file: "_actual/main1.js",
19+
format: "es"
20+
}
21+
};`
2122
);
2223
},
2324
after() {
24-
// synchronous sometimes does not seem to work, probably because the watch is not yet removed properly
25-
setTimeout(() => fs.unlinkSync(configFile), 300);
25+
fs.unlinkSync(configFile);
2626
},
2727
abortOnStderr(data) {
2828
if (data.includes(`created _actual${path.sep}main1.js`)) {
29-
atomicWriteFileSync(configFile, 'throw new Error("Config contains errors");');
29+
setTimeout(
30+
() => atomicWriteFileSync(configFile, 'throw new Error("Config contains errors");'),
31+
600
32+
);
3033
return false;
3134
}
3235
if (data.includes('Config contains errors')) {
@@ -41,11 +44,11 @@ module.exports = {
4144
'\t}\n' +
4245
'};'
4346
);
44-
}, 400);
47+
}, 600);
4548
return false;
4649
}
4750
if (data.includes(`created _actual${path.sep}main2.js`)) {
48-
return true;
51+
return new Promise(resolve => setTimeout(() => resolve(true), 600));
4952
}
5053
}
5154
};

‎test/utils.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ exports.assertDirectoriesAreEqual = assertDirectoriesAreEqual;
1616
exports.assertFilesAreEqual = assertFilesAreEqual;
1717
exports.assertIncludes = assertIncludes;
1818
exports.atomicWriteFileSync = atomicWriteFileSync;
19+
exports.getFileNamesAndRemoveOutput = getFileNamesAndRemoveOutput;
1920

2021
function normaliseError(error) {
2122
delete error.stack;

0 commit comments

Comments
 (0)