-
Notifications
You must be signed in to change notification settings - Fork 154
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP: Build and run tests with webpack #770
base: master
Are you sure you want to change the base?
Changes from all commits
4ceca38
e3b9c57
3e2f780
a52f9aa
36c3d18
adccafd
4858694
57dc4e9
2f4cda1
5857f1a
1ee9f1f
8498460
4c6d94b
463cd3b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,7 @@ node_modules | |
# Parcel build dirs | ||
.cache | ||
tests/dist | ||
tests/dist-webpack | ||
|
||
# nyc code coverage | ||
.nyc_output | ||
|
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
var mocha = require('mocha'); | ||
|
||
mocha.setup('bdd').timeout(10000).slow(250); | ||
|
||
window.onload = function() { | ||
mocha.checkLeaks(); | ||
mocha.run(); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
// Include the mocha css | ||
require('mocha/mocha.css'); | ||
|
||
// Setup mocha and set it to run tests on window load | ||
require('./setup-mocha'); | ||
|
||
var path = require('../shims/path').default; | ||
console.log(path.isAbsolute('/a/b')); | ||
|
||
// Add any webpack specific tests here e.g. shim tests | ||
require('./spec/shims/fs.spec'); | ||
require('./spec/shims/path.spec'); | ||
require('./spec/shims/buffer.spec'); | ||
|
||
// Add any other new tests to `tests/index.js` | ||
require('./index'); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
const webpack = require('webpack'); | ||
const HtmlWebpackPlugin = require('html-webpack-plugin'); | ||
const NodePolyfillPlugin = require('node-polyfill-webpack-plugin'); | ||
const MiniCssExtractPlugin = require('mini-css-extract-plugin'); | ||
const path = require('path'); | ||
|
||
module.exports = env => ({ | ||
mode: 'development', | ||
entry: path.resolve(__dirname, './webpack-tests.js'), | ||
resolve: { | ||
alias: { | ||
'fsProvider': path.resolve(__dirname, '../shims/providers/default'), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Forgive my webpack ignorance here: can we not use Filer in this setup the way you're outlining in the README? It would be great if we had test code that did what the README suggests, so we know if it breaks. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do you mean aliasing fs, path and Buffer? We can definitely do that but obviously we'd need to refactor the shim tests to require fs, path and Buffer instead of the shims so those tests would have to be separated from the rest of the tests. I think that makes sense though as they're webpack specific. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, that's what I was thinking. Given that there are a few ways to do this, having adequate test coverage of all of them is my goal. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hey @humphd, I'm still working on this PR but found an issue with the way we're shimming the path module. In short, the alias messes up the import of the path module in filer (which I'd previously overlooked) and as such the path module ends up missing a lot of the methods (all the ones not explicitly replaced by filer). It should be possible to write a webpack plugin which resolves the path module correctly. Are you happy for me to go ahead with this and to make a separate PR for it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bcheidemann I think that sounds like a good idea. Thanks for pushing on this code a bit, it's great that testing is already discovering ways we can make it better. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No problem! I'm happy to do it and it's teaching me a lot :D I just wish I had more time to work on it at the moment! |
||
}, | ||
}, | ||
output: { | ||
path: path.resolve(__dirname, './dist-webpack'), | ||
filename: 'index.js', | ||
}, | ||
plugins: [ | ||
new webpack.ContextReplacementPlugin( | ||
// Mocha safely uses require in such a way that webpack cannot statically extract dependancies. | ||
// If the ignoreRequestDependancyExpressionWarnings env is set, we will aggregate these warnings | ||
// into one summary warning to minimise spamming the console. | ||
/\/node_modules\/mocha\/lib/, | ||
(data) => { | ||
if (env.ignoreRequestDependancyExpressionWarnings) { | ||
let requestDependencyExpressionsIgnored = 0; | ||
data.dependencies.forEach((dependancy) => { | ||
if (dependancy.critical === 'the request of a dependency is an expression') { | ||
dependancy.critical = undefined; | ||
requestDependencyExpressionsIgnored += 1; | ||
} | ||
}); | ||
console.log(`WARNING: Ignoring ${requestDependencyExpressionsIgnored} "request of a dependency is an expression" warnings from "node_modules/mocha/lib".`); | ||
} | ||
return data; | ||
}, | ||
), | ||
new NodePolyfillPlugin(), | ||
new MiniCssExtractPlugin(), | ||
new HtmlWebpackPlugin({ | ||
title: 'Filer Tests - Webpack Build', | ||
template: './tests/webpack.index.html', | ||
}), | ||
], | ||
module: { | ||
rules: [ | ||
{ | ||
test: /\.css$/i, | ||
use: [MiniCssExtractPlugin.loader, 'css-loader'], | ||
}, | ||
], | ||
}, | ||
optimization: { | ||
minimize: false, | ||
}, | ||
devtool: 'inline-source-map', | ||
devServer: { | ||
contentBase: path.resolve(__dirname, './dist-webpack'), | ||
} | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
<!DOCTYPE html> | ||
<html> | ||
<head> | ||
<meta charset="utf-8"/> | ||
<title><%= htmlWebpackPlugin.options.title %></title> | ||
</head> | ||
<body> | ||
<div id="mocha"></div> | ||
</body> | ||
</html> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Remove this, it's a node built-in module.