Skip to content

Conversation

@aduth
Copy link
Member

@aduth aduth commented Mar 23, 2017

Continuing attempt to enable accurate testing on a Webpack-generated bundle (previously #289 (comment), #303, #301 (comment) ).

This pull request updates the Webpack configuration to create a separate testable bundle when NODE_ENV=test. This allows us to test files which include loaders like the pegjs-loader introduced in #301.

@aduth aduth requested review from nylen and youknowriad March 23, 2017 14:11
@nylen
Copy link
Member

nylen commented Mar 23, 2017

Why the move from registration.js back to index.js? (I also missed where this was moved in the first place.)

@aduth
Copy link
Member Author

aduth commented Mar 23, 2017

registration.js was only moved away from index.js in #301 as a workaround of sorts to the fact that the tests previously failed when trying to import index.js (because of the .pegjs import).

@nylen
Copy link
Member

nylen commented Mar 23, 2017

Ahh I see. I haven't tested it yet but I support the approach here. Can we merge #308 first?

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

I suspect this to be a bit slower, but definitely better to have a unique build workflow

@aduth aduth force-pushed the update/webpack-mocha branch from 8884d12 to 115f064 Compare March 23, 2017 14:44
@aduth
Copy link
Member Author

aduth commented Mar 23, 2017

Rebased and found there was some issue with Chai plugins not being respected. Updated to reincorporate the bootstrap file, working well again. Will merge after CI passes.

@aduth aduth merged commit c18d4f7 into master Mar 23, 2017
@aduth aduth deleted the update/webpack-mocha branch March 23, 2017 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants