Make unit tests that forcibly exercise async security policies codepaths. #10835
Conversation
The usage of Futures.immediateFuture makes the implementation take several shortcuts to process server authorization. This leads to PendingAuthListener being under-tested. This commit adds delays to the futures in the hope that they will be triggered by the new codepaths. Before: https://fusion2.corp.google.com/invocations/fafaa334-2ac1-4d12-814c-33755fa08ad6/coverage After: https://fusion2.corp.google.com/invocations/6e3b4b41-461b-418d-8e9f-d9043dc3e5c6/coverage Part of grpc#10566.
binder/src/androidTest/java/io/grpc/binder/BinderSecurityTest.java
Outdated
Show resolved
Hide resolved
mateusazis
changed the title
binder/src/test/java/io/grpc/binder/BinderTransportSecurityTest.java
Outdated
Show resolved
Hide resolved
binder/src/test/java/io/grpc/binder/BinderTransportSecurityTest.java
Outdated
Show resolved
Hide resolved
binder/src/test/java/io/grpc/binder/BinderTransportSecurityTest.java
Outdated
Show resolved
Hide resolved
binder/src/test/java/io/grpc/binder/BinderTransportSecurityTest.java
Outdated
Show resolved
Hide resolved
| } | ||
|
|
||
| void setSecurityPolicyStatusWhenReady(Status status) { | ||
| Uninterruptibles.takeUninterruptibly(statusesToSet).set(status); |
There was a problem hiding this comment.
This also appears to block the main thread, in the common case where the server hasn't seen the request yet.
There was a problem hiding this comment.
That was intentional. At this point, the test has nothing else to do but wait until the gRPC's executor has ran, invoked the AsyncSecurityPolicy and placed a new SettableFuture<Status> into the queue.
This is meant to work somewhat like a CyclicBarrier: I want to have the test thread and the gRPC thread synchronized at the same point: after this line. Thus, the test must wait until the queue has an entry as a signal that the implementation has generated the future.
There was a problem hiding this comment.
But how do you know that the gRPC executor hasn't enqueued something to run on the main thread and won't run the ServerCallHandler until that completes => Now you are deadlocked because your main thread won't service the event loop again until the ServerCallHandler runs.
It seems to me that you need to poll for this, not block, and idle() the single main looper between take() attempts.
binder/src/test/java/io/grpc/binder/RobolectricBinderSecurityTest.java
Outdated
Show resolved
Hide resolved
|
markb74
added
the
kokoro:run
grpc-kokoro
removed
the
kokoro:run
binder/src/test/java/io/grpc/binder/RobolectricBinderSecurityTest.java
Outdated
Show resolved
Hide resolved
binder/src/test/java/io/grpc/binder/RobolectricBinderSecurityTest.java
Outdated
Show resolved
Hide resolved
jdcormie
added
the
kokoro:force-run
grpc-kokoro
removed
the
kokoro:force-run
|
Looks like this doesn't build at the moment ... |
I broke myself with #10836. Should be fixed now. |
jdcormie
added
the
kokoro:force-run
grpc-kokoro
removed
the
kokoro:force-run
jdcormie
added
kokoro:run
grpc-kokoro
removed
kokoro:run
The usage of Futures.immediateFuture makes the implementation take several shortcuts to process server authorization. This leads to PendingAuthListener being under-tested.
This commit adds delays to the futures in the hope that they will be triggered by the new codepaths.
Before: https://fusion2.corp.google.com/invocations/fafaa334-2ac1-4d12-814c-33755fa08ad6/coverage
After: https://fusion2.corp.google.com/invocations/6e3b4b41-461b-418d-8e9f-d9043dc3e5c6/coverage
Part of #10566.