Github user alopresto commented on the pull request:
https://github.com/apache/nifi/pull/253#issuecomment-194012946
@rickysaltzer Yes, I did not mean full-stack integration tests with a KDC,
just unit tests, primarily focusing on validation, especially of user input and
retrieved credentials, as that is a common security attack vector.
I do appreciate you adding simple validation tests. My only question is
regarding the extraction of the boolean check for
`UserGroupInformation.isSecurityEnabled()` in
[`HBase_1_1_2_ClientService.java`](https://github.com/rickysaltzer/nifi/blob/6dc8416b7f2cb5480a6f6ae63777ea4080263e67/nifi-nar-bundles/nifi-standard-services/nifi-hbase_1_1_2-client-service-bundle/nifi-hbase_1_1_2-client-service/src/main/java/org/apache/nifi/hbase/HBase_1_1_2_ClientService.java)
on line 137. I'm not completely familiar with the HBase components, but my
understanding is that this will now be executed statically at the class
initialization time, rather than during the `customValidate` method call. Is
this intentional?
As for @bbende , I think Groovy tests allow for easier static mocking, but
I do not consider it a priority for this PR. I think the existing unit tests
@rickysaltzer added are sufficient (although perhaps with a perfunctory check
of the error message/cause after the `runner.assertNotValid(service);` just to
ensure it's not accidentally testing something else.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
