From 31067371f2aee12f5dc0a8e316141af77f2870fe Mon Sep 17 00:00:00 2001 From: Connor Peet Date: Thu, 21 Sep 2023 14:48:21 -0700 Subject: [PATCH] eng: fix snapshot tests in macos webkit for real? (#193734) Second attempt at this, which should work. Stop trying to use exposeFunction, which seems to work poorly on macOS webkit in CI, and just run a server with HTTP endpoints to do the "RPC." Reuses Alex's "yaserver" module that we were already using for other tests. Uses a secure random prefix for security in each run. --- package.json | 2 +- .../gettingStartedMarkdownRenderer.test.ts | 2 +- test/unit/browser/index.js | 87 +++++++++++++++---- test/unit/browser/renderer.html | 40 +++++---- yarn.lock | 8 +- 5 files changed, 96 insertions(+), 43 deletions(-) diff --git a/package.json b/package.json index e4e125925a7..48f132b44ef 100644 --- a/package.json +++ b/package.json @@ -219,7 +219,7 @@ "webpack-cli": "^5.0.1", "webpack-stream": "^7.0.0", "xml2js": "^0.5.0", - "yaserver": "^0.2.0" + "yaserver": "^0.4.0" }, "repository": { "type": "git", diff --git a/src/vs/workbench/contrib/welcomeGettingStarted/test/browser/gettingStartedMarkdownRenderer.test.ts b/src/vs/workbench/contrib/welcomeGettingStarted/test/browser/gettingStartedMarkdownRenderer.test.ts index 773842399ae..3ff85835829 100644 --- a/src/vs/workbench/contrib/welcomeGettingStarted/test/browser/gettingStartedMarkdownRenderer.test.ts +++ b/src/vs/workbench/contrib/welcomeGettingStarted/test/browser/gettingStartedMarkdownRenderer.test.ts @@ -23,7 +23,7 @@ suite('Getting Started Markdown Renderer', () => { const rendered = await renderer.renderMarkdown(mdPath, mdBase); const imageSrcs = [...rendered.matchAll(/img src="[^"]*"/g)].map(match => match[0]); for (const src of imageSrcs) { - const targetSrcFormat = /^img src="https:\/\/file\+.vscode-resource.vscode-cdn.net\/.*\/vs\/workbench\/contrib\/welcomeGettingStarted\/common\/media\/.*.png"$/; + const targetSrcFormat = /^img src=".*\/vs\/workbench\/contrib\/welcomeGettingStarted\/common\/media\/.*.png"$/; assert(targetSrcFormat.test(src), `${src} didnt match regex`); } languageService.dispose(); diff --git a/test/unit/browser/index.js b/test/unit/browser/index.js index 2260a1d41ad..5084f654f29 100644 --- a/test/unit/browser/index.js +++ b/test/unit/browser/index.js @@ -17,6 +17,9 @@ const minimatch = require('minimatch'); const fs = require('fs'); const playwright = require('@playwright/test'); const { applyReporter } = require('../reporter'); +const yaserver = require('yaserver'); +const http = require('http'); +const { randomBytes } = require('crypto'); // opts const defaultReporterName = process.platform === 'win32' ? 'list' : 'spec'; @@ -60,7 +63,8 @@ const withReporter = (function () { })(); const outdir = argv.build ? 'out-build' : 'out'; -const out = path.join(__dirname, `../../../${outdir}`); +const rootDir = path.resolve(__dirname, '..', '..', '..'); +const out = path.join(rootDir, `${outdir}`); function ensureIsArray(a) { return Array.isArray(a) ? a : [a]; @@ -126,11 +130,69 @@ function consoleLogFn(msg) { return console.log; } +async function createServer() { + // Demand a prefix to avoid issues with other services on the + // machine being able to access the test server. + const prefix = '/' + randomBytes(16).toString('hex'); + const serveStatic = await yaserver.createServer({ rootDir }); + + /** Handles a request for a remote method call, invoking `fn` and returning the result */ + const remoteMethod = async (/** @type {http.IncomingMessage} */ req, /** @type {http.ServerResponse} */ response, fn) => { + const params = await new Promise((resolve, reject) => { + const body = []; + req.on('data', chunk => body.push(chunk)); + req.on('end', () => resolve(JSON.parse(Buffer.concat(body).toString()))); + req.on('error', reject); + }); + + const result = await fn(...params); + response.writeHead(200, { 'Content-Type': 'application/json' }); + response.end(JSON.stringify(result)); + }; + + const server = http.createServer((request, response) => { + if (!request.url?.startsWith(prefix)) { + return response.writeHead(404).end(); + } + + // rewrite the URL so the static server can handle the request correctly + request.url = request.url.slice(prefix.length); + + switch (request.url) { + case '/remoteMethod/__readFileInTests': + return remoteMethod(request, response, p => fs.promises.readFile(p, 'utf-8')); + case '/remoteMethod/__writeFileInTests': + return remoteMethod(request, response, (p, contents) => fs.promises.writeFile(p, contents)); + case '/remoteMethod/__readDirInTests': + return remoteMethod(request, response, p => fs.promises.readdir(p)); + case '/remoteMethod/__unlinkInTests': + return remoteMethod(request, response, p => fs.promises.unlink(p)); + case '/remoteMethod/__mkdirPInTests': + return remoteMethod(request, response, p => fs.promises.mkdir(p, { recursive: true })); + default: + return serveStatic.handle(request, response); + } + }); + + return new Promise((resolve, reject) => { + server.listen(0, 'localhost', () => { + resolve({ + dispose: () => server.close(), + // @ts-ignore + url: `http://localhost:${server.address().port}${prefix}` + }); + }); + server.on('error', reject); + }); +} + async function runTestsInBrowser(testModules, browserType) { + const server = await createServer(); const browser = await playwright[browserType].launch({ headless: !Boolean(argv.debug), devtools: Boolean(argv.debug) }); const context = await browser.newContext(); const page = await context.newPage(); - const target = url.pathToFileURL(path.join(__dirname, 'renderer.html')); + const target = new URL(server.url + '/test/unit/browser/renderer.html'); + target.searchParams.set('baseUrl', url.pathToFileURL(path.join(rootDir, 'src')).toString()); if (argv.build) { target.searchParams.set('build', 'true'); } @@ -138,24 +200,10 @@ async function runTestsInBrowser(testModules, browserType) { target.searchParams.set('ci', 'true'); } - // see comment on warmupExposedMethods in renderer.html for what's going on - if (browserType === 'webkit') { - target.searchParams.set('ioWarmup', __dirname); - } - const emitter = new events.EventEmitter(); - - await Promise.all([ - page.exposeFunction('mocha_report', (type, data1, data2) => { - emitter.emit(type, data1, data2); - }), - // Test file operations that are common across platforms. Used for test infra, namely snapshot tests - page.exposeFunction('__readFileInTests', (path) => fs.promises.readFile(path, 'utf-8')), - page.exposeFunction('__writeFileInTests', (path, contents) => fs.promises.writeFile(path, contents)), - page.exposeFunction('__readDirInTests', (path) => fs.promises.readdir(path)), - page.exposeFunction('__unlinkInTests', (path) => fs.promises.unlink(path)), - page.exposeFunction('__mkdirPInTests', (path) => fs.promises.mkdir(path, { recursive: true })), - ]); + await page.exposeFunction('mocha_report', (type, data1, data2) => { + emitter.emit(type, data1, data2); + }); await page.goto(target.href); @@ -192,6 +240,7 @@ async function runTestsInBrowser(testModules, browserType) { } catch (err) { console.error(err); } + server.dispose(); await browser.close(); if (failingTests.length > 0) { diff --git a/test/unit/browser/renderer.html b/test/unit/browser/renderer.html index c8a25ecf84d..ae678696483 100644 --- a/test/unit/browser/renderer.html +++ b/test/unit/browser/renderer.html @@ -51,7 +51,7 @@ const baseUrl = window.location.href; require.config({ catchError: true, - baseUrl: new URL('../../../src', baseUrl).href, + baseUrl: urlParams.get('baseUrl'), paths: { vs: new URL(`../../../${!!isBuild ? 'out-build' : 'out'}/vs`, baseUrl).href, assert: new URL('../assert.js', baseUrl).href, @@ -116,6 +116,27 @@ runner.on('pending', test => window.mocha_report('pending', serializeRunnable(test))); }; + const remoteMethods = [ + '__readFileInTests', + '__writeFileInTests', + '__readDirInTests', + '__unlinkInTests', + '__mkdirPInTests', + ]; + + for (const method of remoteMethods) { + const prefix = window.location.pathname.split('/')[1]; + globalThis[method] = async (...args) => { + const res = await fetch(`/${prefix}/remoteMethod/${method}`, { + body: JSON.stringify(args), + method: 'POST', + headers: { 'Content-Type': 'application/json' } + }); + + return res.json(); + } + } + async function loadModules(modules) { for (const file of modules) { @@ -129,25 +150,8 @@ } } - /** - * There is some bug in WebKit on macOS in CI only that causes the first - * invokation of the file functions to (sometimes) take an inordinately - * long period of time to run. Get around this by invoking them here. - */ - async function doIoWarmup() { - const dir = url.searchParams.get('ioWarmup'); - if (!dir) { - return; - } - - // these are the only two functions actually used in CI presently: - await __readFileInTests(dir + '/' + 'renderer.html'); - await __readDirInTests(dir); - } - window.loadAndRun = async function loadAndRun({ modules, grep }, manual = false) { // load - await doIoWarmup(); await loadModules(modules); // await new Promise((resolve, reject) => { // require(modules, resolve, err => { diff --git a/yarn.lock b/yarn.lock index 02ee18f6e8b..468203799a1 100644 --- a/yarn.lock +++ b/yarn.lock @@ -10878,10 +10878,10 @@ yargs@^7.1.0: y18n "^3.2.1" yargs-parser "5.0.0-security.0" -yaserver@^0.2.0: - version "0.2.0" - resolved "https://registry.yarnpkg.com/yaserver/-/yaserver-0.2.0.tgz#56393027dc13f3c1bb89d20e0bd17269aa927802" - integrity sha512-onsELrl7Y42M4P3T9R0N/ZJNJRu4cGwzhDyOWIFRMJvPUIrGKInYGh+DJBefrbr1qoyDu7DSCLl9BL5hSSVfDA== +yaserver@^0.4.0: + version "0.4.0" + resolved "https://registry.yarnpkg.com/yaserver/-/yaserver-0.4.0.tgz#71b5fc53fb14c0f241d2dcfb3910707feeb619da" + integrity sha512-98Vj4sgqB1fLcpf2wK7h3dFCaabISHU9CXZHaAx3QLkvTTCD31MzMcNbw5V5jZFBK7ffkFqfWig6B20KQt4wtA== yauzl@^2.10.0, yauzl@^2.4.2, yauzl@^2.9.2: version "2.10.0"