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

`/* `$lab:coverage:(off|on)$` */` disables coverage for a whole module #459

Closed
eriktrom opened this issue Oct 1, 2015 · 14 comments
Assignees
Labels
bug
Milestone

Comments

@eriktrom
Copy link

@eriktrom eriktrom commented Oct 1, 2015

Howdy folks - when using the following:

/* $lab:coverage:off$ */
import Path from 'path'
import Something from './somewhere'
/* $lab:coverage:on$ */


// do lots of things that currently have poor test coverage

I get 100% coverage for the entire module(in both console and html output). When I remove the $lab:coverage:(off|on)$ comments, I get what I expect.

This seems to indicate that /* $lab:coverage:on$ */ does not turn coverage back on for the remaining lines.

(Ancillary note: import Blah from 'blah' causes the babel compiled output to fail coverage for the ternary statement it does for imports , that's a separate issue(but it does exist, fyi). Putting off/on comments around the imports seemed like a way forward for now, but then I hit this wall.)

I am using the following tools/versions:

node/v4.1.1
babel/5.8.23 (babel-core 5.8.25)
lab/6.1.0

Note: issue also occurred on lab 5.x.x, I just upgraded to 6.x before making this issue(w/ same result)
Note: I have not tried on older versions of node

My babel transform function (is mostly copied from the README with some slight alterations):

let Babel = require('babel-core');

module.exports = [{
  ext: '.js',
  transform: function(content, filename) {

    let isNodeModules = filename.split('/').indexOf('node_modules') === 0;
    let isTest = filename.split('/').indexOf('test') === 0;
    let isLib = filename.split('/').indexOf('lib') === 0;

    // Make sure to only transform your code or the dependencies you want
    if (isTest || isLib) {
      let result = Babel.transform(content, {
        sourceMap: 'inline',
        filename: filename,
        sourceFileName: filename
      });
      return result.code;
    } else if (isNodeModules) {
      return content;
    }

    throw Error('babel transform has had some issues, content %o, filename %s', content, filename);
  }
}];

The lab cli command I run is:

./node_modules/lab/bin/lab --sourcemaps --debug --transform ./test/babel-transform.js -a code -t 100 -Lv -r console -o stdout -r html -o coverage.html ./test/unit

Word of warning: Babel and Lab are both new toolz I am playing with, forgive me if there is user fault involved, although I tried to make sure this was actually an issue before submitting one.

Let me know if more information is needed. Tell me where to look and I'll help fix it :)

Thanks and cheers!

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented Oct 1, 2015

Can we see the transpiled code ?

@eriktrom

This comment has been minimized.

Copy link
Author

@eriktrom eriktrom commented Oct 1, 2015

Sure here's the html reporter output: http://eriktrom.s3.amazonaws.com/coverage.html

Let me know if you'd like to see anything else, like a fresh git init of the source code for public consumption.

@eriktrom

This comment has been minimized.

Copy link
Author

@eriktrom eriktrom commented Oct 1, 2015

Also, here's the html coverage output with the coverage off/on blocks in use(for comparison): http://eriktrom.s3.amazonaws.com/coverage-with-on-off-comments.html

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented Oct 3, 2015

Would help to have the full code to debug on our side yes.

@eriktrom

This comment has been minimized.

Copy link
Author

@eriktrom eriktrom commented Oct 5, 2015

Np - I'll make a repo by the morning - went a little buck wild with log stash one day. Have no idea what keys went into git. Will wipe and share. Thanks BTW. Really dig ur guys frameworks.

@eriktrom

This comment has been minimized.

Copy link
Author

@eriktrom eriktrom commented Oct 6, 2015

@Marsup - https://github.com/eriktrom/xyz

I've committed the dist directory so hopefully you won't have to run the app but if you do, it should just work (or actually give you instructions). Let me know of any problems.

EDIT: In this commit eriktrom/xyz@7fb0932 I show the issue at hand(in lib, dist and coverage.html)

@PhilWaldmann

This comment has been minimized.

Copy link

@PhilWaldmann PhilWaldmann commented Feb 26, 2016

are there any updates for that issue?

@Marsup

This comment has been minimized.

Copy link
Member

@Marsup Marsup commented Feb 26, 2016

Sorry, forgot that one. Transpilation seems to remove comments, of course that won't work. Never had any issue with pure node so my guess is the problem will always be in your build tools.

@willin

This comment has been minimized.

Copy link

@willin willin commented Apr 15, 2016

qq20160415-0 2x

v1.1.1

coverage always 100%

@jedireza

This comment has been minimized.

Copy link
Contributor

@jedireza jedireza commented Aug 13, 2016

I think I found out why this may be happening. Checkout PR #620 for more details.

@jedireza

This comment has been minimized.

Copy link
Contributor

@jedireza jedireza commented Aug 15, 2016

@eriktrom lab@11.x was released and included #622, which I think resolves this. Would you be willing to try it out and see?

@eriktrom

This comment has been minimized.

Copy link
Author

@eriktrom eriktrom commented Aug 16, 2016

@jedireza - from a different project, but exact same setup:

./node_modules/.bin/lab --version
11.0.0

image

For reference, the top of the file looks like:

/* $lab:coverage:off$ */
import Model from './';
import _ from 'lodash';
import Promise from 'bluebird';
/* $lab:coverage:on$ */

export default class Listing extends Model {
  constructor() {
    super(...arguments);
  }
  // ...snip

Note that this output is the same with and without the /* $lab:coverage:off$ */ - aka, the 'off' part is still missing from the top of the coverage file, but the 'on' part is there.

If there is good reason too, I can test the old project linked above, but I'm pretty sure everything related to this issue is the same. (except the new version of lab of course)

Thanks a bunch @jedireza and everyone who put some time into debugging this.

Let me know if there's anything more I can do that would help.

EDIT: Maybe worth noting that

/* $lab:coverage:off$ */;import Model from './';
import _ from 'lodash';
import Promise from 'bluebird';
/* $lab:coverage:on$ */

will output this, which is perhaps closer(and maybe??) similar to your pr code - note lab on/off are both shown here, but start below the import statements, and either way the export default class Blah extends Foo seems like its going to be tough to get coverage over. (the class syntax is not this issue but perhaps very related as both the import's and class declaration seem to be prepended to the file before lab evaluates it)

image

@jedireza

This comment has been minimized.

Copy link
Contributor

@jedireza jedireza commented Aug 16, 2016

@eriktrom I'm using babel-core@6.13.2 and defining auxiliary comments to wrap non user generated code.

Try updating your transform to something like:

let result = Babel.transform(content, {
    sourceMap: 'inline',
    filename: filename,
    sourceFileName: filename,
    auxiliaryCommentBefore: '$lab:coverage:off$', // the magic sauce
    auxiliaryCommentAfter: '$lab:coverage:on$' // ;)
});

Using the auxiliary comment options, I tried the same thing you described above locally and it seems to be working as expected.

screenshot from 2016-08-16 08-57-45

@eriktrom

This comment has been minimized.

Copy link
Author

@eriktrom eriktrom commented Aug 16, 2016

@jedireza - that is some very sweet magic sauce - works even without the coverage on/off comments now for me - just straight es6 - your the man, thanks for all the work - i saw your project a while back and was hoping u figure it out :) Sweet!

@jedireza jedireza closed this Aug 16, 2016
@geek geek added this to the 11.0.0 milestone Aug 16, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.