[TEST] Jspecify bigtable test#13835
Conversation
…-cloud-java into feat/jspecify-gax-auth
…-cloud-java into feat/jspecify-gax-auth
Disable annotations in Mockito mocks for GAX classes across 17 test files in `google-cloud-bigtable` to avoid ArrayStoreException on Java 8. These tests were missed in the previous round of fixes. TAG=agy CONV=de3ef09d-a26c-4190-b578-4c993f807bdc
…-cloud-java into feat/jspecify-gax-auth
Disable annotations in Mockito mocks for GAX classes across 17 test files in `google-cloud-bigtable` to avoid ArrayStoreException on Java 8. These tests were missed in the previous round of fixes. TAG=agy CONV=de3ef09d-a26c-4190-b578-4c993f807bdc
… mocks Remove `defaultAnswer(Answers.RETURNS_DEEP_STUBS)` from programmatic mocks of `ServerStreamingCallable` in `BigtableDataClientTests`. Mockito's deep stubs cause reflection crashes on Java 8 when dealing with JSpecify annotations. Since the tests do not rely on deep stubs for these callables, they can be safely removed. TAG=agy CONV=de3ef09d-a26c-4190-b578-4c993f807bdc
…e-test TAG=agy CONV=7a3e3817-a94f-49c3-aad6-cf6d0b9873ff
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces JSpecify nullability annotations across the codebase to improve null safety and type checking. The changes involve adding @NullMarked to classes and interfaces, and using @Nullable for fields and parameters where appropriate. I have reviewed the changes and provided feedback on some minor inconsistencies in annotation placement and field nullability definitions to ensure compliance with JSpecify guidelines.
| // This is initialized once for the lifetime of the application. This enables re-using | ||
| // channels to S2A. | ||
| private static volatile ChannelCredentials s2aChannelCredentials; | ||
| private static @Nullable volatile ChannelCredentials s2aChannelCredentials; |
There was a problem hiding this comment.
According to JSpecify guidelines and standard Java modifier order, type-use annotations like @Nullable should be placed immediately before the type they qualify, after any modifiers (such as private, static, volatile). Please move @Nullable after volatile.
| private static @Nullable volatile ChannelCredentials s2aChannelCredentials; | |
| private static volatile @Nullable ChannelCredentials s2aChannelCredentials; |
| @Nullable | ||
| @GuardedBy("lock") | ||
| private HttpRequestRunnable<RequestT, ResponseT> requestRunnable; |
There was a problem hiding this comment.
According to JSpecify guidelines, type-use annotations like @Nullable should be placed immediately before the type they qualify, after any modifiers (such as private, final, volatile) and declaration annotations (such as @GuardedBy). Please move @Nullable after private.
| @Nullable | |
| @GuardedBy("lock") | |
| private HttpRequestRunnable<RequestT, ResponseT> requestRunnable; | |
| @GuardedBy("lock") | |
| private @Nullable HttpRequestRunnable<RequestT, ResponseT> requestRunnable; |
| @Nullable | ||
| @GuardedBy("lock") | ||
| private volatile ScheduledFuture<?> timeoutFuture; |
There was a problem hiding this comment.
According to JSpecify guidelines, type-use annotations like @Nullable should be placed immediately before the type they qualify, after any modifiers (such as private, volatile) and declaration annotations (such as @GuardedBy). Please move @Nullable after volatile.
| @Nullable | |
| @GuardedBy("lock") | |
| private volatile ScheduledFuture<?> timeoutFuture; | |
| @GuardedBy("lock") | |
| private volatile @Nullable ScheduledFuture<?> timeoutFuture; |
|
|
||
| private GrpcCallContext( | ||
| Channel channel, | ||
| @Nullable Channel channel, |
There was a problem hiding this comment.
The constructor parameter channel is marked @Nullable, but the corresponding field channel (at line 89) is not marked @Nullable. Since the class is @NullMarked, any unannotated type is considered non-null, which creates a mismatch. Please ensure the channel field is also marked @Nullable if it can be null. The same applies to the endpointContext field.
|
|
||
| private HttpJsonCallContext( | ||
| HttpJsonChannel channel, | ||
| @Nullable HttpJsonChannel channel, |
There was a problem hiding this comment.
The constructor parameter channel is marked @Nullable, but the corresponding field channel (at line 72) is not marked @Nullable. Since the class is @NullMarked, any unannotated type is considered non-null, which creates a mismatch. Please ensure the channel field is also marked @Nullable if it can be null. The same applies to the tracer and endpointContext fields.
|
|
2 participants
No description provided.