Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code

Side by Side Diff: test_runner.js

Issue 29517687: Issue 5079, 5516 - Use webpack for browser tests, modules for content scripts (Closed)
Patch Set: Addressed Wladimir's and Hubert's initial feedback Created Aug. 17, 2017, 12:36 p.m.
Left:
Right:
Use n/p to move between diff chunks; N/P to move between comments.
Jump to:
View unified diff | Download patch
« no previous file with comments | « test/browser/elemHideEmulation.js ('k') | no next file » | no next file with comments »
Toggle Intra-line Diffs ('i') | Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
OLDNEW
1 /* 1 /*
2 * This file is part of Adblock Plus <https://adblockplus.org/>, 2 * This file is part of Adblock Plus <https://adblockplus.org/>,
3 * Copyright (C) 2006-2017 eyeo GmbH 3 * Copyright (C) 2006-2017 eyeo GmbH
4 * 4 *
5 * Adblock Plus is free software: you can redistribute it and/or modify 5 * Adblock Plus is free software: you can redistribute it and/or modify
6 * it under the terms of the GNU General Public License version 3 as 6 * it under the terms of the GNU General Public License version 3 as
7 * published by the Free Software Foundation. 7 * published by the Free Software Foundation.
8 * 8 *
9 * Adblock Plus is distributed in the hope that it will be useful, 9 * Adblock Plus is distributed in the hope that it will be useful,
10 * but WITHOUT ANY WARRANTY; without even the implied warranty of 10 * but WITHOUT ANY WARRANTY; without even the implied warranty of
11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the 11 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
12 * GNU General Public License for more details. 12 * GNU General Public License for more details.
13 * 13 *
14 * You should have received a copy of the GNU General Public License 14 * You should have received a copy of the GNU General Public License
15 * along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>. 15 * along with Adblock Plus. If not, see <http://www.gnu.org/licenses/>.
16 */ 16 */
17 17
18 /* eslint-env node */ 18 /* eslint-env node */
19 19
20 "use strict"; 20 "use strict";
21 21
22 const fs = require("fs"); 22 const fs = require("fs");
23 const path = require("path"); 23 const path = require("path");
24 const url = require("url");
25 24
26 const nodeunit = require("nodeunit"); 25 const nodeunit = require("nodeunit");
26 const webpack = require("webpack");
27 const MemoryFS = require("memory-fs");
Wladimir Palant 2017/08/17 13:06:10 Nit: sort the imports alphabetically?
kzar 2017/08/17 13:26:42 Well I did but case sensitively, I considered A to
Wladimir Palant 2017/08/18 20:49:56 Actually, people usually sort by module name, not
kzar 2017/08/21 09:07:01 Done.
27 28
28 const chromiumProcess = require("./chromium_process"); 29 const chromiumProcess = require("./chromium_process");
29 30
30 let unitFiles = []; 31 let unitFiles = [];
31 let browserFiles = []; 32 let browserFiles = [];
32 33
33 function addTestPaths(testPaths, recurse) 34 function addTestPaths(testPaths, recurse)
34 { 35 {
35 for (let testPath of testPaths) 36 for (let testPath of testPaths)
36 { 37 {
37 let stat = fs.statSync(testPath); 38 let stat = fs.statSync(testPath);
38 if (stat.isDirectory()) 39 if (stat.isDirectory())
39 { 40 {
40 if (recurse) 41 if (recurse)
41 { 42 {
42 addTestPaths(fs.readdirSync(testPath).map( 43 addTestPaths(fs.readdirSync(testPath).map(
43 file => path.join(testPath, file))); 44 file => path.join(testPath, file)));
44 } 45 }
45 continue; 46 continue;
46 } 47 }
47 if (path.basename(testPath).startsWith("_")) 48 if (path.basename(testPath).startsWith("_"))
48 continue; 49 continue;
49 if (path.extname(testPath) == ".js") 50 if (path.extname(testPath) == ".js")
50 { 51 {
51 if (testPath.split(path.sep).includes("browser")) 52 if (testPath.split(path.sep).includes("browser"))
52 browserFiles.push(testPath); 53 {
54 browserFiles.push(
55 path.relative(path.join(__dirname, "test", "browser"),
56 testPath).replace(/\.js$/, "")
57 );
Wladimir Palant 2017/08/17 13:06:10 I'd prefer to have this "path to module name" conv
kzar 2017/08/17 13:26:42 Done.
58 }
53 else 59 else
54 unitFiles.push(testPath); 60 unitFiles.push(testPath);
55 } 61 }
56 } 62 }
57 } 63 }
58 64
59 function getFileURL(filePath) 65 function webpackInMemory(bundleFilename, options)
Wladimir Palant 2017/08/17 13:06:10 Do we need the bundleFilename parameter? It seems
kzar 2017/08/17 13:26:42 Well it just means that when things go wrong the e
60 { 66 {
61 return url.format({ 67 return new Promise((resolve, reject) =>
62 protocol: "file", 68 {
63 slashes: "true", 69 // Based on this example https://webpack.js.org/api/node/#custom-file-system s
Wladimir Palant 2017/08/17 13:06:10 Nit: overlong line, move URL to next line?
kzar 2017/08/17 13:26:42 Done.
64 pathname: path.resolve(process.cwd(), filePath).split(path.sep).join("/") 70 let memoryFS = new MemoryFS();
71
72 options.output = {filename: bundleFilename, path: "/"};
73 let webpackCompiler = webpack(options);
74 webpackCompiler.outputFileSystem = memoryFS;
75
76 webpackCompiler.run((err, stats) =>
77 {
78 // Error handling is based on this example
79 // https://webpack.js.org/api/node/#error-handling
80 if (err)
81 {
82 let reason = err.stack || err;
83 if (err.details)
84 reason += "\n" + err.details;
85 reject(reason);
86 }
87 else if (stats.hasErrors())
88 reject(stats.toJson().errors);
89 else
90 {
91 let bundle = memoryFS.readFileSync("/" + bundleFilename, "utf-8");
92 memoryFS.unlinkSync("/" + bundleFilename);
93 resolve(bundle);
94 }
95 });
65 }); 96 });
66 } 97 }
67 98
68 function runBrowserTests() 99 function runBrowserTests()
69 { 100 {
70 if (!browserFiles.length) 101 if (!browserFiles.length)
71 return; 102 return;
72 103
73 // Navigate to this directory because about:blank won't be allowed to load 104 let nodeunitPath = path.join(__dirname, "node_modules", "nodeunit",
74 // file:/// URLs. 105 "examples", "browser", "nodeunit.js");
75 let initialPage = getFileURL(__dirname); 106 let bundleFilename = "bundle.js";
76 let bootstrapPath = path.join(__dirname, "test", "browser", 107
77 "_bootstrap.js"); 108 return webpackInMemory(bundleFilename, {
78 let nodeunitPath = path.join( 109 entry: path.join(__dirname, "test", "browser", "_bootstrap.js"),
79 path.dirname(require.resolve("nodeunit")), 110 module: {
80 "examples", "browser", "nodeunit.js" 111 rules: [{
81 ); 112 resource: nodeunitPath,
82 let args = [ 113 // I would have rathered used exports-loader here, to avoid treating
Wladimir Palant 2017/08/17 13:06:10 "rathered" => "rather"?
kzar 2017/08/17 13:26:42 Done.
83 getFileURL(nodeunitPath), 114 // nodeunit as a global. Unfortunately the nodeunit browser example
84 ...browserFiles.map(getFileURL) 115 // script is quite slopily put together, if exports isn't falsey it
85 ]; 116 // breaks! As a workaround we need to use script-loader, which means
86 return chromiumProcess(initialPage, bootstrapPath, args); 117 // that exports is falsey for that script as a side-effect.
118 use: ["script-loader"]
119 }]
120 },
121 resolve: {
122 alias: {
123 nodeunit$: nodeunitPath
124 },
125 modules: [path.resolve(__dirname, "lib")]
Wladimir Palant 2017/08/17 13:06:10 We don't need this any more, do we?
kzar 2017/08/17 13:26:43 I think we do, otherwise we'll look for common.js
126 }
127 }).then(bundle =>
128 {
129 return chromiumProcess(bundle, bundleFilename, browserFiles);
130 });
87 } 131 }
88 132
89 if (process.argv.length > 2) 133 if (process.argv.length > 2)
90 addTestPaths(process.argv.slice(2), true); 134 addTestPaths(process.argv.slice(2), true);
91 else 135 else
92 { 136 {
93 addTestPaths( 137 addTestPaths(
94 [path.join(__dirname, "test"), path.join(__dirname, "test", "browser")], 138 [path.join(__dirname, "test"), path.join(__dirname, "test", "browser")],
95 true 139 true
96 ); 140 );
97 } 141 }
98 142
99 Promise.resolve(runBrowserTests()).catch(error => 143 runBrowserTests().catch(error =>
100 { 144 {
101 console.error("Failed running browser tests"); 145 console.error("Failed running browser tests");
102 console.error(error); 146 console.error(error);
103 }).then(() => 147 }).then(() =>
104 { 148 {
105 if (unitFiles.length) 149 if (unitFiles.length)
106 nodeunit.reporters.default.run(unitFiles); 150 nodeunit.reporters.default.run(unitFiles);
107 }); 151 });
OLDNEW
« no previous file with comments | « test/browser/elemHideEmulation.js ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld