Skip to content

Commit b5e0e25

Browse files
committed
test(functional): Add test for auto-reload after failed load
When auto-reload is enabled, and the developer accidentally breaks the extension, then web-ext will try to reload the extension, which causes it to be disabled in the browser. In the case of Chrome, the new CDP-based method can successfully reload the extension (Chrome 126+). The fallback for older Chrome does not work. This patch adds test coverage for that scenario, and updates the fake-chrome-binary to accurately simulate that scenario. Tested with real Chrome 75, 131, 138, 139. CHROME_PATH=/path/to/chrome75/chromium TEST_WEBEXT_USE_REAL_CHROME=1 ./node_modules/.bin/mocha tests/functional/test.cli.run-target-chromium.js
1 parent 478115f commit b5e0e25

File tree

3 files changed

+85
-4
lines changed

3 files changed

+85
-4
lines changed

tests/functional/common.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,10 +131,11 @@ export function execWebExt(argv, spawnOptions) {
131131
export function monitorOutput(spawnedProcess) {
132132
const callbacks = new Set();
133133
let outputData = '';
134+
let errorData = '';
134135
function checkCallbacks() {
135136
for (const callback of callbacks) {
136137
const { outputTestFunc, resolve } = callback;
137-
if (outputTestFunc(outputData)) {
138+
if (outputTestFunc(outputData, errorData)) {
138139
callbacks.delete(callback);
139140
resolve();
140141
}
@@ -144,6 +145,10 @@ export function monitorOutput(spawnedProcess) {
144145
outputData += data;
145146
checkCallbacks();
146147
});
148+
spawnedProcess.stderr.on('data', (data) => {
149+
errorData += data;
150+
checkCallbacks();
151+
});
147152

148153
const waitUntilOutputMatches = (outputTestFunc) => {
149154
return new Promise((resolve) => {

tests/functional/fake-chrome-binary.js

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
#!/usr/bin/env node
22

3-
import { createReadStream, createWriteStream } from 'node:fs';
3+
import { createReadStream, createWriteStream, existsSync } from 'node:fs';
44
import { readFile } from 'node:fs/promises';
55
import http from 'node:http';
66
import net from 'node:net';
@@ -185,6 +185,16 @@ async function handleChromeDevtoolsProtocolMessage(rawRequest) {
185185
}
186186
// No further validation: Unknown keys in params are accepted by Chrome.
187187

188+
if (!existsSync(path.join(request.params.path, 'manifest.json'))) {
189+
return {
190+
id,
191+
error: {
192+
code: -32600,
193+
message: 'Manifest file is missing or unreadable',
194+
},
195+
};
196+
}
197+
188198
const { extensionId } = await fakeLoadChromExtension(request.params.path);
189199
return { id, result: { id: extensionId } };
190200
}
@@ -198,6 +208,7 @@ async function handleChromeDevtoolsProtocolMessage(rawRequest) {
198208
}
199209

200210
let isAttachingToTarget;
211+
let isDisabledDueToLoadFailure;
201212
async function simulateResponsesForReloadViaDeveloperPrivate(request) {
202213
// Supports reloadAllExtensionsFallbackForChrome125andEarlier. This is NOT
203214
// the full real protocol response; just enough for the simulation to work.
@@ -232,6 +243,29 @@ async function simulateResponsesForReloadViaDeveloperPrivate(request) {
232243
}
233244
const extensionsToReload = Array.from(loadedFakeExtensions);
234245
for (const dir of extensionsToReload) {
246+
if (!existsSync(path.join(dir, 'manifest.json'))) {
247+
isDisabledDueToLoadFailure = true;
248+
// In reality an exception has many more fields, but for simplicity we
249+
// omit anything we don't already rely on.
250+
return {
251+
id,
252+
result: {
253+
exceptionDetails: {
254+
text: 'Uncaught (in promise) Error: Manifest file is missing or unreadable',
255+
},
256+
},
257+
};
258+
}
259+
if (isDisabledDueToLoadFailure) {
260+
// In theory, one would expect to be able to reload an extension that
261+
// failed to load. In practice, that is not the case, the developer
262+
// has to manually enter chrome://extensions/, use the file picker to
263+
// select the original extension. This weird issue is tracked at
264+
// https://crbug.com/792277 (but even if it were to be fixed, we are
265+
// never going to encounter the fix, since this code path is only
266+
// taken for Chrome 125 and earlier, which will never be fixed).
267+
continue;
268+
}
235269
await fakeLoadChromExtension(dir);
236270
}
237271
return { id, result: { result: { value: extensionsToReload.length } } };

tests/functional/test.cli.run-target-chromium.js

Lines changed: 44 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@
2626

2727
import path from 'node:path';
2828
import { existsSync } from 'node:fs';
29-
import { writeFile } from 'node:fs/promises';
29+
import { writeFile, rename } from 'node:fs/promises';
3030

3131
import { describe, it, beforeEach, afterEach } from 'mocha';
3232
import { assert, expect } from 'chai';
@@ -148,6 +148,46 @@ describe('web-ext run -t chromium', () => {
148148
assert.equal(testServer.requestCount, 1, 'No reload with --no-reload');
149149
}
150150

151+
// Now test what happens if the extension becomes invalid,
152+
// then valid again.
153+
let isUsingFallbackForChrome125andEarlier = false;
154+
if (expectReload) {
155+
const manifestPathOld = path.join(srcDir, 'manifest.json');
156+
const manifestPathNew = path.join(srcDir, 'manifest.json.deleted');
157+
await rename(manifestPathOld, manifestPathNew);
158+
await outputMonitor.waitUntilOutputMatches((output, errout) => {
159+
if (errout.includes('Cannot load extension via CDP, falling back')) {
160+
isUsingFallbackForChrome125andEarlier = true;
161+
return true;
162+
}
163+
return errout.includes('Manifest file is missing or unreadable');
164+
});
165+
await rename(manifestPathNew, manifestPathOld);
166+
if (isUsingFallbackForChrome125andEarlier) {
167+
// When an extension is disabled due to a bad manifest, there is
168+
// unfortunately no way to revive it, due to https://crbug.com/792277
169+
assert.equal(
170+
testServer.requestCount,
171+
2,
172+
'No reload after bad reload when using --load-extension',
173+
);
174+
} else {
175+
if (testServer.requestCount === 2) {
176+
await testServer.waitForHelloFromExtension();
177+
}
178+
assert.equal(testServer.requestCount, 3, 'Reloaded after bad load');
179+
}
180+
// Sanity check: When using fake-chrome-binary, verify that it falls
181+
// back to --load-extension when we expect it to.
182+
if (!useRealChrome) {
183+
assert.equal(
184+
isUsingFallbackForChrome125andEarlier,
185+
chromeVersion < 126,
186+
'isUsingFallbackForChrome125andEarlier only true in Chrome < 126',
187+
);
188+
}
189+
}
190+
151191
// Must send SIGINT so that chrome-launcher (used by web-ext) has a
152192
// chance to terminate the browser that it spawned. Using plain kill
153193
// can cause Chrome processes to be left behind.
@@ -166,7 +206,9 @@ describe('web-ext run -t chromium', () => {
166206
// Example of showing stderr from the last minute:
167207
// find /tmp/ -mmin 1 -name chrome-err.log -exec cat {} \; ;echo
168208
await cmd.waitForExit;
169-
if (expectReload) {
209+
if (expectReload && !isUsingFallbackForChrome125andEarlier) {
210+
assert.equal(testServer.requestCount, 3, 'No unexpected requests');
211+
} else if (expectReload && isUsingFallbackForChrome125andEarlier) {
170212
assert.equal(testServer.requestCount, 2, 'No unexpected requests');
171213
} else {
172214
assert.equal(testServer.requestCount, 1, 'No unexpected requests');

0 commit comments

Comments
 (0)