Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

render exceptions in conditions as condition failure #49

Closed
wants to merge 2 commits into from

Conversation

Projects
None yet
10 participants
@Fuud
Copy link
Contributor

commented Sep 17, 2014

Idea

Spock has the best exception rendering I have ever see. For example, lets take a spec:

    def "responce code should starts with 200"(){
        when:
        def map = new HashMap<String, String>()
        map.put("responceCode", "404 NOT FOUND")
        then:
        map.get("responceCode").startsWith("200")
    }

It fails and has good rendering:

Condition not satisfied:

map.get("responceCode").startsWith("200")
|   |                   |
|   404 NOT FOUND       false
[responceCode:404 NOT FOUND]

    at org.spockframework.guice.JavaStubs.responce code should starts with 200(JavaStubs.groovy:98)

But how it will be rendered if map miss value for key "responceCode"? For example in this spec:

    def "responce code should starts with 200"(){
        when:
        def map = new HashMap<String, String>()
        map.put("ResponceCode", "404 NOT FOUND")
        then:
        map.get("responceCode").startsWith("200")
    }

just an exception:

java.lang.NullPointerException: Cannot invoke method startsWith() on null object
    at Test.responce code should starts with 200(Test.groovy:8)

we can understand that map does not contains entry for "responceCode" but we can not inspect what map really contains.

With my modifications it will become more friendly:

Condition not satisfied:

map.get("responceCode").startsWith("200")
|   |                   |
|   null                java.lang.NullPointerException: Cannot invoke method startsWith() on null object
[ResponceCode:404 NOT FOUND]

    at org.spockframework.guice.Test.responce code should starts with 200(Test.groovy:11)
Caused by: java.lang.NullPointerException: Cannot invoke method startsWith() on null object
    ... 1 more

So exceptions that occured during evaluating condition become part of rendering.

Implementation

Lets take for example this condition:

map.get("key") == "abc"

Before my changes it will be transformed to (I omit unnecessary parts)

SpockRuntime.verifyCondition(
                valueRecorder.reset(),
                "map.get(\"key\") == \"abc\"",
                8,
                9,
                null,
                valueRecorder.record(
                        6,
                        Boolean.valueOf(
                                ScriptBytecodeAdapter.compareEqual(
                                        valueRecorder.record(4,
                                                ScriptBytecodeAdapter.invokeMethodN(
                                                        Test.class,
                                                        valueRecorder.record(0, map),
                                                        (String)valueRecorder.record(1, "get"),
                                                        [valueRecorder.record(2, "key") ]
                                                )
                                        ),
                                        valueRecorder.record(5, "abc")
                                )
                        )
                )
        );

We need to:

  1. handle exception, so surround this expression with try-catch.
  2. to understand number of value that was not recorded because of exception, so lets remember value number before recording value (and before evaluating expression for this value).
try {
    SpockRuntime.verifyCondition(
            $spock_valueRecorder.reset(), 
            "map.get(\"key\") == \"abc\"", 
            14,
            7,
            null,
            $spock_valueRecorder.record(
                    $spock_valueRecorder.startRecordingValue(6),
                    Boolean.valueOf(
                            ScriptBytecodeAdapter.compareEqual(
                                    $spock_valueRecorder.record(
                                            $spock_valueRecorder.startRecordingValue(4),
                                            ScriptBytecodeAdapter.invokeMethodN(
                                                    ExceptionsInConditionsSpec.class,
                                                    $spock_valueRecorder.record(
                                                            $spock_valueRecorder.startRecordingValue(0),
                                                            map
                                                    ),
                                                    (String) ShortTypeHandling.castToString(
                                                            $spock_valueRecorder.record(
                                                                    $spock_valueRecorder.startRecordingValue(1),
                                                                    "get")
                                                    ),
                                                    [$spock_valueRecorder.record(
                                                            $spock_valueRecorder.startRecordingValue(2),
                                                            "key"
                                                    )]
                                            )
                                    ),
                                    $spock_valueRecorder.record(
                                            $spock_valueRecorder.startRecordingValue(5),
                                            "abc")
                            )
                    )
            )
    );
}
catch (Throwable throwable) {
    SpockRuntime.conditionFailedWithException($spock_valueRecorder, "map.get(\"key\") == \"abc\"", 14, 7, null, throwable);
}

ValueRecorder,startRecording method:

  private final Stack<Integer> startedRecordings = new Stack<Integer>();

  public int startRecordingValue(int index){
    startedRecordings.push(index);
    return index;
  }

So we add value number to stack before evaluating expression for this value. And remove value number from top of stack when value is written to ValueRecording. And when exception is thrown we can learn where exception is thrown by peeking value from top of stack.

@pniederw

This comment has been minimized.

Copy link
Contributor

commented Sep 18, 2014

Thanks a lot for your pull request. I like the idea, and will review over the weekend.

@Fuud

This comment has been minimized.

Copy link
Contributor Author

commented Oct 6, 2014

@pniederw any review?

@jbaruch

This comment has been minimized.

Copy link

commented Oct 26, 2014

👍

@pniederw

This comment has been minimized.

Copy link
Contributor

commented Nov 3, 2014

I've started work on this pull request, and should have it integrated soon. Thanks for your patience.

@PascalSchumacher

This comment has been minimized.

Copy link

commented Jan 12, 2015

very nice enhancement 👍

@Fuud

This comment has been minimized.

Copy link
Contributor Author

commented Mar 11, 2015

@pniederw Any progress?

@leonard84

This comment has been minimized.

Copy link
Member

commented Mar 12, 2015

👍

@Fuud

This comment has been minimized.

Copy link
Contributor Author

commented Apr 21, 2015

@pniederw Any progress?

@jirutka

This comment has been minimized.

Copy link

commented Apr 21, 2015

👍

1 similar comment
@stavytskyi

This comment has been minimized.

Copy link

commented Apr 23, 2015

👍

@Fuud

This comment has been minimized.

Copy link
Contributor Author

commented May 13, 2015

@pniederw Why this pull request was not merged yet? Is anything wrong with code?

@PascalSchumacher

This comment has been minimized.

Copy link

commented Jun 19, 2015

@robfletcher Maybe you can have a look a this pull request (as a spock user this seems like a very nice feature)?

@robfletcher

This comment has been minimized.

Copy link
Contributor

commented Jun 19, 2015

Yeah, this looks excellent. I'm going to merge it if I don't hear a good reason why I shouldn't in the next few hours.

@Fuud

This comment has been minimized.

Copy link
Contributor Author

commented Jun 22, 2015

@robfletcher Did you found "good reason" not to merge?

@Fuud

This comment has been minimized.

Copy link
Contributor Author

commented Sep 3, 2015

Open Question: What to do with invocation of void methods and assignments? Should such constructions became conditions because them can throw an exception.

@PascalSchumacher

This comment has been minimized.

Copy link

commented Nov 13, 2015

Maybe this can be merged before the next release?

@szpak

This comment has been minimized.

Copy link
Contributor

commented Feb 20, 2016

Yeah, this looks excellent. I'm going to merge it if I don't hear a good reason why I shouldn't in the next few hours.

@robfletcher Time flies. Any chance to get it merged in the foreseeable future?

@dpost

This comment has been minimized.

Copy link

commented May 5, 2016

any update on this?

robfletcher added a commit that referenced this pull request May 27, 2016

Merge pull request #549 from Fuud/exceptions-in-condition-as-conditio…
…n-failure__over_spock_master

render exceptions in conditions as condition failure (copy of #49)
@robfletcher

This comment has been minimized.

Copy link
Contributor

commented May 27, 2016

Superseded by #549 which is now merged

@robfletcher robfletcher modified the milestone: 1.1-rc-1 Aug 24, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.