Get all files over 80% test coverage

[alistairjcbrown]

There are files which are 80% or under for test coverage on Coveralls, giving them a red label. Providing these files with dedicated unit tests will help solidify their API, as well as bring their coverage up to 100%.
Please see the coverall coverage report: https://coveralls.io/github/webpack/webpack

This issue aims to help co-ordinate any effort to add unit test for these files, with the aim then have all files with higher than 80% coverage when closed off.

1. (65.73%) lib/MultiCompiler.js - Add tests for MultiCompiler #3802 - Merged
2. (70.45%) lib/LibraryTemplatePlugin.js - Add tests for LibraryTemplatePlugin #3835 - Merged
3. (74.19%) lib/optimize/UglifyJsPlugin.js - Add test for UglifyJsPlugin #4127 - Merged
4. (75.00%) lib/webworker/WebWorkerMainTemplatePlugin.js - Add tests for WebWorkerMainTemplatePlugin #3866 - Merged
5. (75.00%) examples/move-to-parent/webpack.config.js - changed example to made coverage pass. It also seemed like the common… #4092 - Merged
6. (75.93%) lib/node/NodeMainTemplatePlugin.js - Add more tests for lib/node/NodeMainTemplatePlugin.js #3999 - Merged
7. (79.49%) lib/node/NodeWatchFileSystem.js - Bump up test coverage on NodeWatchFileSystem test #3837 - Merged
8. (80.00%) lib/HotUpdateChunkTemplate.js - Add tests for HotUpdateChunkTemplate #3867 - Merged
9. (75.68%) lib/ContextReplacementPlugin.js
10. (78.57%) lib/dependencies/ContextDependencyTemplateAsId.js - Add tests for ContextDependencyTemplateAsId #4203
11. (80.00%) lib/EnvironmentPlugin.js - Test - improve EnvironmentPlugin code coverage #4201 - Merged

Add a comment below if you're adding unit tests for one of the files above to prevent duplication of effort, and then link to the PR once it's up.

[alistairjcbrown]

I'm currently working on adding tests for lib/MultiCompiler.js, splitting it out into three separate modules, each of which can be tested.

[wtgtybhertgeghgtwtg]

Wouldn't it be better to update the syntax of these, first?

[alistairjcbrown]

@wtgtybhertgeghgtwtg Adding tests should be independent of any syntax changes (so they can be done in parallel). If the question is related to effort, then we'd need to elaborate on "better" - is updating the syntax a higher priority than adding tests, and if so, why?
For me, having tests in place allows us to be sure that any syntax changes made have not caused any changes in functionality. The nine files above are the ones least tested, and therefore the ones most likely to introduce a bug if changed.

[wtgtybhertgeghgtwtg]

Syntax change tends to alter coverage, usually negatively. But you're right, testing should probably be done before syntax change.

[alistairjcbrown]

I'm currently working on adding tests for lib/LibraryTemplatePlugin.js

[jtinfors]

Working on test/NodeWatchFileSystem.test.js

[alistairjcbrown]

If you're looking at the low coverage top 10 on coveralls for the latest build, you'll notice two new files <80% coverage that have snuck in. I've got those covered in PR #3839, so the 6 left above are still the ones to tackle next.

[alistairjcbrown]

@jtinfors Thanks - one more struck off the list! 🎉

[alistairjcbrown]

I'm currently working on adding tests for lib/webworker/WebWorkerMainTemplatePlugin.js

[alistairjcbrown]

Note about the NodeWatchFileSystem tests - test run on CI do not include watch tests, so these tests will not be included in the coverage report. This is most likely due to the time required for these tests - perhaps small and quick unit tests could be included, with only costly tests behind the NO_WATCH_TESTS flag?

[alistairjcbrown]

If you're looking at the low coverage top 10 on coveralls for the latest build, you'll notice lib/formatLocation.js with <80% coverage. Tests for this are covered in PR #3862.

[li-kai]

Hello I'm relatively new to this but I am handling lib/optimize/UglifyJsPlugin.js.

[alistairjcbrown]

@li-kai Awesome, thanks! Give me a poke it you need any help with it 👍

[alistairjcbrown]

If you're looking at the low coverage top 10 on coveralls for the latest build, you'll notice two new files with <80% coverage. Tests for these two are covered in PRs #3982 and #3983.

[jure]

I can tackle lib/node/NodeMainTemplatePlugin.js, if that's alright. Just in the webpack groove :)

[alistairjcbrown]

@jure Awesome, thanks for grabbing that and putting a PR up 👍
Gets the NodeMainTemplatePlugin up to 87% coverage and off the sub-80 list 🎉

[jerairrest]

examples/move-to-parent/webpack.config.js is done! 😄

[alistairjcbrown]

@jerairrest Awesome, thanks! Ticked off the list above 🎉

[alistairjcbrown]

@li-kai How are you getting on with testing lib/optimize/UglifyJsPlugin.js? Do you need a hand with anything?

[jerairrest]

I attempted to write a unit test for ContextReplacementPlugin.js but I couldn't wrap my head around it. Is there file similar that I could use as an example?

[li-kai]

@alistairjcbrown I'm getting there! School work caught up for a bit, but the PR will be coming in a day or two.

[alistairjcbrown]

@li-kai No worries, I see there's been a PR up and merged 🎉 - nice work!

[alistairjcbrown]

Hi @jerairrest - it looks like some of the complexity in testing the ContextReplacementPlugin.js apply method will be due to the nested event bindings. You should check out the tests for RequireJsStuffPlugin as it tackles similar nesting. It makes use of the test helpers applyPluginWithOptions.js and PluginEnvironment.js; these helpers give you a way to exposing the event handlers which are defined within the plugin and therefore allows for testing them directly.
Give me a poke if any of that doesn't make sense, or you need some additional pointers.

[jerairrest]

@alistairjcbrown Perfect I'll get crackin' on it.

[fedebertolini]

I've opened a PR adding a couple of EnvironmentPlugin tests :)

[alistairjcbrown]

@fedebertolini Awesome, thanks! I've updated the description 🎉

[alistairjcbrown]

I'll pick up the last outstanding file for tests, lib/dependencies/ContextDependencyTemplateAsId.js.

For anyone looking at this issue and not sure how to contribute, the next plan is to move on and try and get all files up to 85% test coverage (~12 files), so don't be afraid to take a look at the coverage percentages on coveralls and grab one of the files with only ~80% coverage!

[fedebertolini]

@alistairjcbrown I took a look at lib/dependencies/WebpackMissingModule.js test coverage.

Apparently moduleMetaInfo is not being used at all anymore. I searched for moduleMetaInfo in the whole project and no usages were found. I've reviewed the commits log and that function was only used in LabeledModuleDependency.js, a file that was deleted in a14e563

Should I go ahead an open a PR removing that function?

[alistairjcbrown]

@fedebertolini I guess that explains why there's no coverage of that function, if it's not being used!
A PR to remove that function sounds good, thanks 👍

[jerairrest]

PR for ContextReplacementPlugin is in! #4319

[jerairrest]

@alistairjcbrown This issue can probably be resolved now. :)