Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Reporter fix #285

Merged
merged 9 commits into from Jun 8, 2012

Conversation

Projects
None yet
2 participants
Member

dkowis commented Apr 2, 2012

This pull request depends on cucumber/gherkin#174

This is the cucumber JVM side of things that brings it up to date to work with the changes implemented in the aforementioned pull request.

One thing I'm not sure if it's a consequence of the activity or if more refactoring will be needed:

The build doesn't fail. Maven reports a successful build even though, in my case, every scenario fails (spring context failed to load.) There must be something that's not triggered during the before hook execution to get the scenario execution to remember that it completely bombed. Looking at the previous code, there wasn't anything there in the first place, so it probably only died because the formatter died. I'll look at it again.

David Kowis added some commits Mar 29, 2012

David Kowis Implementing the changes to the Gherkin stuff.
It's reporting the appropriate exceptions now, not failures in the
formatters.
160ba07
David Kowis Merge branch 'master' into reporter_fix d7b86f1
David Kowis Fixing implementation errors regarding newer version of gherkin 0ba6267
David Kowis Making the test happy 7758506
David Kowis Adding this bit of a change ensures that the scenario fails.
My maven build still returns a succesful build though. That doesn't seem right.
b7c965b
Member

dkowis commented Apr 2, 2012

Oh, I didn't bump versions in the pom.xml of either of the pull requests. Wasn't sure exactly how to facilitate that :)

@aslakhellesoy aslakhellesoy and 1 other commented on an outdated diff Apr 10, 2012

...c/main/java/cucumber/formatter/ProgressFormatter.java
@@ -99,6 +100,28 @@ public void result(Result result) {
}
@Override
+ public void before(HookResult result) {
+ handleHook(result);
+ }
+
+ @Override
+ public void after(HookResult result) {
+ handleHook(result);
+ }
+
+ private void handleHook(HookResult result){
+ if(result.getStatus().equals(Result.FAILED)) {
+ if(!monochrome){
+ ANSI_ESCAPES.get(result.getStatus()).appendTo(out);
+ }
+ out.append("H"); //TODO: H for hook?
@aslakhellesoy

aslakhellesoy Apr 10, 2012

Owner

Maybe B for Before and A for After?

@dkowis

dkowis Apr 10, 2012

Member

I thought about that. I figure you won't see these unless there is a failure. And the beginning/end of the list would indicate which hook it is.

I also was concerned that B and A would not be understood to be Before and After, since we use simple . Passing and F Failing for step definitions, not specifically calling out a Given When Then letter.

All that being said, it's easy to change to B/A, and I'm not really opposed to it.

Owner

aslakhellesoy commented May 17, 2012

See the hook-reporting branch (both cucumber-jvm and gherkin) for the latest on this. Does that work for you?

Member

dkowis commented May 17, 2012

I'll take a look at it this evening, I hope. Work is insane right now, and then I've been married for 6 years, so time with the wife wins this weekend :)

Owner

aslakhellesoy commented May 29, 2012

@dkowis I'll try to make new releases with the hook-reporting branches of gherkin and cucumber-jvm this week. If I don't hear anything from you I don't think I'll make any further changes.

Member

dkowis commented May 29, 2012

@aslakhellesoy I'll try to make a point of testing a failing hook on it this week. I've been insanely busy and will be this week and next. Then I have time to breathe again :(

@aslakhellesoy aslakhellesoy merged commit 3f8f171 into cucumber:master Jun 8, 2012

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