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 the ability to ignore errors for files required with --all #172

Closed
kentcdodds opened this issue Feb 25, 2016 · 2 comments
Closed

Add the ability to ignore errors for files required with --all #172

kentcdodds opened this issue Feb 25, 2016 · 2 comments

Comments

@kentcdodds
Copy link
Member

I understand if you're not interested in supporting this use case, but I thought I'd ask. I have a codebase that's bundled with webpack and we utilize aliases and modulesDirectories and whatnot that sorta break the rules of CommonJS. Also, some of these files depend on DOM globals (like getComputedStyle for example) that are not exposed in the node environment in which we're running our tests.

While we work out what to do about these things, we'd still like to get code coverage reporting on those files. However, using the --all flag makes things fail because of what I mentioned above.

So is there a chance that we could make files required by the --all flag ignore errors. I've successfully implemented this myself by adding a single test:

/*
 * This file's purpose is to require all the files
 * to make sure that everything we care to track
 * coverage for is instrumented. 
 */
import test from 'ava'
import { resolve } from 'path'
import glob from 'glob'

const badPaths = [
    'app.js',
    'App.js',
    '__tests__',
    'vendor/',
]

const globToCover = resolve(__dirname, '../..') + '/js/**/*.js'

const filesToCover = glob
    .sync(globToCover)
    .filter((filepath) => {
        return badPaths.every((file) => !filepath.includes(file))
    })

filesToCover.forEach((file) => {
    try {
        require(file)
    } catch (error) {
        // ignore the error.
        // our purposes of getting the file
        // insturmented for coverage have been accomplished
        // by simply requiring the file.
        // if you're curious, you could log out the filename
        // here to see which files break CommonJS rules or
        // depend on browser globals :-)
    }
})

test('covered files', t => {
    // if we don't have any filesToCover, then something's probably wrong...
    t.notSame(filesToCover.length, 0)
})

This works out pretty well, but I'd love to have this just built into how --all works. Thoughts 💭

@novemberborn
Copy link
Contributor

I guess this line throws? https://github.com/bcoe/nyc/blob/f417897cc7cbde397dd82240003b5bd509304cbb/index.js#L202:L205

The module needs to execute so the Istanbul instrumentation can track it. I wonder if Istanbul should wrap everything in a try/catch so in can report modules that fail to run. Alternatively nyc could catch the error. Not sure if it could still include it in the coverage report or if this needs warning output.

@kentcdodds
Copy link
Member Author

I no longer use nyc so I'll go ahead and close this 👍

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

No branches or pull requests

2 participants