markap14 commented on pull request #4225: URL: https://github.com/apache/nifi/pull/4225#issuecomment-743432494
I was wondering the same thing @joewitt - the change from Object[] to vararg is backward compatible code-wise. I.e., code that was written against a Object[] will still compile against a vararg. But if it was already compiled against Object[], will it run okay when the method is changed varaarg? That needs to be verified before we merge. This could be tested by building this branch, and then copying in a processor from 1.12.1, for instance, and verifying that the logging is still working properly. If so, then I'm good with the changes. Regarding the unit test for ParseEvtx: I would just delete the assertion all together. It's testing that something got logged at a warn level. Anything. As long as that log message was the only one and it had some arguments. That's a very odd set of constraints to put on the log message. I would also avoid the change here to an explicit log message. IMO. it is always a bad practice to write unit tests that make assertions against human-readable text. I.e., log message, Exception messages, etc. Those can change at any time in order to make the message more clear, and doing so should never result in a unit test failure. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected]
