Skip to content

[JENKINS-29328] NullPointerException parsing a Background section that has a Before hook.#9

Open
stopiccot wants to merge 1 commit intojenkinsci:masterfrom
stopiccot:JENKINS-29328
Open

[JENKINS-29328] NullPointerException parsing a Background section that has a Before hook.#9
stopiccot wants to merge 1 commit intojenkinsci:masterfrom
stopiccot:JENKINS-29328

Conversation

@stopiccot
Copy link
Copy Markdown

After recent update of our testing setup to cucumber-ruby 2.0 jenkins json parsing step started to fail. It turned out to be a known issue with a JIRA task for it. Here is simple fix for that.

@stopiccot stopiccot changed the title JENKINS-29328 [JENKINS-29328] NullPointerException parsing a Background section that has a Before hook. Sep 2, 2016
@jhansche
Copy link
Copy Markdown
Member

@jtnord any objections to merging this?

@jtnord
Copy link
Copy Markdown
Member

jtnord commented Oct 25, 2016

@jtnord any objections to merging this?

Yes - it changes an existing test (by changing the json) that still needs to pass - also not convinced that the UI will be updated accordingly to show this

@stopiccot
Copy link
Copy Markdown
Author

Yeah, I've changed existing json to match json's generated by cucumber-ruby 2.0+, got test failure and then updated code so test passed again. You think we need separate test case for this?

@JeanMertz
Copy link
Copy Markdown

At our company, we just ran into this issue as well. Hoping to see this PR move forward soon.

@bbossola
Copy link
Copy Markdown

bbossola commented Dec 6, 2016

It's really time to have this merged. In the meantime I just rebuild the hpi myself, but it's unfortunate that you are not officially releasing this

@edwardvfluke
Copy link
Copy Markdown

Any update on this merge into master?

@benSlaughter
Copy link
Copy Markdown

@jtnord What are your requirements for this fix?
I have been following for some time, and I have this issue.

Are some extra tests required to cover other use cases?
@bbossola Have you been running the custom HPI with success?

@jtnord
Copy link
Copy Markdown
Member

jtnord commented Feb 14, 2017

@benSlaughter

it changes an existing test (by changing the json) that still needs to pass - also not convinced that the UI will be updated accordingly to show this

There also needs to be a test to show that a failire in a before hook in a background section will correctly report the failing test and ideally render it in the UI (which at the moment I think is unaddressed by this change).

I would also really like the cucumber guys to define the gherkin json output spec and stick to it rather than changing it on a whim and making each tool do different things. The output of the different tools (cucumber-jvm, cucumber-ruby etc is all different....)

@kingdonb
Copy link
Copy Markdown

kingdonb commented Mar 15, 2017

I do have a lot of Background steps, as well as Before hooks and AfterStep hooks, and I do want to know which tests have failed because of problems executing the Background steps. So I agree that this should not be merged as-is.

Thanks for this PR, I've built the plugin from it and subscribed to this issue.

If there is another release without merging a fix for this, would be good to note prominently on the release page that using Cucumber 2.0.1+ with Background steps and before hooks can trigger this.

(Note: this PR did manage to prevent the crash bug for me. 👍 )

@intrigeri
Copy link
Copy Markdown

What are the remaining blockers (if any)?

@kingdonb
Copy link
Copy Markdown

kingdonb commented Apr 18, 2017

Still needs someone to Add a test that shows what happens when a Background step caused a test to fail before the Scenario steps have begun.
(I am fairly sure it will not be reflected correctly in the detail. I don't think it will cause the whole report to fail, I'm not sure if it will accurately reflect a failure)

I can probably add this test myself, but I'm not familiar enough with the Java to get this fixed to work optimally.

@damontgomery
Copy link
Copy Markdown

Are there any updates around this? We're hitting similar issues with recent versions of Cucumber and the plugin.

@kingdonb
Copy link
Copy Markdown

@damontgomery The change was never merged back into a release, because it wasn't really complete. The PR fixes the crash bug though. It might be best to just merge the thing and create a new release from it, even if it's not complete, it fixes a crash bug and it's a bug that probably affects 80% or more who tried using this plugin.

Personally I use the other Cucumber plugin for Jenkins because it didn't have a crash bug. The fix has some serious issues still, but they are not as bad as a crash bug.

(You can build the plugin from the PR branch from @stopiccot if you need to get it installed and get to work right now)

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.