bamaer commented on PR #8778: URL: https://github.com/apache/hop/pull/8778#issuecomment-6066593447
Thanks — the `execute()` fix is right: the `setNrErrors(1)` calls were overwritten by `setNrErrors(nrErrors)` at the end, so `updateErrors()` is the correct fix. **Should fix** - **Empty rows now fail the action.** Adding `updateErrors()` to the `RowEmpty` / `ArgEmpty` branches changes existing behaviour. A blank row from the previous result, or a grid table name that resolves to empty, used to be logged and skipped. Now it fails the action and stops the remaining truncates (`continueProcess = false`). That's defensible, but it isn't in the issue or the description. Please either mention it in the PR description or keep the skip behaviour and only count connection/execution errors. - **No test for the actual bug.** A small unit test that runs `execute()` with a missing connection and asserts `getResult() == false` / `getNrErrors() > 0` would stop this from regressing. Today the module only has `ActionTruncateTablesLoadSaveTest`. -- 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]
