mavdi / grunt-cucumberjs

Grunt plugin for cucumber.js
MIT License
31 stars 36 forks source link

Fails to parse JSON (and create HTML output) on cucumber error #71

Closed shaneobrien20 closed 8 years ago

shaneobrien20 commented 8 years ago

First of all great work on this, very impressive reporting outputs!

However we found 1 issue where the cucumber json output was not being parsed for the html report to be generated. When a test failed with a cucumber error output, it generated the output json in processHandler.js (around line 98) with the error first, followed by the json test results, but when it tried to extract the json test results for generating the html, it incorrectly pulled the errors in as well, causing it to be invalid json and it would give the error: "unable to parse json".

Here is an example of the output:

` Error: Error: This is some error thrown by cucumber. Error occurred on line ........

[ { "id": "This-the-id", "name": "This the name", "description": "A description of the object" }, { "id": "This-the-second-id", "name": "This the second name", "description": "A description of the second object" } ] `

I managed to get it working by changing the regex on line 98 in processHandler.js from this:

var featureStartIndex = jsonOutput.search(/\[\s*{\s*\"\[\"/g);

to this:

var featureStartIndex = jsonOutput.search(/\[\s*{\s*\"/g);

It looks for the start of the json object and extracts it from the output.

The problem seemed to be that it wasnt finding the start of the json based on the regex it was using. It seems to work now and will still generate the html report when there are cucumber errors, showing these errors on the report.

If you can find a better solution then great, but this worked for me.

lenntt commented 8 years ago

I can confirm this issue. I tried the workaround, which works... though I'd prefer a stronger regex (since I'm logging objects and array).

I guess this is the part we want to capture:

[
   {
     "elements": [
       {
          "id": ".......",
          .....
shaneobrien20 commented 8 years ago

Hi SirLenz0rlot, couldn't agree more about stronger regex.

Try this, seems to work for both:

/\[\s*{\s*\"|\[\"/g

All I added was a pipe in the middle there, searching for either object start or array start. It satisfies the above example but would probably need some more thorough testing.

lenntt commented 8 years ago

To make this more reliable, the whole flow of the json should be improved.

I noticed that when I console.log something in a AfterFeatures handler, the json breaks too :S

lenntt commented 8 years ago

The regex I suggested is a bad one by the way. The first property can as well be a "description"

gkushang commented 8 years ago

@shaneobrien20 @SirLenz0rlot thanks for your inputs. I agree about improving on JSON parsing and do not depend on RegEx for future compatibility.

Let's not parse the JSON based on RegEx, instead read the JSON file created at the end of run. Since Cucumber's formatter -f json: fileName.json writes the JSON for HTML reporting, it can be used to create HTML.

gkushang commented 8 years ago

Hi @shaneobrien20 @SirLenz0rlot - PR https://github.com/mavdi/grunt-cucumberjs/pull/75 will fix the JSON parsing issue as we do not depend on RegEx anymore. Would you guys mind to test it out by pulling the module thru "grunt-cucumberjs": "git+ssh://git@github.com/gkushang/grunt-cucumberjs.git#tests" or npm i git+ssh://git@github.com/gkushang/grunt-cucumberjs.git#tests till it's merged and publish?

lenntt commented 8 years ago

I'm afraid it doesn't work for me. jsonOutput doesn't seem like valid json to me, as all the console.log() from the test code gets in there too.

I guess this is caused by reading the entire buffer (jsonOutput = Buffer.concat(buffer).toString(); in processHandler)

lenntt commented 8 years ago

Let's get rid of reading from the buffer!

in cucumber.js:35, couldn't we just replace:

                commands.push('-f', 'json');

by

                commands.push('-f', 'json:' + options.output + '.json' );

Then, I think we should remove the saveJson option (since its always true when enabling html) and in processHandler throw if the json doesnt exist instead of trying to read from buffer.

Edit: working on a PR

gkushang commented 8 years ago

@SirLenz0rlot Can you share your cucumberjs "options"?

gkushang commented 8 years ago

Published @0.10.1. Here is the CHANGELOG