Introducing Hamcrest API to simplify assertion in `ImplicitPlannerTest.checkStrictModeWithCurrentAccountVmsPresent`

[Codegass]

ISSUE TYPE

• Improvement Request

COMPONENT NAME

unit-test

OS / ENVIRONMENT

N/A

SUMMARY

When I am running the test for checkStrictModeWithCurrentAccountVmsPresent I noticed that the logic of the assertion in test case is to check that all the items in the clusterList are equal to one.

This is currently expressed by the combination of a for loop and an if-else block, see line 214 to line 221. List code below:

(This pattern also happens in checkStrictModeHostWithCurrentAccountVmsFulland checkPreferredModePreferredHostAvailable in the same test class ImplicitPlannerTest.)

...
        boolean foundNeededCluster = false;
        for (Long cluster : clusterList) {
            if (cluster != 1) {
                fail("Found a cluster that shouldn't have been present, cluster id : " + cluster);
            } else {
                foundNeededCluster = true;
            }
        }
        assertTrue("Didn't find cluster 1 in the list. It should have been present", foundNeededCluster);
...

The assertion logic is not explicit by reading the current code. This can be simplified into one line of code using hamcrest API, which make the assertion logic straightforward and explit:

After refactoring

import static org.hamcrest.Matchers.everyItem;
import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.MatcherAssert.assertThat; // org.junit.Assert.assertThat has been deprecated
...
        assertThat("Found cluster that shouldn't have been present, only cluster 1 should have been present",clusterList, everyItem(equalTo(1L)));
...

The same refactoring can also be applied to checkStrictModeHostWithCurrentAccountVmsFulland checkPreferredModePreferredHostAvailable.

Rationale

Make the code clearer and easier to understand with hamcrest collection checker.

Operational impact

No impacts to operations; only changes to test code.