mattcasters commented on code in PR #8911:
URL: https://github.com/apache/hop/pull/8911#discussion_r4272005189
##########
plugins/actions/columnsexist/src/test/java/org/apache/hop/workflow/actions/columnsexist/ActionColumnsExistTest.java:
##########
@@ -47,4 +66,86 @@ void testSerialization() throws Exception {
Assertions.assertEquals("F1", action.getColumns().get(0).getName());
Assertions.assertEquals("F2", action.getColumns().get(1).getName());
}
+
+ @Test
+ void testExecuteWithNoConnection() {
+ ActionColumnsExist action = new ActionColumnsExist();
+ action.setConnectionName(null);
Review Comment:
**[suggestion]** `testExecuteWithNoConnection` and
`testExecuteWithNonExistentConnection` never set a table name. The constructor
leaves `tableName` null, and the pre-change `execute` already returned failure
for an empty table before it looked up the connection. Those two tests
therefore stay green if the new connection guards are removed, and they do not
reproduce issue #8904 (connection missing, table set, column list empty, `0 ==
0` success). `testExecuteWithEmptyColumns` does lock the empty-list failure,
but only after a connection has already been resolved. Nothing asserts the
success contract: a found connection, an existing table, and every column
present still sets `result=true` and `nrErrors=0`. An inverted success
condition would pass this class.
**Suggestion:** Set a non-empty table name on the missing-connection cases
so they reach the connection guard rather than the empty-table return. Add one
test that stubs `Database.checkTableExists` / `checkColumnExists` to return
true for a non-empty column list and asserts `result` true and `nrErrors` 0.
--
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]
