Skip to content

Add test-code-coverage command - #165

Open
mydea wants to merge 1 commit into
ember-cli-code-coverage:masterfrom
mydea:test-code-coverage-command
Open

Add test-code-coverage command#165
mydea wants to merge 1 commit into
ember-cli-code-coverage:masterfrom
mydea:test-code-coverage-command

Conversation

@mydea

@mydea mydea commented Feb 26, 2018

Copy link
Copy Markdown

This PR adds a command ember test-code-coverage. This can be used e.g. in CI environments.

The command can also be used to simply print out the total coverage percentages, e.g. so you don't need to go looking into HTML files for them.

It takes the following parameters:

  • target-lines: The target percentage for lines covered
  • target-functions: The target percentage for functions covered
  • target-statements: The target percentage for statements covered
  • target-branches: The target percentage for branches covered

For example: ember test-code-coverage -target-lines=80 -target-branches=85

You can also configure those in the config/coverage.js file, in their camel cased form, like this:

 module.exports = {
    targetLines: 80,
    targetFunctions: 85.5,
  
    // Other configuration
    coverageEnvVar: 'COV'
  }

These values will then be the defaults when running ember test-code-coverage.

Running the command will output something like this to your console:

+ Lines covered:       60.00%   69.17%
+ Functions covered:   --.--%   72.18%
+ Statements covered:  --.--%   68.93%
- Branches covered:    70.00%   60.16%
- Test coverage check failed

For any type of check where no target is specified, it will simply output the actual value without doing any comparison.

@lstrzebinczyk

Copy link
Copy Markdown

Something like this would be very nice, gitlab can parse tests output and make a good use of coverage data out of the box, but it must be provided by the tests command.

@RobbieTheWagner

Copy link
Copy Markdown
Collaborator

@kategengler @adamjmcgrath what do you guys think about this feature?

Comment thread index.js
return this._inRepoAddons;
},

includedCommands: function() {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this may need to be rebased; there's already an includedCommands hook

@RobbieTheWagner

Copy link
Copy Markdown
Collaborator

@kategengler aside from the need for a rebase, are you in favor of this feature in general?

Comment thread index.js

includedCommands: function() {
return {
'test-code-coverage': require('./lib/commands/test-code-coverage')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I prefer we name it code-coverage:test or code-coverage-test

beforeRun() {
this._super.apply(this, arguments);

// try to read global options from `config/coverage.js`

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Use the util at lib/config.js to get the configuration and and the project's configPath for the location of the config (see https://github.com/kategengler/ember-cli-code-coverage/blob/master/index.js#L142)

@kategengler

Copy link
Copy Markdown
Collaborator

I am uncertain whether this is something we should be doing ourselves, or deferring to nyc

@mydea

mydea commented Apr 24, 2018

Copy link
Copy Markdown
Author

If you decide to go forward with this, I'll gladly make the changes as requested. :)

@RobbieTheWagner

Copy link
Copy Markdown
Collaborator

@kategengler I also think we may want to defer this to nyc. @adamjmcgrath what do you think?

@adamjmcgrath

Copy link
Copy Markdown
Collaborator

My preference would be nyc support and improving that tool, but I can't offer any help with that at the moment, so don't mind if you want to proceed with this solution.

@RobbieTheWagner

Copy link
Copy Markdown
Collaborator

I agree. I think we should perhaps close, in favor of nyc.

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.

6 participants