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

TypeError when running karma in singleRun:false and aggregating reporters #8

Closed
1 task done
joeljeske opened this issue Feb 27, 2018 · 8 comments
Closed
1 task done

Comments

@joeljeske
Copy link
Owner

Note: for support questions, please use stackoverflow. This repository's issues are reserved for feature requests and bug reports.

  • **I'm submitting a ... **

    • bug report
  • What is the current behavior?

If I run with singleRun:false and am aggregating a reporter, like junit or istaunbul; then on sequential runs I receive a TypeError.

21 02 2018 10:24:57.816:ERROR [karma]: TypeError: Cannot read property 'HeadlessChrome 0.0.0 (Mac OS X 10.13.3)' of null
    at AggregatedCoverageReporter.onBrowserStart (../node_modules/karma-parallel/lib/reporter.js:103:29)
    at Server.<anonymous> (../node_modules/karma/lib/events.js:13:22)
    at emitTwo (events.js:131:20)
    at Server.emit (events.js:214:7)
    at Browser.onStart (../node_modules/karma/lib/browser.js:133:13)
    at Socket.<anonymous> (../node_modules/karma/lib/events.js:13:22)
    at emitOne (events.js:116:13)
    at Socket.emit (events.js:211:7)
    at ../node_modules/karma/node_modules/socket.io/lib/socket.js:513:12
    at _combinedTickCallback (internal/process/next_tick.js:131:7)
    at process._tickCallback (internal/process/next_tick.js:180:9)
  • What is the expected behavior?

It should not have this error. It should be able to run multiple times without failing.

  • What is the motivation / use case for changing the behavior?

To allow tdd while aggregating reporters.

  • Please tell us about your environment:
  • version: 0.2.4
@ben8p
Copy link
Contributor

ben8p commented Apr 25, 2018

same here...

@ben8p
Copy link
Contributor

ben8p commented Apr 25, 2018

After adding some debug info, I see the following flow:

onRunStart
onBrowserStart
onBrowserStart
onBrowserStart
onBrowserStart
onBrowserStart
onBrowserStart
onRunStart
Reporter :  undefined total   undefined passed   undefined failed   undefined skipped
onRunComplete
onBrowserStart
onBrowserStart
onBrowserStart
onBrowserStart
onBrowserStart
onBrowserStart

@ben8p ben8p mentioned this issue Apr 25, 2018
@joeljeske
Copy link
Owner Author

@ben8p Thanks for the contribution!

I'm trying to understand karma's lifecycle a bit better. Perhaps I am missing something or perhaps karma is a little inconsistent with itself: It appears this is the lifecycle of events for a karma reporter

x = number of browsers
y = number of tests

  1. 1 onRunStart (this is where I initialize that object)
  2. x onBrowserStart
  3. y onSpecComplete
  4. x onBrowserComplete
  5. 1 onRunComplete (this is where I clear that object)

If watching / using TDD, I would expect that if I change a file, I would start over at onRunStart, not starting with onBrowserStart. But if your fix does indeed fix this issue, then perhaps karma is not very consistent by not calling onRunStart for sequential runs.

What do you think?

@ben8p
Copy link
Contributor

ben8p commented Apr 27, 2018

Well,
I have the feeling it has to do with a file changed between step 1 and step 2.
When I look at Nyan cat reporter I see it restart multiple times (and all aborted instance are saying "undefined" instead of the number of tests.
There's probably a sort of life cycle optimization built in karma...
As far as I tested the fix has not side effects...

@ben8p
Copy link
Contributor

ben8p commented May 5, 2018

@joeljeske will you consider merging the PR ? Do you need more inputs ?

@abeymg
Copy link

abeymg commented May 5, 2018

I am running into this issue as well. @joeljeske Can this PR be merged (assuming it fixes the issue) ?

@ben8p
Copy link
Contributor

ben8p commented May 5, 2018

@abeymg maybe you can try the modified code locally and confirm it also fixes the issue you're having?
It is only one single line to change.

@abeymg
Copy link

abeymg commented May 6, 2018

@joeljeske @ben8p I did that and can confirm that it fixed the issue for me.

joeljeske added a commit that referenced this issue May 10, 2018
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

3 participants