Skip to content
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

Add support for thresholds #9

Merged
merged 2 commits into from May 12, 2013
Merged

Add support for thresholds #9

merged 2 commits into from May 12, 2013

Conversation

larsthorup
Copy link
Contributor

I wanted to have the ability to fail the build when a coverage threshold is not met. This is already possible with Istanbul. This pull request adds this feature when using Istanbul with Grunt. Let me know what you think!

@larsthorup
Copy link
Contributor Author

Thanks! I added thresholds of 100 to Gruntfile.js, which will not produce any warning. However if you manually change them to, say, 200, they will produce warnings. Don't know how we can verify the warning scenario automatically though, let me know if you have a suggestion. Maybe as simple as a seperate target: 'grunt test:shouldfail'?

@maenu
Copy link
Owner

maenu commented Apr 26, 2013

Hm, good point. When I think about it, testing that a warning is produced really not as simple as I thought, since subsequent tasks after a warning are only executed when forced. So the same pattern as used in the already existing integration test can not be applied. Maybe mocking grunt.warn() to set a global flag and check this one afterwards may be possible.

@larsthorup
Copy link
Contributor Author

I've added a test using your suggestion, see my additional commit. Thanks!

@larsthorup
Copy link
Contributor Author

Let me know if there is more I can do to facilitate the acceptance of this pull request. Thanks!

@maenu
Copy link
Owner

maenu commented May 8, 2013

I just couldn't find any time for this project in the last two weeks. But I have the next few days off and this is on the top of my to-do list.

maenu added a commit that referenced this pull request May 12, 2013
@maenu maenu merged commit cb72d52 into maenu:master May 12, 2013
@larsthorup
Copy link
Contributor Author

Excellent. Thank you!

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.

None yet

2 participants