NIFI-11873 Refactored PeerSelectorTest from Groovy to Java.

[dan-s1]

Refactored PeerSelectorTest from Groovy to Java. Replaced the Groovy mock objects with Mockito mocks and changed scope of certain variables and methods in PeerSelector from private to package private in order to allow access by PeerSelectorTest.

Summary

NIFI-11873

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

• Apache NiFi Jira issue created

Pull Request Tracking

• Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
• Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

• Pull Request based on current revision of the main branch
• Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

• Build completed using mvn clean install -P contrib-check
  • JDK 17

Licensing

• New dependencies are compatible with the Apache License 2.0 according to the License Policy
• New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

• Documentation formatting appears as expected in rendered files

[dan-s1]

@exceptionfactory 21/22 of the unit tests pass. There is one test testShouldFetchRemotePeerStatusesInFailureScenario which fails but the reasons vary each time it is run. It appears it is due to some random feature which is being used. I am not sure though how this all works and I would appreciate another set of eyes why this is happening.
What I am seeing is the failure is either at attempt 2 and sometimes at attempt 4
Each time actual value is
PeerStatus[hostname=node1.nifi,port=-1,secure=false,flowFileCount=0]
instead of
PeerStatus[hostname=node2.nifi,port=-1,secure=false,flowFileCount=0] or null

[exceptionfactory]

Thanks for working on this refactoring @dan-s1. Unfortunately the Groovy-based test included a number of access violations for calling private methods. It would be much better to avoid the method visibility changes solely for the purpose of maintaining these tests. That may mean some tests need to be removed or rewritten completely. This is one of the more complicated classes, so that may also point to the need for other refactoring, as a first step, I recommend making adjustment to avoid visibility changes. If that presents too much difficulty, it may be better to close and revisit later.

[exceptionfactory]

I realize that I recommended changing the visibility of one method when commenting on the Jira, but I did not expect it to be necessary for so many methods.

[dan-s1]

I did not realize either. I worked off your original comment thinking it would be okay for the others. Aside from variables I am not sure how I would not reduce the scope of the methods whose scope had to change. I am not sure I understand everything going on in PeerSelector so I do not feel I would be the right one to refactor it. Is there anyway what I have done in this PR be used? I did get most of the tests to work by using Mockito which I thought be worthwhile to preserve.

[exceptionfactory]

Thanks for your work on it, it may be a helpful background for someone else to take it up. If there are any tests that can be migrated without method visibility changes, that would might be a helpful starting point for a new Java-based test class. Then subsequent work could handle refactoring the remaining tests.

[dan-s1]

Should I close the ticket or just have the ticket no longer assigned to me? Also should I add you comments regarding the visibility issues to the ticket so others will know what to be aware of?

[exceptionfactory]

Should I close the ticket or just have the ticket no longer assigned to me? Also should I add you comments regarding the visibility issues to the ticket so others will know what to be aware of?

The ticket should be kept open, but you can unassign yourself. The pull request comments are automatically mirrored to the Jira issue.

[exceptionfactory]

Thanks again for your work on this pull request @dan-s1. I am closing it for now, but it will be linked in the Jira issue for future reference.