Fix "Cannot read property 'length' of undefined"

[killzoner]

See #222

[codecov]

Codecov Report

Merging #223 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #223   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          14     14           
  Lines         450    450           
  Branches       61     61           
=====================================
  Hits          450    450

Impacted Files	Coverage Δ
src/spec-reporter.ts	100% <100%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fb467c5...d785cd7. Read the comment docs.

[bcaudan]

Could we expose the failing case with a unit test?

[killzoner]

Sure i'll try to get a repro

[brandonaaskov]

Here's the output from when I got the issue (again, rolling back to 4.1.1 worked for me):

50 specs, 0 failures
Finished in 2.08 seconds
/mnt/jenkins/jobs/my-project/workspace/node_modules/jasmine-spec-reporter/built/spec-reporter.js:53
        if (runDetails.failedExpectations.length) {
                                         ^

TypeError: Cannot read property 'length' of undefined
    at SpecReporter.jasmineDone (/mnt/jenkins/jobs/my-project/workspace/node_modules/jasmine-spec-reporter/built/spec-reporter.js:53:42)

[brandonaaskov]

I actually was able to reproduce this locally using node 7.10.0, but... how can we cover this with a unit test? This is something that happens in the jasmineDone() method when nothing fails. If you can point me in the right direction, @bcaudan, I can try and help.

[bcaudan]

IMO, this fix seems legit.
However, I would like to understand the cause of this issue to be sure that this fix is sufficient.
It could be:

• a specific case that is not covered by unit tests or integration tests
• a behavior with an older version of jasmine

Are you able to reproduce this behavior with a corresponding example:

• examples/node
• examples/protractor

[killzoner]

For now i'm not being able to get a repro. Currently working on Node v6.10.3, i tried with my current config and your tests but no success (no success to fail i mean...):

• protractor 4.0.14
• jasmine-reporters
• protractor-html-reporter

And the current config (i'm working with local PhantomJS setup) :

let SpecReporter = require('jasmine-spec-reporter').SpecReporter;
let jasmineReporters = require('jasmine-reporters');
var HTMLReport = require('protractor-html-reporter');
// https://github.com/angular/protractor/issues/1451
require('protractor/built/logger').Logger.logLevel = 1;

exports.config = {
  baseUrl: 'http://localhost:3000/',
  framework: 'jasmine2',
  seleniumServerJar : 'node_modules/selenium-server-standalone-jar/jar/selenium-server-standalone-2.53.1.jar',
  directConnect: false,
  jasmineNodeOpts: {
    showTiming: true,
    showColors: true,
    isVerbose: false,
    includeStackTrace: false,
    defaultTimeoutInterval: 400000,
    print: function () {
    }
  },
  allScriptsTimeout: 110000,
  specs: [
    './spec/protractor-spec.js'
  ],
  multiCapabilities: [
        {
            browserName: 'phantomjs',
            'phantomjs.binary.path': require('phantomjs-prebuilt').path,
            'phantomjs.cli.args': ['--ignore-ssl-errors=true', '--web-security=false']
        }
  ],
  onPrepare: function () {
    browser.ignoreSynchronization = true;

    /**
     * Additional console reporter
     */
    jasmine.getEnv().addReporter(new SpecReporter({
      spec: {
        displayPending: false
      }
    }));

    /**
     * XML report
     */
    jasmine.getEnv().addReporter(new jasmineReporters.JUnitXmlReporter({
        consolidateAll: true,
        savePath: 'qa/e2e',
        filePrefix: 'report'
    }));
  },

  onComplete: function() {
    var capsPromise = browser.getCapabilities();

    /**
     * HTML report
     */
    capsPromise.then(function (caps) {
        var testConfig = {
            outputPath: 'qa/e2e'
        };
        new HTMLReport().from('qa/e2e/report.xml', testConfig);
    });
  },
};

I can't think of another setup difference that would make this happen. @brandonaaskov, anything which sounds familiar in this setup?
BTW, i don't think it's related to Node version because i also got this failing on a Node 5.1.1 CI.

[johnjbarton]

I believe the issue here is the underlying version of jasmine (not node). This PR is not needed for jasmine 2.7.0 (current dependency in jasmine-spec-reporter) but it is needed for eg jasmine 2.4.1.

[killzoner]

Well, was not around for a while but glad the reason was found. Thanks!