From 3d02a6c7697014a73d61ce21f6c01b10820130ea Mon Sep 17 00:00:00 2001 From: adroitwhiz Date: Tue, 7 Jan 2020 14:46:34 -0500 Subject: [PATCH 1/6] Replace Chromeless with Puppeteer --- package.json | 2 +- test/integration/pick-tests.js | 26 +++++----- test/integration/scratch-tests.js | 85 ++++++++++++++++--------------- 3 files changed, 59 insertions(+), 54 deletions(-) diff --git a/package.json b/package.json index 91c1c6de3..65e0dd4fc 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,6 @@ "babel-loader": "^7.1.4", "babel-polyfill": "^6.22.0", "babel-preset-env": "^1.6.1", - "chromeless": "^1.5.1", "copy-webpack-plugin": "^4.5.1", "docdash": "^0.4.0", "eslint": "^4.6.1", @@ -37,6 +36,7 @@ "gh-pages": "^1.0.0", "jsdoc": "^3.5.5", "json": "^9.0.4", + "puppeteer": "^2.0.0", "scratch-vm": "0.2.0-prerelease.20191227164934", "tap": "^11.0.0", "travis-after-all": "^1.4.4", diff --git a/test/integration/pick-tests.js b/test/integration/pick-tests.js index 6d3222e50..37341fc62 100644 --- a/test/integration/pick-tests.js +++ b/test/integration/pick-tests.js @@ -1,29 +1,31 @@ /* global vm, render, Promise */ -const {Chromeless} = require('chromeless'); +const puppeteer = require('puppeteer'); const test = require('tap').test; const path = require('path'); -const chromeless = new Chromeless(); const indexHTML = path.resolve(__dirname, 'index.html'); const testDir = (...args) => path.resolve(__dirname, 'pick-tests', ...args); -const runFile = (file, action, script) => +const runFile = async (file, action, page, script) => { // start each test by going to the index.html, and loading the scratch file - chromeless.goto(`file://${indexHTML}`) - .setFileInput('#file', testDir(file)) - // the index.html handler for file input will add a #loaded element when it - // finishes. - .wait('#loaded') - .evaluate(`function () {return (${script})(${action});}`) -; + await page.goto(`file://${indexHTML}`); + const fileInput = await page.$('#file'); + await fileInput.uploadFile(testDir(file)); + // the index.html handler for file input will add a #loaded element when it + // finishes. + await page.waitForSelector('#loaded'); + return page.evaluate(`(function () {return (${script})(${action});})()`); +}; // immediately invoked async function to let us wait for each test to finish before starting the next. (async () => { + const browser = await puppeteer.launch(); + const page = await browser.newPage(); const testOperation = async function (name, action, expect) { await test(name, async t => { - const results = await runFile('test-mouse-touch.sb2', action, boundAction => { + const results = await runFile('test-mouse-touch.sb2', action, page, boundAction => { vm.greenFlag(); const sendResults = []; @@ -97,5 +99,5 @@ const runFile = (file, action, script) => } // close the browser window we used - await chromeless.end(); + await browser.close(); })(); diff --git a/test/integration/scratch-tests.js b/test/integration/scratch-tests.js index f11bc5ba7..022841b4f 100644 --- a/test/integration/scratch-tests.js +++ b/test/integration/scratch-tests.js @@ -1,54 +1,54 @@ /* global vm, Promise */ -const {Chromeless} = require('chromeless'); +const puppeteer = require('puppeteer'); const test = require('tap').test; const path = require('path'); const fs = require('fs'); -const chromeless = new Chromeless(); const indexHTML = path.resolve(__dirname, 'index.html'); const testDir = (...args) => path.resolve(__dirname, 'scratch-tests', ...args); -const testFile = file => test(file, async t => { +const testFile = (file, page) => test(file, async t => { // start each test by going to the index.html, and loading the scratch file - const says = await chromeless.goto(`file://${indexHTML}`) - .setFileInput('#file', testDir(file)) - // the index.html handler for file input will add a #loaded element when it - // finishes. - .wait('#loaded') - .evaluate(() => { - // This function is run INSIDE the integration chrome browser via some - // injection and .toString() magic. We can return some "simple data" - // back across as a promise, so we will just log all the says that happen - // for parsing after. - - // this becomes the `says` in the outer scope - const messages = []; - const TIMEOUT = 5000; - - vm.runtime.on('SAY', (_, __, message) => { - messages.push(message); - }); + await page.goto(`file://${indexHTML}`); + const fileInput = await page.$('#file'); + await fileInput.uploadFile(testDir(file)); + // the index.html handler for file input will add a #loaded element when it + // finishes. + await page.waitForSelector('#loaded'); + const says = await page.evaluate(() => { + // This function is run INSIDE the integration chrome browser via some + // injection and .toString() magic. We can return some "simple data" + // back across as a promise, so we will just log all the says that happen + // for parsing after. + + // this becomes the `says` in the outer scope + const messages = []; + const TIMEOUT = 5000; + + vm.runtime.on('SAY', (_, __, message) => { + messages.push(message); + }); - vm.greenFlag(); - const startTime = Date.now(); - - return Promise.resolve() - .then(async () => { - // waiting for all threads to complete, then we return - while (vm.runtime.threads.some(thread => vm.runtime.isActiveThread(thread))) { - if ((Date.now() - startTime) >= TIMEOUT) { - // if we push the message after end, the failure from tap is not very useful: - // "not ok test after end() was called" - messages.unshift(`fail Threads still running after ${TIMEOUT}ms`); - break; - } - - await new Promise(resolve => setTimeout(resolve, 50)); + vm.greenFlag(); + const startTime = Date.now(); + + return Promise.resolve() + .then(async () => { + // waiting for all threads to complete, then we return + while (vm.runtime.threads.some(thread => vm.runtime.isActiveThread(thread))) { + if ((Date.now() - startTime) >= TIMEOUT) { + // if we push the message after end, the failure from tap is not very useful: + // "not ok test after end() was called" + messages.unshift(`fail Threads still running after ${TIMEOUT}ms`); + break; } - return messages; - }); - }); + await new Promise(resolve => setTimeout(resolve, 50)); + } + + return messages; + }); + }); // Map string messages to tap reporting methods. This will be used // with events from scratch's runtime emitted on block instructions. @@ -103,13 +103,16 @@ const testFile = file => test(file, async t => { // immediately invoked async function to let us wait for each test to finish before starting the next. (async () => { + const browser = await puppeteer.launch(); + const page = await browser.newPage(); + const files = fs.readdirSync(testDir()) .filter(uri => uri.endsWith('.sb2') || uri.endsWith('.sb3')); for (const file of files) { - await testFile(file); + await testFile(file, page); } // close the browser window we used - await chromeless.end(); + await browser.close(); })(); From 0a214a989dcea97705ea790993518318d8d0b784 Mon Sep 17 00:00:00 2001 From: adroitwhiz Date: Wed, 8 Jan 2020 04:18:49 -0500 Subject: [PATCH 2/6] download Chromium only when tests are run --- .travis.yml | 4 ---- package.json | 2 +- test/helper/download-chromium.js | 24 ++++++++++++++++++++++++ test/integration/pick-tests.js | 5 ++++- test/integration/scratch-tests.js | 5 ++++- 5 files changed, 33 insertions(+), 7 deletions(-) create mode 100644 test/helper/download-chromium.js diff --git a/.travis.yml b/.travis.yml index 969909f79..f3da52b33 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,14 +1,10 @@ language: node_js dist: trusty -addons: - chrome: stable node_js: - 8 - node env: - NODE_ENV=production -before_install: - - google-chrome-stable --headless --no-sandbox --remote-debugging-port=9222 & install: - npm --production=false install - npm --production=false update diff --git a/package.json b/package.json index 65e0dd4fc..213244c3b 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,7 @@ "gh-pages": "^1.0.0", "jsdoc": "^3.5.5", "json": "^9.0.4", - "puppeteer": "^2.0.0", + "puppeteer-core": "^2.0.0", "scratch-vm": "0.2.0-prerelease.20191227164934", "tap": "^11.0.0", "travis-after-all": "^1.4.4", diff --git a/test/helper/download-chromium.js b/test/helper/download-chromium.js new file mode 100644 index 000000000..dc475a155 --- /dev/null +++ b/test/helper/download-chromium.js @@ -0,0 +1,24 @@ +const packageJson = require('puppeteer-core/package.json'); +const puppeteer = require('puppeteer-core'); + +const fetcher = puppeteer.createBrowserFetcher(); +const revision = packageJson.puppeteer.chromium_revision; + +/* eslint-disable no-console */ +module.exports = async () => { + const downloadedRevisions = await fetcher.localRevisions(); + if (downloadedRevisions.indexOf(revision) !== -1) { + console.log('Chromium already downloaded'); + return Promise.resolve(); + } + + console.log('Downloading Chromium...'); + return fetcher.download(revision) + .then(() => { + console.log('Downloaded Chromium successfully'); + }) + .catch(error => { + console.error(error); + process.exit(1); + }); +}; diff --git a/test/integration/pick-tests.js b/test/integration/pick-tests.js index 37341fc62..a767ec409 100644 --- a/test/integration/pick-tests.js +++ b/test/integration/pick-tests.js @@ -1,8 +1,10 @@ /* global vm, render, Promise */ -const puppeteer = require('puppeteer'); +const puppeteer = require('puppeteer-core'); const test = require('tap').test; const path = require('path'); +const downloadChromium = require('../helper/download-chromium'); + const indexHTML = path.resolve(__dirname, 'index.html'); const testDir = (...args) => path.resolve(__dirname, 'pick-tests', ...args); @@ -19,6 +21,7 @@ const runFile = async (file, action, page, script) => { // immediately invoked async function to let us wait for each test to finish before starting the next. (async () => { + await downloadChromium(); const browser = await puppeteer.launch(); const page = await browser.newPage(); diff --git a/test/integration/scratch-tests.js b/test/integration/scratch-tests.js index 022841b4f..d6cfebfd9 100644 --- a/test/integration/scratch-tests.js +++ b/test/integration/scratch-tests.js @@ -1,9 +1,11 @@ /* global vm, Promise */ -const puppeteer = require('puppeteer'); +const puppeteer = require('puppeteer-core'); const test = require('tap').test; const path = require('path'); const fs = require('fs'); +const downloadChromium = require('../helper/download-chromium'); + const indexHTML = path.resolve(__dirname, 'index.html'); const testDir = (...args) => path.resolve(__dirname, 'scratch-tests', ...args); @@ -103,6 +105,7 @@ const testFile = (file, page) => test(file, async t => { // immediately invoked async function to let us wait for each test to finish before starting the next. (async () => { + await downloadChromium(); const browser = await puppeteer.launch(); const page = await browser.newPage(); From 5081a71fe9633e27d46bfed6820e62b9feab97b9 Mon Sep 17 00:00:00 2001 From: adroitwhiz Date: Sun, 8 Mar 2020 08:12:40 -0400 Subject: [PATCH 3/6] Fix file input in tests --- test/helper/page-util.js | 53 +++++++++++++++++++++++++++++++ test/integration/cpu-render.html | 43 +++++++------------------ test/integration/index.html | 31 ++---------------- test/integration/pick-tests.js | 9 ++++-- test/integration/scratch-tests.js | 8 +++-- 5 files changed, 79 insertions(+), 65 deletions(-) create mode 100644 test/helper/page-util.js diff --git a/test/helper/page-util.js b/test/helper/page-util.js new file mode 100644 index 000000000..0648e1bfd --- /dev/null +++ b/test/helper/page-util.js @@ -0,0 +1,53 @@ +/* global window, VirtualMachine, ScratchStorage, ScratchSVGRenderer */ +/* eslint-env browser */ + +// Wait for all SVG skins to be loaded. +// TODO: this is extremely janky and should be removed once vm.loadProject waits for SVG skins to load +window.waitForSVGSkinLoad = renderer => new Promise(resolve => { + // eslint-disable-next-line prefer-const + let interval; + + const waitInner = () => { + let numSVGSkins = 0; + let numLoadedSVGSkins = 0; + for (const skin of renderer._allSkins) { + if (skin.constructor.name !== 'SVGSkin') continue; + numSVGSkins++; + if (skin._svgRenderer.loaded) numLoadedSVGSkins++; + } + + if (numSVGSkins === numLoadedSVGSkins) { + clearInterval(interval); + resolve(); + } + }; + + interval = setInterval(waitInner, 1); +}); + +window.loadFileInputIntoVM = (fileInput, vm, render) => { + const reader = new FileReader(); + return new Promise(resolve => { + reader.onload = () => { + vm.start(); + vm.loadProject(reader.result) + .then(() => window.waitForSVGSkinLoad(render)) + .then(() => { + resolve(); + }); + }; + reader.readAsArrayBuffer(fileInput.files[0]); + }); +}; + +window.initVM = render => { + const vm = new VirtualMachine(); + const storage = new ScratchStorage(); + + vm.attachStorage(storage); + vm.attachRenderer(render); + vm.attachV2SVGAdapter(new ScratchSVGRenderer.SVGRenderer()); + vm.attachV2BitmapAdapter(new ScratchSVGRenderer.BitmapAdapter()); + + return vm; +}; diff --git a/test/integration/cpu-render.html b/test/integration/cpu-render.html index d6d363084..26a79c3f4 100644 --- a/test/integration/cpu-render.html +++ b/test/integration/cpu-render.html @@ -2,6 +2,7 @@ + @@ -17,38 +18,18 @@ window.devicePixelRatio = 1; const gpuCanvas = document.getElementById('test'); var render = new ScratchRender(gpuCanvas); - var vm = new VirtualMachine(); - var storage = new ScratchStorage(); + var vm = initVM(render); - vm.attachStorage(storage); - vm.attachRenderer(render); - vm.attachV2SVGAdapter(new ScratchSVGRenderer.SVGRenderer()); - vm.attachV2BitmapAdapter(new ScratchSVGRenderer.BitmapAdapter()); - - document.getElementById('file').addEventListener('click', e => { - document.body.removeChild(document.getElementById('loaded')); - }); - - document.getElementById('file').addEventListener('change', e => { - const reader = new FileReader(); - const thisFileInput = e.target; - reader.onload = () => { - vm.start(); - vm.loadProject(reader.result) - .then(() => { - // we add a `#loaded` div to our document, the integration suite - // waits for that element to show up to assume the vm is ready - // to play! - const div = document.createElement('div'); - div.id='loaded'; - document.body.appendChild(div); - vm.greenFlag(); - setTimeout(() => { - renderCpu(); - }, 1000); - }); - }; - reader.readAsArrayBuffer(thisFileInput.files[0]); + const fileInput = document.getElementById('file'); + const loadFile = loadFileInputIntoVM.bind(null, fileInput, vm, render); + fileInput.addEventListener('change', e => { + loadFile() + .then(() => { + vm.greenFlag(); + setTimeout(() => { + renderCpu(); + }, 1000); + }); }); const cpuCanvas = document.getElementById('cpu'); diff --git a/test/integration/index.html b/test/integration/index.html index e3d8dd838..114fa5b9d 100644 --- a/test/integration/index.html +++ b/test/integration/index.html @@ -2,6 +2,7 @@ + @@ -15,39 +16,13 @@ var canvas = document.getElementById('test'); var render = new ScratchRender(canvas); - var vm = new VirtualMachine(); - var storage = new ScratchStorage(); + var vm = initVM(render); var mockMouse = data => vm.runtime.postIOData('mouse', { canvasWidth: canvas.width, canvasHeight: canvas.height, ...data, }); - vm.attachStorage(storage); - vm.attachRenderer(render); - vm.attachV2SVGAdapter(new ScratchSVGRenderer.SVGRenderer()); - vm.attachV2BitmapAdapter(new ScratchSVGRenderer.BitmapAdapter()); - - document.getElementById('file').addEventListener('click', e => { - document.body.removeChild(document.getElementById('loaded')); - }); - - document.getElementById('file').addEventListener('change', e => { - const reader = new FileReader(); - const thisFileInput = e.target; - reader.onload = () => { - vm.start(); - vm.loadProject(reader.result) - .then(() => { - // we add a `#loaded` div to our document, the integration suite - // waits for that element to show up to assume the vm is ready - // to play! - const div = document.createElement('div'); - div.id='loaded'; - document.body.appendChild(div); - }); - }; - reader.readAsArrayBuffer(thisFileInput.files[0]); - }); + const loadFile = loadFileInputIntoVM.bind(null, document.getElementById('file'), vm, render); diff --git a/test/integration/pick-tests.js b/test/integration/pick-tests.js index a767ec409..a36ee1503 100644 --- a/test/integration/pick-tests.js +++ b/test/integration/pick-tests.js @@ -13,9 +13,12 @@ const runFile = async (file, action, page, script) => { await page.goto(`file://${indexHTML}`); const fileInput = await page.$('#file'); await fileInput.uploadFile(testDir(file)); - // the index.html handler for file input will add a #loaded element when it - // finishes. - await page.waitForSelector('#loaded'); + + await page.evaluate(() => + // `loadFile` is defined on the page itself. + // eslint-disable-next-line no-undef + loadFile() + ); return page.evaluate(`(function () {return (${script})(${action});})()`); }; diff --git a/test/integration/scratch-tests.js b/test/integration/scratch-tests.js index d6cfebfd9..9d3188a37 100644 --- a/test/integration/scratch-tests.js +++ b/test/integration/scratch-tests.js @@ -14,9 +14,11 @@ const testFile = (file, page) => test(file, async t => { await page.goto(`file://${indexHTML}`); const fileInput = await page.$('#file'); await fileInput.uploadFile(testDir(file)); - // the index.html handler for file input will add a #loaded element when it - // finishes. - await page.waitForSelector('#loaded'); + await page.evaluate(() => + // `loadFile` is defined on the page itself. + // eslint-disable-next-line no-undef + loadFile() + ); const says = await page.evaluate(() => { // This function is run INSIDE the integration chrome browser via some // injection and .toString() magic. We can return some "simple data" From ddccd0fa79c9a5873a91d7ad4e7caef42a28f1b1 Mon Sep 17 00:00:00 2001 From: adroitwhiz Date: Thu, 7 May 2020 16:08:28 -0400 Subject: [PATCH 4/6] reference "wait for SVG skins to load" issue --- test/helper/page-util.js | 1 + 1 file changed, 1 insertion(+) diff --git a/test/helper/page-util.js b/test/helper/page-util.js index 0648e1bfd..25f5b007e 100644 --- a/test/helper/page-util.js +++ b/test/helper/page-util.js @@ -3,6 +3,7 @@ // Wait for all SVG skins to be loaded. // TODO: this is extremely janky and should be removed once vm.loadProject waits for SVG skins to load +// https://github.com/LLK/scratch-render/issues/563 window.waitForSVGSkinLoad = renderer => new Promise(resolve => { // eslint-disable-next-line prefer-const let interval; From 51c867fa6ddd0adbc519485c064d1a61ec8c3f7c Mon Sep 17 00:00:00 2001 From: adroitwhiz Date: Thu, 7 May 2020 16:21:37 -0400 Subject: [PATCH 5/6] Use Playwright instead of Puppeteer --- package.json | 2 +- test/helper/download-chromium.js | 24 ------------------------ test/integration/pick-tests.js | 9 +++------ test/integration/scratch-tests.js | 9 +++------ 4 files changed, 7 insertions(+), 37 deletions(-) delete mode 100644 test/helper/download-chromium.js diff --git a/package.json b/package.json index 213244c3b..b69fef97b 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,7 @@ "gh-pages": "^1.0.0", "jsdoc": "^3.5.5", "json": "^9.0.4", - "puppeteer-core": "^2.0.0", + "playwright-chromium": "^1.0.1", "scratch-vm": "0.2.0-prerelease.20191227164934", "tap": "^11.0.0", "travis-after-all": "^1.4.4", diff --git a/test/helper/download-chromium.js b/test/helper/download-chromium.js deleted file mode 100644 index dc475a155..000000000 --- a/test/helper/download-chromium.js +++ /dev/null @@ -1,24 +0,0 @@ -const packageJson = require('puppeteer-core/package.json'); -const puppeteer = require('puppeteer-core'); - -const fetcher = puppeteer.createBrowserFetcher(); -const revision = packageJson.puppeteer.chromium_revision; - -/* eslint-disable no-console */ -module.exports = async () => { - const downloadedRevisions = await fetcher.localRevisions(); - if (downloadedRevisions.indexOf(revision) !== -1) { - console.log('Chromium already downloaded'); - return Promise.resolve(); - } - - console.log('Downloading Chromium...'); - return fetcher.download(revision) - .then(() => { - console.log('Downloaded Chromium successfully'); - }) - .catch(error => { - console.error(error); - process.exit(1); - }); -}; diff --git a/test/integration/pick-tests.js b/test/integration/pick-tests.js index a36ee1503..4012bcdad 100644 --- a/test/integration/pick-tests.js +++ b/test/integration/pick-tests.js @@ -1,10 +1,8 @@ /* global vm, render, Promise */ -const puppeteer = require('puppeteer-core'); +const {chromium} = require('playwright-chromium'); const test = require('tap').test; const path = require('path'); -const downloadChromium = require('../helper/download-chromium'); - const indexHTML = path.resolve(__dirname, 'index.html'); const testDir = (...args) => path.resolve(__dirname, 'pick-tests', ...args); @@ -12,7 +10,7 @@ const runFile = async (file, action, page, script) => { // start each test by going to the index.html, and loading the scratch file await page.goto(`file://${indexHTML}`); const fileInput = await page.$('#file'); - await fileInput.uploadFile(testDir(file)); + await fileInput.setInputFiles(testDir(file)); await page.evaluate(() => // `loadFile` is defined on the page itself. @@ -24,8 +22,7 @@ const runFile = async (file, action, page, script) => { // immediately invoked async function to let us wait for each test to finish before starting the next. (async () => { - await downloadChromium(); - const browser = await puppeteer.launch(); + const browser = await chromium.launch(); const page = await browser.newPage(); const testOperation = async function (name, action, expect) { diff --git a/test/integration/scratch-tests.js b/test/integration/scratch-tests.js index 9d3188a37..bad0774d2 100644 --- a/test/integration/scratch-tests.js +++ b/test/integration/scratch-tests.js @@ -1,11 +1,9 @@ /* global vm, Promise */ -const puppeteer = require('puppeteer-core'); +const {chromium} = require('playwright-chromium'); const test = require('tap').test; const path = require('path'); const fs = require('fs'); -const downloadChromium = require('../helper/download-chromium'); - const indexHTML = path.resolve(__dirname, 'index.html'); const testDir = (...args) => path.resolve(__dirname, 'scratch-tests', ...args); @@ -13,7 +11,7 @@ const testFile = (file, page) => test(file, async t => { // start each test by going to the index.html, and loading the scratch file await page.goto(`file://${indexHTML}`); const fileInput = await page.$('#file'); - await fileInput.uploadFile(testDir(file)); + await fileInput.setInputFiles(testDir(file)); await page.evaluate(() => // `loadFile` is defined on the page itself. // eslint-disable-next-line no-undef @@ -107,8 +105,7 @@ const testFile = (file, page) => test(file, async t => { // immediately invoked async function to let us wait for each test to finish before starting the next. (async () => { - await downloadChromium(); - const browser = await puppeteer.launch(); + const browser = await chromium.launch(); const page = await browser.newPage(); const files = fs.readdirSync(testDir()) From b873b6fec49e89b8a58949e8c2a09f5d43e12505 Mon Sep 17 00:00:00 2001 From: adroitwhiz Date: Thu, 7 May 2020 16:37:17 -0400 Subject: [PATCH 6/6] Bump Node to 10 --- .travis.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index f3da52b33..1a04bcbe1 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ language: node_js dist: trusty node_js: -- 8 +- 10 - node env: - NODE_ENV=production @@ -20,7 +20,7 @@ jobs: - npm run docs - npm run tap - stage: deploy - node_js: 8 + node_js: 10 script: npm run build before_deploy: - VPKG=$($(npm bin)/json -f package.json version)