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.

Sign up for GitHub

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

Jump to bottom

HttpLifecycleObserverTest failure #2343

Closed
Scottmitch opened this issue Sep 3, 2022 · 1 comment · Fixed by #2376
Closed

HttpLifecycleObserverTest failure #2343

Scottmitch opened this issue Sep 3, 2022 · 1 comment · Fixed by #2376
Assignees
@idelpivnitskiy
Labels
flaky tests Unit tests are flaky

Comments

@Scottmitch
Copy link
Member

https://github.com/apple/servicetalk/pull/2341/checks?check_run_id=8172351623

HttpLifecycleObserverTest.testCompleteWithPayloadBodyAndTrailers(HttpProtocol) [2] protocol=HTTP_2

org.mockito.exceptions.verification.NoInteractionsWanted: 
No interactions wanted here:
-> at io.servicetalk.http.netty.HttpLifecycleObserverTest.verifyObservers(HttpLifecycleObserverTest.java:395)
But found this interaction on mock 'serverRequestObserver':
-> at io.servicetalk.http.utils.BiHttpLifecycleObserver$BiHttpRequestObserver.onRequestDataRequested(BiHttpLifecycleObserver.java:133)
***
For your reference, here is the list of all invocations ([?] - means unverified).
1. -> at io.servicetalk.http.utils.BiHttpLifecycleObserver$BiHttpRequestObserver.onRequestDataRequested(BiHttpLifecycleObserver.java:133)
2. -> at io.servicetalk.http.utils.BiHttpLifecycleObserver$BiHttpRequestObserver.onRequestData(BiHttpLifecycleObserver.java:142)
3. -> at io.servicetalk.http.utils.BiHttpLifecycleObserver$BiHttpRequestObserver.onRequestComplete(BiHttpLifecycleObserver.java:160)
4. [?]-> at io.servicetalk.http.utils.BiHttpLifecycleObserver$BiHttpRequestObserver.onRequestDataRequested(BiHttpLifecycleObserver.java:133)

	at app//io.servicetalk.http.netty.HttpLifecycleObserverTest.verifyObservers(HttpLifecycleObserverTest.java:395)
	at app//io.servicetalk.http.netty.HttpLifecycleObserverTest.testCompleteWithPayloadBodyAndTrailers(HttpLifecycleObserverTest.java:168)

related? #1847
@Scottmitch Scottmitch added the flaky tests Unit tests are flaky label Sep 3, 2022
@Scottmitch Scottmitch mentioned this issue Sep 3, 2022
Merged
@Scottmitch
Copy link
Member Author

timeout during local build, maybe we need to increase timeouts too?

HttpLifecycleObserverTest#testClientCancelsRequestAfterResponse(HttpProtocol) [1] protocol=HTTP_1

java.util.concurrent.TimeoutException: testClientCancelsRequestAfterResponse(io.servicetalk.http.netty.HttpProtocol) timed out after 10 seconds
	at org.junit.jupiter.engine.extension.TimeoutExceptionFactory.create(TimeoutExceptionFactory.java:29)
	at org.junit.jupiter.engine.extension.SameThreadTimeoutInvocation.proceed(SameThreadTimeoutInvocation.java:58)
	at org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:156)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestableMethod(TimeoutExtension.java:147)
	at org.junit.jupiter.engine.extension.TimeoutExtension.interceptTestTemplateMethod(TimeoutExtension.java:94)


	Suppressed: java.lang.InterruptedException
		at java.base/java.util.concurrent.locks.AbstractQueuedSynchronizer.acquireSharedInterruptibly(AbstractQueuedSynchronizer.java:1048)
		at java.base/java.util.concurrent.CountDownLatch.await(CountDownLatch.java:230)
		at io.servicetalk.http.netty.HttpLifecycleObserverTest.testClientCancelsRequestAfterResponse(HttpLifecycleObserverTest.java:328)

idelpivnitskiy added a commit to idelpivnitskiy/servicetalk that referenced this issue Sep 28, 2022
@idelpivnitskiy
Fix flaky [Http|Grpc]LifecycleObserverTest
3814fae 
Motivation:

It's possible that requesting more data from the payload publisher can
race with completion. Subsequent `request(N)` are possible even after
both observers terminate.

Modifications:

- Await for client & server shutdown before starting verification of
invoked observer callbacks.

Result:

No more events are possible after server shutdown. Verification step
sees all observed events.

Resolves apple#2343.
Resolves apple#2298.
@idelpivnitskiy idelpivnitskiy mentioned this issue Sep 28, 2022
Merged
idelpivnitskiy added a commit that referenced this issue Sep 28, 2022
@idelpivnitskiy
Fix flaky [Http|Grpc]LifecycleObserverTest (#2376)
c74af36 
Motivation:

It's possible that requesting more data from the payload publisher can
race with completion. Subsequent `request(N)` are possible even after
both observers terminate.

Modifications:

- Await for client & server shutdown before starting verification of
invoked observer callbacks.

Result:

No more events are possible after server shutdown. Verification step
sees all observed events.

Resolves #2343.
Resolves #2298.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@idelpivnitskiy idelpivnitskiy

Labels
flaky tests Unit tests are flaky
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix flaky [Http|Grpc]LifecycleObserverTest idelpivnitskiy/servicetalk
2 participants
@idelpivnitskiy @Scottmitch