Add test cases for different ProcessorSlot

[cdfive]

Describe what this PR does / why we need it

Add some test cases for different ProcessorSlot, NodeSelectorSlot,ClusterBuilderSlot and so on.

Does this pull request fix one issue?

Related to #580

Describe how you did it

• Add testFireEntry method for each Slot to verify fireEntry method has been called in entry method, and called only once.

• Add testFireExit method for each Slot to verify fireExit method has been called in exit method, and called only once.

• Add some test cases to test the interactions in entry method, by mocking and verify the interaction of Node and Slot.

Describe how to verify it

Run the test cases.

Special notes for reviews

The ClusterNodeBuilderTest is renamed to ClusterBuilderSlotTest for more standard test Class name.

The DegradeSlot and SystemSlot using static method to check the rules: DegradeRuleManager.checkDegrade(resourceWrapper, context, node, count); SystemRuleManager.checkSystem(resourceWrapper);, while FlowSlot and AuthoritySlot call the internal method.

Maybe they can be unified in the furture. I think using static method in XxxRuleManager is better, since it makes the slot class more testable, and we can write separate test cases for XxxRuleManager. This is a suggestion, they're not modified yet.

[sczyh30]

Thanks for contributing.

Maybe they can be unified in the future. I think using static method in XxxRuleManager is better, since it makes the slot class more testable, and we can write separate test cases for XxxRuleManager.

IMHO, XxxRuleManager is used for managing rules (update / get rules), not for rule checking. In legacy design, the rule checking logic is located in Rule#passCheck method, which will be deprecated and removed later. Maybe it's better to move all checkXxx methods to each slot? Discussions are welcomed.

[cdfive]

XxxRuleManager is used for managing rules (update / get rules), not for rule checking Maybe it's better to move all checkXxx methods to each slot.

Oh, I see. Previously I just found the implement way is different in four slots, and thought unified may be better.

I agree with you, the XxxRuleManager is used for managing rules (update / get rules)，as the class name indicates its responsibility.

How about move checkXxx method in XxxRuleChecker class, like FlowRuleChecker and FlowRuleCheckerTest.

In slot entry method, just call XxxRuleChecker.checkXxx() and fireEntry, make the slot code simple and clear, and easy to test. Then in XxxRuleChecker.checkXxx() method, call XxxRuleManager to get all rules and do checkRule logic for each rule, and we can add separate test cases for XxxRuleChecker and XxxRuleManager.

[codecov-io]

Codecov Report

Merging #598 into master will decrease coverage by 0.55%. The diff coverage is n/a.

@@             Coverage Diff              @@
##             master     #598      +/-   ##
============================================
- Coverage     43.03%   42.48%   -0.56%     
+ Complexity     1568     1428     -140     
============================================
  Files           337      307      -30     
  Lines          9877     8894     -983     
  Branches       1332     1203     -129     
============================================
- Hits           4251     3779     -472     
+ Misses         5097     4658     -439     
+ Partials        529      457      -72

Impacted Files	Coverage Δ	Complexity Δ
...sp/sentinel/slots/system/SystemBlockException.java	0% <0%> (-44.45%)	0% <0%> (-2%)
...a/csp/sentinel/slots/system/SystemRuleManager.java	43.16% <0%> (-21.84%)	7% <0%> (-12%)
.../alibaba/csp/sentinel/slots/system/SystemRule.java	28.57% <0%> (-16.33%)	8% <0%> (-4%)
...libaba/csp/sentinel/slotchain/ResourceWrapper.java	71.42% <0%> (-15.24%)	3% <0%> (-3%)
...aba/csp/sentinel/adapter/servlet/CommonFilter.java	74.41% <0%> (-13.39%)	9% <0%> (-2%)
...m/alibaba/csp/sentinel/node/metric/MetricNode.java	51.94% <0%> (-13.32%)	18% <0%> (-5%)
...inel/adapter/gateway/sc/SentinelGatewayFilter.java	26.92% <0%> (-8.57%)	3% <0%> (-1%)
.../java/com/alibaba/csp/sentinel/util/SpiLoader.java	8.64% <0%> (-7.69%)	2% <0%> (-2%)
...nel/adapter/reactor/SentinelReactorSubscriber.java	80% <0%> (-6.89%)	19% <0%> (-2%)
...baba/csp/sentinel/slotchain/SlotChainProvider.java	63.15% <0%> (-6.85%)	5% <0%> (+2%)
... and 71 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 20ad3c8...77059ce. Read the comment docs.

[sczyh30]

Could you please resolve the conflicts? :)

[CLAassistant]

All committers have signed the CLA.