RocMarshal commented on code in PR #10:
URL:
https://github.com/apache/flink-connector-http/pull/10#discussion_r2593123531
##########
flink-connector-http/src/test/java/org/apache/flink/connector/http/table/lookup/HttpLookupTableSourceITCaseTest.java:
##########
@@ -1123,6 +1127,35 @@ private void
assertEnrichedRowsNoDataBadStatus(Collection<Row> collectedRows) {
});
}
+ private void assertEnrichedRowsNoDataGoodStatus(Collection<Row>
collectedRows) {
+
+ final int rowArity = 10;
+ // validate every row and its column.
+
+ assertAll(
Review Comment:
Thanks @davidradl for the contribution.
I noticed that the API style in the assertion sections of the Flink HTTP
test cases is quite arbitrary, with a mix of Jupiter Assertions and AssertJ
Assertions being used.
I’d like to briefly share two small suggestions:
- A: Could we align with the assertion style used in the Flink repository?
That is, to use only AssertJ-related assertion APIs to implement assertion
statements.
- B: Building on A, we could handle the existing assertion statements as
follows:
- a) There’s no need to rush and change them to the AssertJ style, as this
is a historical issue and not changing it won’t affect the assertion
functionality. It would be a good approach to restart the API migration for
this part when our contributors have enough bandwidth to focus on it.
- b) For newly introduced test case code, we expect to use AssertJ
assertion APIs to complete the assertion statements.
--
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]
