Skip to content

docs(recipe): Add comments for precompiling multiple test files #1385

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

Merged
merged 3 commits into from
Jun 17, 2017
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
241 changes: 236 additions & 5 deletions docs/recipes/precompiling-with-webpack.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,33 @@

Translations: [Français](https://github.com/avajs/ava-docs/blob/master/fr_FR/docs/recipes/precompiling-with-webpack.md)

The AVA [readme](https://github.com/avajs/ava#transpiling-imported-modules) mentions precompiling your imported modules as an alternative to runtime compilation, but it doesn't explain how. This recipe shows how to do this using webpack. (This example uses webpack 2.0)
The AVA [readme](https://github.com/avajs/ava#transpiling-imported-modules) mentions precompiling your imported modules as an alternative to runtime compilation, but it doesn't explain how. This recipe various approaches using webpack. (These examples use webpack 2.0). Multiple approaches are discussed as each has its own pros and cons. You should select the approach that best fits your use case. This might not be necessery once [this](https://github.com/avajs/ava/blob/master/docs/specs/001%20-%20Improving%20language%20support.md) is completed. See the original discussion [here](/avajs/ava/pull/1385).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than:

This recipe various approaches using webpack. (These examples use webpack 2.0).

Perhaps:

This recipe discusses several approaches using webpack v2.

This seems distracting:

This might not be necessery once this is completed.

And the discussion URL should have https://github.com in front of it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems distracting:

Thought it would be better to acknowledge the difficulty of precompiling and mention that a better permanent solution is in the works. Would you prefer I reword it or remove it?

And the discussion URL should have https://github.com in front of it.

I thought it might be better to have relative links. And that / would go down to the root. I'll just change this to use full URL. There is also the option of using avajs/ava#1385. Let me know if you'd prefer it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thought it would be better to acknowledge the difficulty of precompiling and mention that a better permanent solution is in the works. Would you prefer I reword it or remove it?

I'd rather these recipes focus on what's possible now, rather than what might be possible eventually. I mean I wrote that RFC in January… it's not a quick fix 😉

I thought it might be better to have relative links. And that / would go down to the root. I'll just change this to use full URL. There is also the option of using #1385. Let me know if you'd prefer it.

Yea I know. I suppose I just don't trust linking within GitHub that much, plus it means the links won't work if the file is rendered outside of GitHub.


- [Single test file](#single-test-file)
- [Multiple test files](#multiple-test-files)

### Single test file
This is the simplest use case. You might need this if you are [using aliases](https://github.com/avajs/ava/issues/1011).

###### webpack.config.js

```js
const path = require('path');
const nodeExternals = require('webpack-node-externals');

module.exports = {
entry: ['src/tests.js'],
entry: ['tests.js'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This assumes you only have one test file (named tests.js)? Perhaps the file name should be test.js then? (At least that's how I name such a file, analogous to having a test directory.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although there is a single file that's not the same thing as having a single test. I don't mind changing this though.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I hear ya. We do talk about test.js in the readme though.

Alternatively: your-test-file.js?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's go with test.js. Sold on the readme.

target: 'node',
output: {
path: '_build',
path: path.resolve(__dirname, '_build'),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure the sample code needs to prescribe this. What does webpack show in its examples?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to the webpack docs, the target directory for all output files must be an absolute path (use the Node.js path module). You can see this in the code example here - https://webpack.js.org/configuration/

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

filename: 'tests.js'
},
externals: [nodeExternals()],
module: {
rules: [{
test: /\.(js|jsx)$/,
use: 'babel-loader'
test: /\.(js|jsx)$/,
use: 'babel-loader',
options: { cacheDirectory: true }
}]
}
};
Expand All @@ -29,3 +37,226 @@ module.exports = {
The important bits are `target: 'node'`, which ignores Node.js-specific `require`s (e.g. `fs`, `path`, etc.) and `externals: nodeModules` which prevents webpack from trying to bundle external Node.js modules which it may choke on.

You can now run `$ ava _build/tests.js` to run the tests contained in this output.

### Multiple test files
Things are a little more complicated with multiple test files. We recommend [using babel-register](babelrc.md) until the performance penalty becomes too great.

The possible approaches are:

- [Refer precompiled source in tests](#refer-precompiled-source-in-tests)
- [Single entry file](#single-entry-file)
- [Multiple entry files](#multiple-entry-files)
- [Multiple entry files with externalized source files](#multiple-entry-files-with-externalized-source-files)

#### Refer precompiled source in tests
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps Test against precompiled sources?

Source files can be compiled with source maps(for coverage) to `_src` folder and referenced in tests. While this is less than elegant, it performs well and the worklow can be optimized with [webpack's watch mode](https://webpack.js.org/configuration/watch/).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's not bring code coverage into this discussion, since that's a whole other can of worms.

Why _src here versus _build above?

There's a typo in workflow (it's missing the l).

How would users do this precompilation?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why _src here versus _build above?

I was thinking _build for tests and _src for source.

How would users do this precompilation?

I was thinking babel-cli.. I'll change this to mention babel-cli watch mode instead of webpack. Sound good?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking _build for tests and _src for source.

Sure. I'm probably overthinking this 😄

How would users do this precompilation?

I was thinking babel-cli.. I'll change this to mention babel-cli watch mode instead of webpack. Sound good?

👍


```js
// Before
import fresh from '../src';
// After
import fresh from '../_src';
```

#### Single entry file
To pre-compile folder with multiple test files, we can use globbing(see [stackoverflow answer](http://stackoverflow.com/questions/32874025/how-to-add-wildcard-mapping-in-entry-of-webpack/34545812#34545812)). Although this performs quite possibly the best, it comes at a cost. AVA uses the filename and path for test names in verbose mode as well as test failure reports. This can make the error report harder to understand. Also, it can matter if tests modify globals, Node.js built-ins. You wouldn't want that to leak across tests.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about putting it like this?

Multiple test files can be compiled into a single file. This may have the best performance but that does come at a cost. All tests will be in the same file, which can make it harder to know which test has failed, since AVA can't show the file name the test was originally in. You'll also lose process isolation.


###### webpack.config.js

```js
const path = require('path');
const glob = require('glob');

const nodeExternals = require('webpack-node-externals');

module.exports = {
target: 'node',
entry: glob.sync('./test/**/*.js'),
output: {
path: path.resolve(__dirname, '_build'),
filename: 'tests.js'
},
externals: [nodeExternals()],
module: {
rules: [{
test: /\.(js|jsx)$/,
use: {
loader: 'babel-loader',
options: { cacheDirectory: true }
}
}]
}
};
```

<details>
<summary>Error report comparison</summary>

```
# Before
aggregations-test » cardinality-agg » sets precision_threshold option
E:\Projects\repos\elastic-builder\test\_macros.js:167

166: const expected = getExpected(keyName, recursiveToJSON(propValue));
167: t.deepEqual(value, expected);
168: }

Difference:

Object {
my_agg: Object {
cardinality: Object {
- precision_threshol: 5000,
+ precision_threshold: 5000,
},
},
}

# After
sets precision_threshold option
E:\Projects\repos\elastic-builder\_build\tests.js:106

105: column: 21
106: }
107: },

Difference:

Object {
my_agg: Object {
cardinality: Object {
- precision_threshol: 5000,
+ precision_threshold: 5000,
},
},
}

```

</details>

#### Multiple entry files
We can ask webpack to generate multiple entry files. This helps retain file names so that error reports are easy to interpret. But each entry file gets it's own copy of the source files. This results in considerably larger file sizes. This can [perform quite poorly](https://github.com/avajs/ava/pull/1385#issuecomment-304684047) on the first execution.

###### webpack.config.js

```js
const path = require('path');
const glob = require('glob');

const nodeExternals = require('webpack-node-externals');

const testFiles = glob.sync('./test/**/*.js');

const entryObj = {};
const len = testFiles.length;
for (let idx = 0; idx < len; idx++) {
const file = testFiles[idx];
entryObj[path.basename(file, path.extname(file))] = file;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This code can be improved a bit:

const testFiles = glob.sync('./test/**/*.js').reduce((entryObj, file) => {
  entryObj[path.basename(file, path.extname(file))] = file;
  return entryObj;
}, {});

Then use entry: testFiles below.

Copy link
Contributor Author

@sudo-suhas sudo-suhas Jun 10, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I too used reduce initially but felt the for method was easier to understand. The reduce returns the entryObj and not the testFiles though. I'll change it to use reduce.


module.exports = {
target: 'node',
entry: entryObj,
output: {
path: path.resolve(__dirname, '_build'),
filename: '[name].js'
},
externals: [nodeExternals()],
module: {
rules: [{
test: /\.(js|jsx)$/,
use: {
loader: 'babel-loader',
options: { cacheDirectory: true }
}
}]
}
};
```

#### Multiple entry files with externalized source files
This is the most complicated to setup but performs quite well and also retains file names. In this approach, we use the babel cli to compile the src files but preserve file structure. Then we compile the tests with a require path rewrite. The following example is for a specific file structure. Depending on how your source and test files are organised, you might need to make changes.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd replace "babel cli" with babel-cli. Rather than "compile the src file" I'd use "compile the source files". And I'd invert the sentence structure from:

Then we compile the tests with a require path rewrite"

To:

Require paths in tests are rewritten when compiling them in webpack.


File structure:
```
├───src
│ ├───my-pkg-fldr
│ │ ├───my-module.js
│ │ └───index.js
│ └───index.js
└───test
├───my-pkg-fldr
│ └───my-module.test.js
└───index.test.js

# Generated file structure
├───_src
│ ├───my-pkg-fldr
│ │ ├───my-module.js
│ │ └───index.js
│ └───index.js
└───_build
├───my-module.test.js
└───index.test.js
```

npm scripts:
```js
{
"scripts": {
"precompile-src": "cross-env NODE_ENV=test babel src --out-dir _src",
"precompile-tests": "cross-env NODE_ENV=test webpack --config webpack.config.test.js",
"pretest": "npm run precompile-src && npm run precompile-tests",
"test": "cross-env NODE_ENV=test nyc --cache ava _build --concurrency 3"
}
}
```

###### webpack.config.js

Webpack Externals Docs - https://webpack.js.org/configuration/externals/#function

```js
const path = require('path');
const glob = require('glob');

const nodeExternals = require('webpack-node-externals');

const testFiles = glob.sync('./test/**/*.js');

const entryObj = {};
const len = testFiles.length;
for (let idx = 0; idx < len; idx++) {
const file = testFiles[idx];
entryObj[path.basename(file, path.extname(file))] = file;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above.


module.exports = {
target: 'node',
entry: entryObj,
output: {
path: path.resolve(__dirname, '_build'),
filename: '[name].js'
},
externals: [
nodeExternals(),
// Rewrite the require paths to use _src
(context, request, callback) => {
// This is a little messy because tests are not output in original file structure
// test/index.test.js -> _build/index.test.js
// => ../src -> ../_src
// test/my-pkg-fldr/my-module.test.js -> _build/my-module.test.js
// => ../../src -> ../_src
if (request.includes('/src')) {
const requestReqwrite = request
.replace('/src', '/_src')
.replace('../../_src', '../_src');
return callback(null, `commonjs ${requestReqwrite}`);
}
callback();
}
]
};
```