Skip to content

Remove Hystrix test retry and timeout#2494

Merged
bantonsson merged 1 commit into
masterfrom
ban/hystrix-remove-test-retry-and-timeout
Mar 17, 2021
Merged

Remove Hystrix test retry and timeout#2494
bantonsson merged 1 commit into
masterfrom
ban/hystrix-remove-test-retry-and-timeout

Conversation

@bantonsson
Copy link
Copy Markdown
Contributor

No description provided.

@bantonsson bantonsson requested a review from a team as a code owner March 15, 2021 10:27
@richardstartin
Copy link
Copy Markdown
Contributor

What's the motivation? #2389 removes the scope leak which prevents this test from passing in strict mode, but either uncovers or causes more problems.

@bantonsson
Copy link
Copy Markdown
Contributor Author

bantonsson commented Mar 15, 2021

So the motivation is that all the failures that I have seen is a timeout on this form:

Method timed out after 5.00 seconds
        at net.bytebuddy.description.type.TypeDescription$AbstractBase$OfSimpleType$WithDelegation.getModifiers(TypeDescription.java:8383)
        ...
        at HystrixObservableChainTest$__spock_feature_2_0_closure2.closure5$_closure10(HystrixObservableChainTest.groovy:137)
        at datadog.trace.agent.test.asserts.SpanAssert.assertSpan(SpanAssert.groovy:37)
        ...

The test is obviously in the middle of verifying the results via assertTrace/assertSpan but it's too slow and times out because of the 5 second range (probably for the 3rd time). I think that having the @Retry annotation on any test, except maybe as a port conflict mitigation (until that is solved properly), is just plain wrong.

I'm going to rerun this whole PR on CI for a number of times and see if it fails.

@bantonsson bantonsson self-assigned this Mar 17, 2021
@bantonsson bantonsson added the tag: no release notes Changes to exclude from release notes label Mar 17, 2021
@bantonsson
Copy link
Copy Markdown
Contributor Author

I have now run the whole build for all platforms (rerun from start) 13 times without this test failing once.

Copy link
Copy Markdown
Contributor

@richardstartin richardstartin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll rebase on this because I had an improvement to make the test run in strict mode

@bantonsson bantonsson merged commit 995aa6f into master Mar 17, 2021
@bantonsson bantonsson deleted the ban/hystrix-remove-test-retry-and-timeout branch March 17, 2021 14:40
@github-actions github-actions Bot added this to the 0.76.0 milestone Mar 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tag: no release notes Changes to exclude from release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants