MINOR: Remove unused isSticky assert out from tests only do constrainedAssign

[showuon]

Suggested by @ableegoldman in #8778 (comment), remove the unused isSticky assert out from tests only do constrainedAssign. I printed out the test name and which assign method used during tests. Below is the output. So, we can remove the isSticky() assertion from the tests using constrainedAssign method.

In summary, total 9 tests were doing assertTrue(assignor.isSticky()) but using constrainedAssign. Thanks.

StickyAssignorTest - testAssignmentWithConflictingPreviousGenerations
2020-06-03T09:44:03.362+0800 [DEBUG] [TestEventLogger] constrainedAssign

StickyAssignorTest - testSchemaBackwardCompatibility
2020-06-03T09:44:03.376+0800 [DEBUG] [TestEventLogger] constrainedAssign

StickyAssignorTest - testAssignmentWithMultipleGenerations1
2020-06-03T09:44:03.378+0800 [DEBUG] [TestEventLogger] constrainedAssign

StickyAssignorTest - testAssignmentWithMultipleGenerations2
2020-06-03T09:44:03.380+0800 [DEBUG] [TestEventLogger] constrainedAssign

AbstractStickyAssignorTest - testStickiness
2020-06-03T09:44:03.383+0800 [DEBUG] [TestEventLogger] constrainedAssign

AbstractStickyAssignorTest - testAddRemoveConsumerOneTopic
2020-06-03T09:44:03.386+0800 [DEBUG] [TestEventLogger] constrainedAssign

AbstractStickyAssignorTest - testReassignmentWithRandomSubscriptionsAndChanges
2020-06-03T09:44:03.393+0800 [DEBUG] [TestEventLogger] generalAssign

AbstractStickyAssignorTest - testNewSubscription
2020-06-03T09:44:06.238+0800 [DEBUG] [TestEventLogger] generalAssign

AbstractStickyAssignorTest - testAddRemoveTopicTwoConsumers
2020-06-03T09:44:06.239+0800 [DEBUG] [TestEventLogger] constrainedAssign

AbstractStickyAssignorTest - testReassignmentAfterOneConsumerLeaves
2020-06-03T09:44:06.243+0800 [DEBUG] [TestEventLogger] generalAssign

AbstractStickyAssignorTest - testLargeAssignmentWithMultipleConsumersLeavingAndRandomSubscription
2020-06-03T09:44:06.268+0800 [DEBUG] [TestEventLogger] generalAssign

AbstractStickyAssignorTest - testSameSubscriptions
2020-06-03T09:44:08.400+0800 [DEBUG] [TestEventLogger] constrainedAssign

AbstractStickyAssignorTest - testReassignmentAfterOneConsumerAdded
2020-06-03T09:44:08.403+0800 [DEBUG] [TestEventLogger] constrainedAssign

Committer Checklist (excluded from commit message)

• Verify design and implementation
• Verify test coverage and CI build status
• Verify documentation (including upgrade notes)

[showuon]

hi @ableegoldman , could you review this PR? Thanks.

[ableegoldman]

Thanks @showuon ! This LGTM and I'm fine with merging as-is, but I'm wondering if we can improve on the stickiness verification instead of just removing it.

Granted, I think the check that we're removing (ie #isSticky) wasn't doing much in the way of stickiness verification to begin with. It would be nice to write a quick method in AbstractStickyAssignorTest that could validate the stickiness of an assignment. The challenge there is how to check the stickiness in a general fashion, since just checking the specific assignment that's produced will be really brittle and of course we'd have to assert the specific assignment of each test.

Just some thoughts I had while reading this, we don't need necessarily to solve the problem in this PR but it would be nice to replace the check we removed with a better one. Do you have any ideas?

[showuon]

Thanks for the comment, @ableegoldman . Yes, I agree we should improve the stickiness verification, but I haven't got a better idea for that so far. I've created a ticket to track it:
KAFKA-10118 - Improve stickiness verification for AbstractStickyAssignorTest

For this PR, I also put the partitionMovements initialization back to generalAssign method sine we won't have NPE during testing anymore after this fix. I think we can firstly merge this PR, and then discuss the improvement in KAFKA-10118.

How do you think?

[ableegoldman]

SGTM! Thanks for the patch

[mjsax]

Retest this please.