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

Syntax error in source, causes fatal error #63

Open
mryellow opened this issue Sep 17, 2015 · 5 comments
Open

Syntax error in source, causes fatal error #63

mryellow opened this issue Sep 17, 2015 · 5 comments

Comments

@mryellow
Copy link

Mostly a workflow issue when running under gulp. When running "progress" only, any syntax errors are spat out in console and watch gulp task continues to run. However when "junit" is added to the reporters the same situation will fail on att of undefined and watch will be exited.

Would checking for suite before attempting to use it be an acceptable fix? Go ahead and PR?

Same lack of validation as #16 and karma-runner/karma-coverage#70, the existing fix #17 seems to have been lost at some stage.

@mryellow
Copy link
Author

Current implementation of old fix to onBrowserComplete:

if (!suite || !result) {
return // don't die if browser didn't start
}

Same idea in writeXmlForBrowser:

if (!xmlToOutput) {
return // don't die if browser didn't start
}

Missing from initliazeXmlForBrowser.

Think browser.id in onBrowserComplete may need checking also.

Fixes are missing from onBrowserComplete here https://github.com/karma-runner/karma-junit-reporter/blob/v0.3.4/index.js#L74

@martinmicunda
Copy link

+1 for fix

@osherx
Copy link

osherx commented Sep 20, 2015

+1

@mryellow
Copy link
Author

Not sure it needs a PR, linking master branch without commit hash, looks like it has everything:

https://github.com/karma-runner/karma-junit-reporter/blob/master/index.js#L44-L46

https://github.com/karma-runner/karma-junit-reporter/blob/master/index.js#L78-L80

This one missing, could probably do with a check here too:

https://github.com/karma-runner/karma-junit-reporter/blob/master/index.js#L31

Where tag v.0.3.4 is missing these:

https://github.com/karma-runner/karma-junit-reporter/blob/v0.3.4/index.js

Seems onBrowserComplete check was removed in this commit, then later fixed and not merged back in:

e4f7ebd#diff-168726dbe96b3ce427e7fedce31bb0bc

Can't find when the writeXmlForBrowser bit came along or was removed, a whole bunch of back and forth recently with filename stuff in that one.

@theBull
Copy link

theBull commented Dec 2, 2016

+1

Any luck with a fix for this?

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

No branches or pull requests

5 participants