-
Notifications
You must be signed in to change notification settings - Fork 452
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.
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
Test case that fails due to NullPointerException #1346
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mocking makes it hard to figure out whether there's a bug in the model code or just a bad mock setup. See #1345 for a test that uses a real HTTP server instead of a mock.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Possibly BatchUnparsedResponse needs to handle empty input better
I agree that this test case is not clear which one is the problem. I'll try to simplify this further. |
} | ||
|
||
@Test | ||
public void testErrorContentRead_NoRetry() throws IOException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This succeeds. Maybe problem in retry?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this test demonstrates a bug or an issue with the http client. The NullPointerException here occurs in google-api-java-client. The code there looks very suspicious to me.
If there is an issue here it should be able to be demonstrated with classes already in this repository.
} | ||
} | ||
}; | ||
try (FakeServer server = new FakeServer(handler)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
With the fake server, this test succeeds. This implies that the mock setup in Beam is not enough (at least for this test case).
The FakeServer in #1345 worked. Thank you. Now I'm feeling something is lacking in the mock setup.
Right. I haven't been able to isolate the problem in a class yet. |
Next step: investigate why mock fails and fake server succeeds. With FakeServerWith FakeServer, BatchRequest line 262 gets a buffer with 137 bytes with position 0. With MockWith mock, BatchRequest line 262 gets a buffer with 17 bytes with position 0. It seems that since this change https://github.com/googleapis/google-http-java-client/pull/986/files#diff-f8b4bfc64923817722aee3aebebae240783e193ebbd305c2e3291d2cfddd5c31R1015, lowLevelHttpResponse , |
Investigating the reason for apache/beam#14527 (comment)