WIP: Build and run tests with webpack #770

bcheidemann · 2021-03-21T13:42:27Z

Adds the option to build and run tests with webpack using the command npm run test:webpack. (Similar to npm run test:manual in that it spins up a local server)

bcheidemann · 2021-03-21T13:57:39Z

@humphd Is this the kind of thing you were talking about?

codecov-io · 2021-03-21T14:11:41Z

Codecov Report

Merging #770 (05ea9e7) into master (c0a41d6) will increase coverage by 0.16%.
The diff coverage is 96.66%.

@@            Coverage Diff             @@
##           master     #770       /-   ##
==========================================
  Coverage   86.89%   87.06%    0.16%     
==========================================
  Files          16       20        4     
  Lines        1740     1770       30     
==========================================
  Hits         1512     1541       29     
- Misses        228      229        1

Impacted Files	Coverage Δ
shims/fs.js	`95.83% <95.83%> (ø)`
shims/buffer.js	`100.00% <100.00%> (ø)`
shims/path.js	`100.00% <100.00%> (ø)`
shims/providers/default.js	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f8c9732...05ea9e7. Read the comment docs.

humphd · 2021-03-21T17:58:42Z

package.json

@@ -46,14  47,17 @@
  },
  "dependencies": {
    "es6-promisify": "^6.1.0",
-    "minimatch": "^3.0.4"
+    "minimatch": "^3.0.4",
+    "node-polyfill-webpack-plugin": "^1.0.3",


These additional deps look like they can move to devDependencies

Yeah, I'll move those over 👍

humphd · 2021-03-21T17:59:18Z

tests/spec/shims/buffer.spec.js

@@ -4,7  4,7 @@ const bufferDefault = require('../../../shims/buffer').default;
 const bufferNamed = require('../../../shims/buffer').Buffer;


Let's rebase on master to pick up these changes there vs. duplicating.

humphd · 2021-03-21T22:42:20Z

tests/webpack.config.js

+  entry: path.resolve(__dirname, './webpack-tests.js'),
+  resolve: {
+    alias: {
+      'fsProvider': path.resolve(__dirname, '../shims/providers/default'),


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.

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.

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.

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?

@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.

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!

This was done so the sister webpack build can use the same mocha setup script

humphd · 2021-04-06T00:21:40Z

Ping me when you want reviews on these.

bcheidemann · 2021-04-06T13:50:56Z

No problem 👍

bcheidemann · 2021-04-18T14:58:21Z

Ping me when you want reviews on these.

I will come back to these after #771 is merged as I think it makes sense to use the webpack plugin to build these tests. Also, if we move the plugin to it's own repo (as discussed in #771) would you want to move these tests to that repo or leave them here?

humphd · 2021-04-18T15:46:37Z

I think doing it in this repo is probably the best idea, since it has CI setup already, and is so closely related to the other code. I'm more wondering about reducing the bundle size for Filer for cases where people aren't using the webpack plugin. We could write a separate set of build/test scripts to be run with npm-run-all or something that builds it beside vs. as part of Filer.

humphd

One nit and the lock file needs to be regenerated, otherwise looks good.

humphd · 2021-05-26T13:52:41Z

package.json

@@ -46,14  47,17 @@
  },
  "dependencies": {
    "es6-promisify": "^6.1.0",
+    "path": "^0.12.7",


Remove this, it's a node built-in module.

bcheidemann mentioned this pull request Mar 21, 2021

docs: correct error in buffer shim documentation #768

Merged

humphd requested changes Mar 21, 2021

View reviewed changes

bcheidemann added 10 commits March 26, 2021 15:32

test: move require chai to top of file for consistency with other tests

4ceca38

chore: npm install

e3b9c57

test: refactor index.html to include mocha setup as an external script

3e2f780

This was done so the sister webpack build can use the same mocha setup script

test: enable building tests with webpack as well as parcel

a52f9aa

chore: npm install webpack dependancies

36c3d18

test: add 'npm run test:webpack' command

adccafd

chore: add tests/dist-webpack to .gitignore

4858694

chore: fix linting issues

57dc4e9

docs: update CONTRIBUTING.md to include documentation for webpack tests

2f4cda1

build: move node-polyfill-webpack-plugin and path to dev dependancies

5857f1a

bcheidemann force-pushed the webpackTestBuild branch from 05ea9e7 to 5857f1a Compare March 26, 2021 15:34

bcheidemann added 4 commits April 3, 2021 18:02

test: update fs spec to import fs module instead of shim

1ee9f1f

test: separate webpack tests so they're not run with the other test

8498460

test: separate shim spec tests from other tests

4c6d94b

fix: make path package a dependancy

463cd3b

bcheidemann changed the title ~~Build and run tests with webpack~~ WIP: Build and run tests with webpack Apr 4, 2021

humphd requested changes May 26, 2021

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Build and run tests with webpack #770

WIP: Build and run tests with webpack #770

bcheidemann commented Mar 21, 2021 •

edited

Loading

bcheidemann commented Mar 21, 2021

codecov-io commented Mar 21, 2021 •

edited

Loading

humphd Mar 21, 2021

bcheidemann Mar 22, 2021

humphd Mar 21, 2021

humphd Mar 21, 2021

bcheidemann Mar 22, 2021

humphd Mar 24, 2021

bcheidemann Apr 1, 2021

humphd Apr 2, 2021

bcheidemann Apr 3, 2021

humphd commented Apr 6, 2021

bcheidemann commented Apr 6, 2021

bcheidemann commented Apr 18, 2021

humphd commented Apr 18, 2021

humphd left a comment

humphd May 26, 2021

		@@ -4,7 4,7 @@ const bufferDefault = require('../../../shims/buffer').default;
		const bufferNamed = require('../../../shims/buffer').Buffer;

WIP: Build and run tests with webpack #770

Are you sure you want to change the base?

WIP: Build and run tests with webpack #770

Conversation

bcheidemann commented Mar 21, 2021 • edited Loading

bcheidemann commented Mar 21, 2021

codecov-io commented Mar 21, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

humphd commented Apr 6, 2021

bcheidemann commented Apr 6, 2021

bcheidemann commented Apr 18, 2021

humphd commented Apr 18, 2021

humphd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bcheidemann commented Mar 21, 2021 •

edited

Loading

codecov-io commented Mar 21, 2021 •

edited

Loading