Convert hystrix 1.4 test from groovy to java#9217
Conversation
| testing.waitAndAssertTraces( | ||
| trace -> | ||
| trace.hasSpansSatisfyingExactly( | ||
| span -> span.hasName("parent").hasNoParent(), |
There was a problem hiding this comment.
groovy code also verified that there are no attributes for which in java tests we usually use .hasAttributes(Attributes.empty())
There was a problem hiding this comment.
thank you for pointing this out, i'll be sure to include these going forward
| span.hasName("ExampleGroup.HystrixTest$1.execute") | ||
| .hasParent(trace.getSpan(0)) | ||
| .hasAttributesSatisfyingExactly( | ||
| equalTo(stringKey("hystrix.command"), "HystrixTest$1"), |
There was a problem hiding this comment.
IDK whether this is worth the effort but instead of anonymous classes could use method local classes, something like
class TestCommand extends HystrixCommand<String> {
protected TestCommand(Setter setter) {
super(setter);
}
@Override
protected String run() throws Exception {
return tracedMethod();
}
private String tracedMethod() {
testing.runWithSpan("tracedMethod", () -> {});
return "Hello!";
}
}
HystrixCommand<String> command = new TestCommand(setter());then here this assertion would be equalTo(stringKey("hystrix.command"), "TestCommand") Then for each test in the same file would need to use a different name for the command to avoid $x in the class name.
There was a problem hiding this comment.
great idea, thank you!
| }; | ||
|
|
||
| String result = testing.runWithSpan("parent", () -> operation.apply(command)); | ||
| assertThat(Objects.equals(result, "Hello!")).isTrue(); |
There was a problem hiding this comment.
To me using Object.equals here doesn't make sense, try assertThat(result).isEqualTo("Hello!");
…ses for easier assertions, fix equalTo assertions
| .map( | ||
| error -> { | ||
| if (error instanceof Throwable) { | ||
| return ((Throwable) error).getMessage(); | ||
| } else { | ||
| throw new IllegalStateException("Expected Throwable result"); | ||
| } | ||
| }) |
There was a problem hiding this comment.
This is a new addition, not present in the original groovy test. Is this used?
There was a problem hiding this comment.
when I was writing the test I had some return type errors for this method and I wrote this to get around them, but now when I remove this code I no longer see those issues so I must have had another unrelated issue at the time that I have since resolved. I have removed the unneeded code from this PR
| .hasEventsSatisfyingExactly( | ||
| event -> | ||
| event | ||
| .hasName("exception") | ||
| .hasAttributesSatisfyingExactly( | ||
| equalTo( | ||
| EXCEPTION_TYPE, "java.lang.IllegalArgumentException"), | ||
| satisfies( | ||
| EXCEPTION_STACKTRACE, | ||
| val -> val.isInstanceOf(String.class)))) |
There was a problem hiding this comment.
| .hasEventsSatisfyingExactly( | |
| event -> | |
| event | |
| .hasName("exception") | |
| .hasAttributesSatisfyingExactly( | |
| equalTo( | |
| EXCEPTION_TYPE, "java.lang.IllegalArgumentException"), | |
| satisfies( | |
| EXCEPTION_STACKTRACE, | |
| val -> val.isInstanceOf(String.class)))) | |
| .hasException(exception.getCause()) |
| .hasEventsSatisfyingExactly( | ||
| event -> | ||
| event | ||
| .hasName("exception") | ||
| .hasAttributesSatisfyingExactly( | ||
| satisfies( | ||
| EXCEPTION_STACKTRACE, | ||
| val -> val.isInstanceOf(String.class)), | ||
| equalTo( | ||
| EXCEPTION_TYPE, | ||
| "com.netflix.hystrix.exception.HystrixRuntimeException"), | ||
| equalTo( | ||
| EXCEPTION_MESSAGE, | ||
| "TestCommand failed and no fallback available."))), |
There was a problem hiding this comment.
| .hasEventsSatisfyingExactly( | |
| event -> | |
| event | |
| .hasName("exception") | |
| .hasAttributesSatisfyingExactly( | |
| satisfies( | |
| EXCEPTION_STACKTRACE, | |
| val -> val.isInstanceOf(String.class)), | |
| equalTo( | |
| EXCEPTION_TYPE, | |
| "com.netflix.hystrix.exception.HystrixRuntimeException"), | |
| equalTo( | |
| EXCEPTION_MESSAGE, | |
| "TestCommand failed and no fallback available."))), | |
| .hasException(exception), |
ef68787 to
4bb7b9b
Compare
Related to #7195