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

yarsay tests fail when running with --cache flag #124

Closed
bcoe opened this issue Jan 2, 2016 · 9 comments
Closed

yarsay tests fail when running with --cache flag #124

bcoe opened this issue Jan 2, 2016 · 9 comments
Labels

Comments

@bcoe
Copy link
Member

bcoe commented Jan 2, 2016

@jamestalmage I know this is something that was fixed at one point, but I can't seem to get the lcov report working if --cache is set for yarsay.

Benjamins-MacBook-Pro:yarsay benjamincoe$ nyc report --reporter=lcov
/Users/benjamincoe/bcoe/nyc/node_modules/istanbul/lib/report/html.js:241
            text = structuredText[startLine].text;
                                            ^

TypeError: Cannot read property 'text' of undefined
    at /Users/benjamincoe/bcoe/nyc/node_modules/istanbul/lib/report/html.js:241:45
    at Array.forEach (native)
    at annotateFunctions (/Users/benjamincoe/bcoe/nyc/node_modules/istanbul/lib/report/html.js:224:26)
    at HtmlReport.Report.mix.writeDetailPage (/Users/benjamincoe/bcoe/nyc/node_modules/istanbul/lib/report/html.js:427:9)
    at /Users/benjamincoe/bcoe/nyc/node_modules/istanbul/lib/report/html.js:489:26
    at SyncFileWriter.extend.writeFile (/Users/benjamincoe/bcoe/nyc/node_modules/istanbul/lib/util/file-writer.js:57:9)
    at FileWriter.extend.writeFile (/Users/benjamincoe/bcoe/nyc/node_modules/istanbul/lib/util/file-writer.js:147:23)
    at /Users/benjamincoe/bcoe/nyc/node_modules/istanbul/lib/report/html.js:488:24
    at Array.forEach (native)
    at HtmlReport.Report.mix.writeFiles (/Users/benjamincoe/bcoe/nyc/node_modules/istanbul/lib/report/html.js:482:23)

I rolled back several commits, and it doesn't seem as though this was ever working on master.

@bcoe bcoe added the bug label Jan 2, 2016
@jamestalmage
Copy link
Member

Worked on my machine with 5.1.1 yargs/yarsay#6

@bcoe
Copy link
Member Author

bcoe commented Jan 3, 2016

@jamestalmage mind trying it with a linked copy of nyc on master. I thought we'd fixed this, but I can't seem to get it not to explode.

@jamestalmage
Copy link
Member

yep - it explodes with a linked copy.

@bcoe
Copy link
Member Author

bcoe commented Jan 3, 2016

@jamestalmage for the life of me I can not seem to isolate the change that is causing this exception in yarsay -- it almost seems like a race condition:

  1. I rolled nyc back to 5.1.1 and linked in, was able to successfully output an lcov report a few times.
  2. I moved forward to the next commit, ran tests, and the report again started throwing an exception.
  3. I pulled in your most recent fix, and continue to have problems.

I have not seen things fail unless the --cache flag is set, really confused.

jamestalmage added a commit to jamestalmage/nyc that referenced this issue Jan 3, 2016
@jamestalmage
Copy link
Member

I moved forward to the next commit, ran tests, and the report again started throwing an exception.

718a319 definitely broke things.

I pulled in your most recent fix, and continue to have problems.

#127? That should have fixe what 718a319 broke

@jamestalmage
Copy link
Member

make sure to clear node_modules/.cache and the self coverage files.

@bcoe
Copy link
Member Author

bcoe commented Jan 3, 2016

Oh, didn't know to remove self coverage will give that a shot.

On Saturday, January 2, 2016, James Talmage [email protected]
wrote:

make sure to clear node_modules/.cache and the self coverage files.


Reply to this email directly or view it on GitHub
#124 (comment).

@jamestalmage
Copy link
Member

Regenerating self-coverage should also work. We automatically use self-coverage if it exists. If you pull in a change, and try to use it immediately before generating new self-coverage via node ./build-self-coverage.js (which is part of npm test), then you are actually using covered code from before the merge.

I usually just do npm run clean in nyc to delete the self coverage. Self coverage shouldn't create problems as long as it is up to date, but I'm just paranoid.

It's also annoying when hunting a bug while linked (your changes to the uncovered code will not be used if *.covered.js exists), so my process is usually to just delete it.

@bcoe
Copy link
Member Author

bcoe commented Jan 3, 2016

fixed by: #127

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants