theintern / intern

A next-generation code testing stack for JavaScript.
https://theintern.io/
Other
4.36k stars 310 forks source link

PreExecutor.resolveSuites() failing on IE9 #596

Closed kitsonk closed 8 years ago

kitsonk commented 8 years ago

I think SauceLabs is having some problems, but I got a break in Intern where Proxy.js could be a bit more robust:

TypeError: JSON.parse(...).map is not a function
  at IncomingMessage.<anonymous>  <node_modules/intern/lib/Proxy.js:56:38>
  at emitNone  <events.js:67:13>
  at IncomingMessage.emit  <events.js:166:7>
  at endReadableNT  <_stream_readable.js:893:12>
  at doNTCallback2  <node.js:430:9>
  at process._tickCallback  <node.js:344:17>
rodneyrehm commented 8 years ago

I'm experiencing the same problem. The data handed to JSON.parse() is the following string, which produces an object, not an array - hence the .map is not a function error.

{
  "sequence": 0,
  "sessionId": "<session-id>",
  "payload": [
    "fatalError",
    {
      "name": "SyntaxError",
      "message": "Invalid character",
      "sessionId": "<session-id>"
    }
  ]
}

I have then replaced this chunk with:

var _data = JSON.parse(data);
var messages = Array.isArray(_data) ? _data.map(function (messageString) {
  return JSON.parse(messageString);
}) : [_data];

And saw the following output (instead of that ungraceful stop):

‣ Created session IE9 on WIN (<session-id>)
Suite IE9 on WIN - unit tests FAILED
SyntaxError: Invalid character
No stack or location

<output from functional tests>
<coverage table>

TOTAL: tested 1 platforms, 0/30 tests failed (29 skipped); fatal error occurred
Error: One or more suite errors occurred during testing
  at <node_modules/intern/lib/executors/Executor.js:301:13>
  at <node_modules/intern/browser_modules/dojo/Promise.ts:393:15>
  at runCallbacks  <node_modules/intern/browser_modules/dojo/Promise.ts:11:11>
  at <node_modules/intern/browser_modules/dojo/Promise.ts:317:4>
  at run  <node_modules/intern/browser_modules/dojo/Promise.ts:237:7>
  at <node_modules/intern/browser_modules/dojo/nextTick.ts:44:3>
  at doNTCallback0  <node.js:419:9>
  at process._tickCallback  <node.js:348:13>
rodneyrehm commented 8 years ago

For the record, in my case the error was thrown in a beforeEach callback. I did not dig through that dojo/aspect thing to try to fix this, but maybe an uncaught error in before, after, beforeEach and afterEach should fail specific tests / suites, rather than everything.

kitsonk commented 8 years ago

I suspect I must be having a similar problem, because yes, I am getting almost exactly same thing.

rodneyrehm commented 8 years ago

after rolling back to 75c1472 my tests are running without a hitch again.

kitsonk commented 8 years ago

Yes, I am seeing the same, reverting to 75c1472d6d1e3fcf7b1ed7f92deb14ea8b631a44 resolves the issue. Seems like there is something more wrong.

rodneyrehm commented 8 years ago

@jason0x43 looks like 86193c2 introduced this problem. reproducible with IE9 right after session is created, sadly SyntaxError: Invalid character is not a very useful message…

rodneyrehm commented 8 years ago

I'm closing in on the problem.

The PreExecuter's resolveSuites() is failing because it is executing JSON.parse(undefined). In IE9 response.data is not populated:

// request: request "/node_modules/intern/__resolveSuites__?suites=<suites>

// dump of response in Chrome
{
  "data": "[\"test/unit/alpha\",\"test/unit/bravo\"]",
  "nativeResponse": {},
  "requestOptions": {
    "method": "GET"
  },
  "statusCode": 200,
  "statusText": "OK",
  "url": "/node_modules/intern/__resolveSuites__?suites=<suites>"
}

// dump of reponse in IE9
{
  "nativeResponse": {},
  "requestOptions": {
    "method": "GET"
  },
  "statusCode": 200,
  "statusText": "OK",
  "url": "/node_modules/intern/__resolveSuites__?suites=<suites>"
}

replacing this } else { with } else if (response.data) {avoids the catastrophic failure and my tests are run fine in IE9.

jason0x43 commented 8 years ago

Ah IE, always making life exciting. Thanks for tracking that down.

kitsonk commented 8 years ago

Awesome @rodneyrehm! Thank you!

rodneyrehm commented 8 years ago

and here we have the root cause…

request.data is empty because dojo/request/xhr only returns request.response, but not request.responseText. Replacing response.data = request.response; with response.data = request.response || request.responseText; means IE9 could also make use of the globbed suites…

jason0x43 commented 8 years ago

This should be fixed as of 942baa040a824477f1cda55abd9d029553873db5, which doesn't assume response.data exists (a warning is logged if it's missing) and which updates PreExecutor's error sending code to use the proper data format.

rodneyrehm commented 8 years ago

Thank you @jason0x43! I see you fixed the reason for the JSON.parse() issue on the sender's end - very nice :)

rodneyrehm commented 8 years ago

see the upstream-bug dojo/dojo2#19 for the IE9 not able to use suite resolution issue

jason0x43 commented 8 years ago

The upstream bug was fixed in https://github.com/dojo/dojo2/commit/20a03661c53ec3082db49d081050dd48d7f09cdc.