Uh oh!
There was an error while loading. Please reload this page.
Migrate dd-trace-core groovy files to java part 11 - #11566
Conversation
we migrate 11 tests: - PrintingWriterTest - TraceStructureWriterTest - CiVisibilityApmProtocolInterceptorTest - CiVisibilityTraceInterceptorTest - CiTestCovMapperV2Test - CiTestCycleMapperV1PayloadTest - LambdaAppSecHandlerTest - LambdaHandlerTest - SkipUnhandledTypeJsonSerializerTest - LLMObsSpanMapperTest - TracerConnectionReliabilityTest
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:f4a032df81
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @Test | ||
| void processRequestStartHandlesExceptionDuringStreamReading() throws IOException { | ||
| ByteArrayInputStream mockStream = mock(ByteArrayInputStream.class); | ||
| when(mockStream.available()).thenThrow(new IOException("Stream error")); |
There was a problem hiding this comment.
Avoid stubbing a checked exception on available()
When this test runs, Mockito rejects thenThrow(new IOException(...)) here because ByteArrayInputStream.available() does not declare IOException; the stubbing fails with a Mockito checked-exception error before processRequestStart is exercised. Use a subclass/test stream that can throw from a method the handler calls, or throw an unchecked exception if that is the path being tested.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Wrong, the mocking works perfectly. checked expcetion are not visible by the JVM. only javac is checking them. So if a method is not declaring a checked exception this is not an issue
This comment has been minimized.
This comment has been minimized.
🟢 Java Benchmark SLOs — All performance SLOs passed
PR vs. master results
Commit: Load and DaCapo benchmarks can be triggered manually in the GitLab pipeline. Results will appear in the Benchmarking Platform UI after completion. |
/merge |
View all feedbacks in Devflow UI.
The expected merge time in
|
Uh oh!
There was an error while loading. Please reload this page.
615f932
into
masterUh oh!
There was an error while loading. Please reload this page.
What Does This Do
we migrate 11 tests:
Motivation
this is part of the effort to migrate groovy tests to Java/JUnit
part1: #11053
part2: #11062
part3: #11085
part4: #11146
part5: #11217
part6: #11362
part7: #11374
part8: #11437
part9: #11488
part10: #11543
Additional Notes
Contributor Checklist
type:and (comp:orinst:) labels in addition to any other useful labelsclose,fix, or any linking keywords when referencing an issueUse
solvesinstead, and assign the PR milestone to the issue/merge. You can also:/merge --commit-message "..."/merge -c/merge -f --reason "reason"; please use this judiciously, as some checks do not run at the PR-level (note: the PR still needs to be mergeable, this will only skip the pre-merge build)Jira ticket: [PROJ-IDENT]