Support code coverage

[unional]

@felixfbecker I have updated the project to use latest code.
Now all script loading is done by systemjs.

Do you know how to hook into it for code coverage purpose?

[unional]

Bump into one issue: systemjs/systemjs#1744

[felixfbecker]

I don't, I would imagine that the istanbul hooks into require() to instrument required code (but this is just a guess). Have you looked at https://www.npmjs.com/package/systemjs-istanbul-hook?

On a side note, when you wrote this, did you think about just doing a mini-bundle with webpack or browserify and evaluating that in jsdom? Then the whole mini-bundle could probably be instrumented instead of instrumenting on every require.

[unional]

On a side note, when you wrote this, did you think about just doing a mini-bundle with webpack or browserify and evaluating that in jsdom?

I think that will lose the ability to track the individual dependency between files.
ava and jest use that to determine which test to run when a file changes.

In my idea, this instrumentation has two use cases: code coverage and dependency tracking used by test runner.

[unional]

What is your use case?
Are you loading all files needed all at once? Or you need to do dynamic imports?

[unional]

Have you looked at https://www.npmjs.com/package/systemjs-istanbul-hook

That's probably would work. Thanks for the link. 🌷

[felixfbecker]

I never used the watch features of test runners, I always run tests manually (with debugger attached) and filter to the tests I want to run - didn't think about that use case. So for me, support for code coverage > support for watch. Of course it would be great if both worked.

[unional]

I just gave systemjs-istanbul-hook a try. Seems like I can't use it directly. Need to implement it to work with jsdom.

systemjs-istanbul-hook uses fs and path which does not exists inside jsdom.

[unional]

Also, seems like it does not work with [email protected]. Need to figure out a way to hook into the system. May need to create a plugin.

[unional]

I have some update:
systemjs still have some problems in format detection.
I'm trying to use domture on a global namespace project and it detects some files as amd or cjs.

To get around it, all preloadScripts will now be loaded using <script>.
For loading test files and sources, in cjs or esm environment should still be fine in using import().

#23

[felixfbecker]

I have some time I could invest this week, what's the current state?

[unional]

Same as before. Did some investigation and I think we need to implement similar mechanism as systemjs-istanbul-hook but make it work in jsdom.

One approach is store the result in window.__coverage__ and the outside world can sync that to /__coverage__.

Since there can be multiple tests running, multiple writes can occurs in /__coverage__ folder..

Currently, I'm trying to make one improvement: #29

[unional]

Done with #29

Feel free to experiment any idea you have. 🌷

[unional]

@felixfbecker as I have encountered some issues with node resolution in systemjs. I try switching to using webpack and ts-loader to do the bundling and load into jsdom. It seems to work so far.

If I get all my test passes then will release a new version based on that.

Adding istanbul should be easy by then.

[felixfbecker]

That sounds awesome. Do you have a branch for it?

[unional]

It's on the next branch. I have it published under the @next tag as 0.12

[felixfbecker]

I rewrote my tests to use domture (from the next dist tag) and it works. How do see the instrumentation come in? Maybe https://github.com/webpack-contrib/istanbul-instrumenter-loader? I currently don't see a way to provide webpack options to createDomture

[unional]

Yes. Just need to test and add that into the code.

Maybe later I can open up the config for webpack as I was doing for systemjs.

FYI, I'll need to maintain both solutions (systemjs and webpack). Another package of mine global-test-harness, needs the magic from systemjs to work.

Will need to release them in two different names. Any suggestion?

[unional]

On a second thought, the API of the domture instance is the same. The only difference this the configuration.

So I'll keep the same names and use a type property in the config to do the differentiation.

i.e.:

createDomture({
  loader: 'webpack' // 'systemjs'
  ...
})

interface WebpackDomtureConfig {
  loader: 'webpack'
  ...
}
interface SystemJSDomtureConfig {
  loader: 'systemjs'
  ...
}
createDomture(config: WebpackDomtureConfig | SystemJSDomtureConfig) { ... }

[felixfbecker]

@unional could you add a way to pass webpack config? Your proposal looks good to me

[unional]

Sure, will create 1.1 to just expose the config so you can play with it.

I'm doing some validation to see if I can completely drop systemjs.
I'm able to get global-test-harness to use the webpack version and do some of the magic myself.

If that works, then I can simplify the code.

[unional]

1.1 1.2 released. Need to see how to use istanbul-instrumenter-loader properly in this context.

[unional]

FYI you will not see the converge folder created because webpack output is streamed to memfs:
https://github.com/unional/domture/blob/master/src/createWebpackDomture.ts#L84

So in order to get coverage, we need to collect the coverage from each test and merge them together (along with the coverage folder outside when there are tests in the project does not use domture.

[unional]

Do you want to be added as a contributor to domture? I would love to have multiple contributors in my projects so that in case I can't maintain it someone can. 🌷

[felixfbecker]

Yeah feel free to add me. I can't make any commitment but would hack on it when I find the time

[unional]

No problem. Invite sent.

[felixfbecker]

So I just tried it with this config:

const domture = await createDomture({
                    transpiler: 'typescript',
                    rootDir: __dirname,
                    webpackConfig: {
                        module: {
                            rules: [
                                {
                                    test: /\.js$/,
                                    use: {
                                        loader: 'istanbul-instrumenter-loader',
                                    },
                                },
                            ],
                        },
                    },
                })
                dom = await domture.import('./dom') // this is the browser utility under test

Then running it with nyc mocha and I get a coverage report for the dom.js file 🎉
(not exactly sure why that works, I thought I would have to collect domture.window.__coverage__ manually and merge it with global.__coverage__, but nyc is magic).

------------|----------|----------|----------|----------|----------------|
File        |  % Stmts | % Branch |  % Funcs |  % Lines |Uncovered Lines |
------------|----------|----------|----------|----------|----------------|
All files   |    53.22 |    19.13 |    52.38 |    53.22 |                |
 out/util   |    88.52 |    61.36 |       50 |    88.52 |                |
  dom.js    |    88.52 |    61.36 |       50 |    88.52 |... 7,54,86,100 |
 src/util   |     40.7 |    14.43 |    52.94 |     40.7 |                |
  index.tsx |    12.36 |     2.01 |       25 |    12.36 |... 524,529,532 |
  url.tsx   |    71.08 |    53.13 |    61.54 |    71.08 |... 180,181,183 |
------------|----------|----------|----------|----------|----------------|

The only thing I am missing is the source map support (out/util/dom.js should be src/util/dom.ts)

[felixfbecker]

I think this is because of webpack-contrib/istanbul-instrumenter-loader#73

[felixfbecker]

I finally got it to work with Browserify instead of webpack: 🎉 🎉 🎉

export const createTestBundle = async (modulePath: string): Promise<TestBundle> => {
    const bundler = browserify(modulePath, {
        // Generate sourcemaps for debugging
        debug: true,
        // Expose the module under this global variable
        standalone: 'moduleUnderTest',
        // rootDir for sourcemaps
        basedir: __dirname + '/../../src',
    })
    bundler.plugin(tsify, { project: __dirname + '/../../tsconfig.test.json' })

    // If running through nyc, instrument bundle for coverage reporting too
    if (process.env.NYC_CONFIG) {
        bundler.transform(babelify.configure({ plugins: [babelPluginIstanbul], extensions: ['.tsx', '.ts'] }))
    }

    const bundle = await getStream(bundler.bundle())
    return {
        load(): DOMModuleSandbox {
            const jsdom = new JSDOM('', { runScripts: 'dangerously' }) as JSDOM & {
                window: { moduleUnderTest: any }
            }
            jsdom.window.eval(bundle)
            return { window: jsdom.window, module: jsdom.window.moduleUnderTest }
        },
    }
}

[unional]

Nice! Add 'browserify' as another loader?

[felixfbecker]

Yeah that should be possible, but I don't know how we can make the above part of domture while also making it configurable.

Wondering also how many loaders we want to bake into dependencies - it's not that great having to pull down SystemJS, webpack and Browserify. Maybe split into multiple packages.

Another thought I had when doing the above was that in tests there is the desire to run each test in a fresh JSDOM instance, but rebundling on every test is very slow. That's why I separated the two above.

[unional]

Yes, splitting would make sense.
Most likely will use only one of them.

but rebundling on every test is very slow

You probably mean rebuilding (or rebundling, or both. :P).
Yes, it is slow.

I thought about saving the context and inflate it back for each test, but it could be leaky on the DOM side, and it can become fragile when JSDOM upgrades.

let domture
before(() => { domture = new Domture(...) })
afterEach(() => {
  domture.cleanup()
})

test('...', () => {
  await domture.loadScript(...)
  // do test
})

[felixfbecker]

That's basically what I am doing above in my snippet, first bundle in a before(), then call load() in beforeEach().

[unional]

I originally wanted to avoid that to reduce the mental burden of the consumer.
Another way is to provide some internal caching of the bundles.

Apologize for not pushing this forward fast enough. Recently I have been fully occupied by komondor.

[unional]

@felixfbecker I'm hitting on some issue in using JSDOM to run tests.
What do you think about switching the underlying work to chromeless?
I notice you are also a maintainer there.

Or use headless chrome directly, instead of JSDOM or electron

[felixfbecker]

I think that would make debugging a lot harder because you cannot step through the code with a debugger anymore, it would break at the RPC boundary. So you would have to attach a debugger to the headless Chrome and to Node running the test.

I think headless Chrome is great for e2e testing but for unit tests that just need a DOM I prefer JSDOM.

What are the issues you are having?

Btw, Puppeteer is a lot more advanced than Chromeless and developed by Chrome.