karma-junit-reporter icon indicating copy to clipboard operation
karma-junit-reporter copied to clipboard

Syntax error in source, causes fatal error

Open mryellow opened this issue 9 years ago • 5 comments

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 https://github.com/karma-runner/karma-coverage/issues/70, the existing fix #17 seems to have been lost at some stage.

mryellow avatar Sep 17 '15 02:09 mryellow

Current implementation of old fix to onBrowserComplete: https://github.com/karma-runner/karma-junit-reporter/blob/4c2abd00b99251757b6c53ee539979e4fb9c705f/index.js#L78-L80

Same idea in writeXmlForBrowser: https://github.com/karma-runner/karma-junit-reporter/blob/4c2abd00b99251757b6c53ee539979e4fb9c705f/index.js#L44-L46

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

mryellow avatar Sep 17 '15 02:09 mryellow

+1 for fix

martinmicunda avatar Sep 17 '15 13:09 martinmicunda

+1

osherx avatar Sep 20 '15 08:09 osherx

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:

https://github.com/karma-runner/karma-junit-reporter/commit/e4f7ebdcfd72748df09be04eef344555dd9bc36e#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.

mryellow avatar Sep 20 '15 19:09 mryellow

+1

Any luck with a fix for this?

theBull avatar Dec 02 '16 23:12 theBull