Skip to content

Add tests for RequireJsStuffPlugin#3702

Merged
TheLarkInn merged 1 commit intowebpack:masterfrom
alistairjcbrown:add-requirejsstuffplugin-tests
Jan 3, 2017
Merged

Add tests for RequireJsStuffPlugin#3702
TheLarkInn merged 1 commit intowebpack:masterfrom
alistairjcbrown:add-requirejsstuffplugin-tests

Conversation

@alistairjcbrown
Copy link
Copy Markdown
Contributor

What kind of change does this PR introduce?

Add tests for RequireJsStuffPlugin

Did you add tests for your changes?

This PR is only tests

If relevant, link to documentation update:

N/A

Summary

Based on the coveralls report, the RequireJsStuffPlugin file has 82% test coverage. There is no RequireJsStuffPlugin specific test file - this is added in this PR and aims to achieve 100% test coverage.
https://coveralls.io/builds/9498425/source?filename=lib%2FRequireJsStuffPlugin.js

This plugin, although a higher percentage test coverage than other files, was taken on as an example of how to test nested event bindings. This PR extracts the PluginEnvironment helper into a separate file, and makes use of it in order to capture internal event handlers for testing.

Does this PR introduce a breaking change?

No

Other information

None

Copy link
Copy Markdown
Member

@TheLarkInn TheLarkInn left a comment

Choose a reason for hiding this comment

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

This is incredibly insightful and educational about how to cover plugins with sinon, mocking, and spying. Great job.

@TheLarkInn TheLarkInn merged commit b25189a into webpack:master Jan 3, 2017
@alistairjcbrown alistairjcbrown deleted the add-requirejsstuffplugin-tests branch January 3, 2017 20:39
timse pushed a commit to timse/webpack that referenced this pull request Jan 4, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants