Copilot commented on code in PR #19908:
URL: https://github.com/apache/druid/pull/19908#discussion_r3731009273
##########
extensions-contrib/cloudfiles-extensions/src/test/java/org/apache/druid/storage/cloudfiles/CloudFilesObjectApiProxyTest.java:
##########
@@ -51,10 +51,10 @@ public void getTest()
CloudFilesObjectApiProxy cfoApiProxy = new
CloudFilesObjectApiProxy(cloudFilesApi, region, container);
CloudFilesObject cloudFilesObject = cfoApiProxy.get(path, 0);
- Assert.assertEquals(cloudFilesObject.getPayload(), payload);
- Assert.assertEquals(cloudFilesObject.getRegion(), region);
- Assert.assertEquals(cloudFilesObject.getContainer(), container);
- Assert.assertEquals(cloudFilesObject.getPath(), path);
+ Assertions.assertEquals(cloudFilesObject.getPayload(), payload);
+ Assertions.assertEquals(cloudFilesObject.getRegion(), region);
+ Assertions.assertEquals(cloudFilesObject.getContainer(), container);
+ Assertions.assertEquals(cloudFilesObject.getPath(), path);
Review Comment:
In JUnit Jupiter, `assertEquals(expected, actual)` is the conventional
order; the current assertions have the operands reversed. This doesn't change
pass/fail behavior, but it will produce confusing failure diffs
(expected/actual swapped). Consider switching to
`Assertions.assertEquals(payload, cloudFilesObject.getPayload())` (and
similarly for the other fields).
##########
extensions-contrib/cloudfiles-extensions/src/test/java/org/apache/druid/storage/cloudfiles/CloudFilesByteSourceTest.java:
##########
@@ -78,10 +78,10 @@ public void openStreamWithRecoverableErrorTest() throws
IOException
byteSource.openStream();
}
catch (Exception e) {
- Assert.assertEquals("Recoverable exception", e.getMessage());
+ Assertions.assertEquals("Recoverable exception", e.getMessage());
}
- Assert.assertEquals(stream, byteSource.openStream());
+ Assertions.assertEquals(stream, byteSource.openStream());
Review Comment:
This test will still pass if `byteSource.openStream()` does *not* throw on
the first call, because the `try` block contains no assertion that an exception
must occur. Consider using `Assertions.assertThrows(...)` for the first call
(and assert the message/category there), so the test reliably fails if no
exception is thrown.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
