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

html reporter errors when encountering sourcemap chunks with typescript #750

Closed
dboune opened this issue Sep 17, 2017 · 3 comments
Assignees
Labels
bug
Milestone

Comments

@dboune
Copy link

@dboune dboune commented Sep 17, 2017

Repro: https://github.com/dboune/lab-chunks

When running lab with --sourcemaps --transform node_modules/lab-transform-typescript in order to run tests written in typescript, if sourcemap "chunks" are encountered, the html reporter (-r html -o coverage.html) will error.

lab-chunks/node_modules/lab/lib/reporters/html.js:210
                    const target = descriptors[originalFilename].source[originalLine];
                                                                ^
TypeError: Cannot read property 'source' of undefined
    at Object.keys.forEach (lab-chunks/node_modules/lab/lib/reporters/html.js:210:65)

Sourcemap chunks appear to occur when code branches.

Poking around I found that a variable descriptors is empty, but chunk processing in the html reporter appears to require it.

The following callback never runs (sourceContents is empty when SourceNode is processing): https://github.com/hapijs/lab/blob/v14.3.0/lib/reporters/html.js#L151

I was able to get the html reporter to complete by wrapping the entire chunks processing block in a test to see if descriptors had anything in it. I do not know whether or not this is an appropriate fix.

https://github.com/hapijs/lab/blob/master/lib/reporters/html.js#L171

        if (Object.keys(descriptors).length) {

            // Now maps coverage information to original files
            Object.keys(file.source).forEach((generatedLine) => {

Hopefully I am not reporting my own mistake :)

Thank you!

@glennjones

This comment has been minimized.

Copy link

@glennjones glennjones commented Sep 17, 2017

Came across the same problem. Had to comment out lines lib/reporters/html.js#L206 to lib/reporters/html.js#L223 For my project the HTML page seem to render without issue with commented section missing.

@dboune

This comment has been minimized.

Copy link
Author

@dboune dboune commented Sep 17, 2017

Looking at it again, can also return [file] if the descriptors aren't generated, as that is the final outcome at the bottom of the function for this case.

        });

        if (!Object.keys(descriptors).length) {
            return [file]
        }

        // Now maps coverage information to original files
        Object.keys(file.source).forEach((generatedLine) => {
@geek geek added the bug label Sep 17, 2017
@Nysosis

This comment has been minimized.

Copy link

@Nysosis Nysosis commented Oct 16, 2017

I've taken a look at this to try and figure out a fix.

I've push some work into a branch on my fix.

There's two issues I've found going on.

  1. Typescript isn't generating the optional sourcesContent property on the source map, which seems to be required at some level to make it all work
  2. The map chunk loading was not correctly resolving the original position for the first chunk of a line that was chunked

I've put fixes in for both of these

  1. If after loading the sourceMap, we don't have any sourcesContent, then attempt to load it from the original file via the filesystem
  2. Look for the original position of the chunk using the SourceMapConsumer.LEAST_UPPER_BOUND bias, instead of the default option (SourceMapConsumer.MOST_LOWER_BOUND)).

However in doing these fixes, it appears to break one of the unit tests in lab:

breaking-test

Right now I can't figure out how to proceed on debugging this test, because when I execute just this test via -g "from external sourcemaps$" the test fails for a different reason (which I'll come on to below). I'm not sure why this is, as each test should be idempotent, so the output of a test shouldn't change as a result of not running other tests (or at least that's the way I build my tests, otherwise this sort of mess can theoretically occur).

In regards to the alternative breaking mechanism, I think there's something not quite right with the source map stuff as a whole. Though putting my fix in works for our particular use case (i.e. the code coverage is reporting and highlighting correctly), there still seems to be some peculiarities with it.

If I fail the above test, on the current master branch (by way of change the expectation on reporters.js#L1510 ) then I can get the expected html, and render that through the browser, which gives me this:

master-coverage

As you can see, the highlight in while.js isn't quite correct, and is also reporting 2 Not covered lines. So I think there may be an underlying issue with the source map processing.

If I run the tests with the -g ... in place, then I can get the code coverage for the same test in the same way, and it gives this output:

fix-coverage

Which has the highlighting in less of a correct position, and still recording 2 Not Covered lines.

adesege added a commit to adesege/lab that referenced this issue Sep 1, 2018
adesege added a commit to adesege/lab that referenced this issue Sep 2, 2018
adesege added a commit to adesege/lab that referenced this issue Sep 2, 2018
adesege added a commit to adesege/lab that referenced this issue Sep 2, 2018
@geek geek added this to the 16.1.1 milestone Oct 22, 2018
@geek geek self-assigned this Oct 22, 2018
@geek geek closed this in #863 Oct 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.