Copilot commented on code in PR #6313:
URL: https://github.com/apache/shenyu/pull/6313#discussion_r3019412050
##########
shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/AbstractShenyuPluginTest.java:
##########
@@ -248,6 +253,52 @@ public void executeRuleFullTest() {
verify(testShenyuPlugin).doExecute(exchange, shenyuPluginChain,
selectorData, ruleData);
}
+ @Test
+ public void executeSelectorCacheHitShouldNotReMatchTest() {
+ List<ConditionData> conditionDataList =
Collections.singletonList(conditionData);
+ this.selectorData.setMatchMode(0);
+ this.selectorData.setMatchRestful(false);
+ this.selectorData.setLogged(true);
+ this.selectorData.setConditionList(conditionDataList);
+ this.selectorData.setContinued(false);
+ BaseDataCache.getInstance().cachePluginData(pluginData);
+ BaseDataCache.getInstance().cacheSelectData(selectorData);
+
MatchDataCache.getInstance().cacheSelectorData(exchange.getRequest().getURI().getRawPath(),
selectorData, 1000, 10000L);
+ try (MockedStatic<MatchStrategyFactory> mockedStatic =
Mockito.mockStatic(MatchStrategyFactory.class)) {
+ mockedStatic.when(() ->
MatchStrategyFactory.match(selectorData.getMatchMode(),
selectorData.getConditionList(), exchange)).thenReturn(true);
+ StepVerifier.create(testShenyuPlugin.execute(exchange,
shenyuPluginChain)).expectSubscription().verifyComplete();
+ verify(testShenyuPlugin).doExecute(Mockito.same(exchange),
Mockito.same(shenyuPluginChain), Mockito.same(selectorData),
+ argThat(rule ->
Constants.DEFAULT_RULE.equals(rule.getId()) &&
selectorData.getId().equals(rule.getSelectorId())));
+ mockedStatic.verifyNoInteractions();
Review Comment:
`mockedStatic.verifyNoInteractions()` will fail here because the prior
`mockedStatic.when(() -> MatchStrategyFactory.match(...))` stubbing records an
invocation. To assert the execute-path doesn't call the matcher, either clear
invocations after stubbing (`mockedStatic.clearInvocations()`) before
verifying, or replace this with an explicit `mockedStatic.verify(() ->
MatchStrategyFactory.match(...), never())` check (and drop the stub if it's not
needed).
##########
shenyu-plugin/shenyu-plugin-base/src/test/java/org/apache/shenyu/plugin/base/AbstractShenyuPluginTest.java:
##########
@@ -248,6 +253,52 @@ public void executeRuleFullTest() {
verify(testShenyuPlugin).doExecute(exchange, shenyuPluginChain,
selectorData, ruleData);
}
+ @Test
+ public void executeSelectorCacheHitShouldNotReMatchTest() {
+ List<ConditionData> conditionDataList =
Collections.singletonList(conditionData);
+ this.selectorData.setMatchMode(0);
+ this.selectorData.setMatchRestful(false);
+ this.selectorData.setLogged(true);
+ this.selectorData.setConditionList(conditionDataList);
+ this.selectorData.setContinued(false);
+ BaseDataCache.getInstance().cachePluginData(pluginData);
+ BaseDataCache.getInstance().cacheSelectData(selectorData);
+
MatchDataCache.getInstance().cacheSelectorData(exchange.getRequest().getURI().getRawPath(),
selectorData, 1000, 10000L);
+ try (MockedStatic<MatchStrategyFactory> mockedStatic =
Mockito.mockStatic(MatchStrategyFactory.class)) {
+ mockedStatic.when(() ->
MatchStrategyFactory.match(selectorData.getMatchMode(),
selectorData.getConditionList(), exchange)).thenReturn(true);
+ StepVerifier.create(testShenyuPlugin.execute(exchange,
shenyuPluginChain)).expectSubscription().verifyComplete();
+ verify(testShenyuPlugin).doExecute(Mockito.same(exchange),
Mockito.same(shenyuPluginChain), Mockito.same(selectorData),
+ argThat(rule ->
Constants.DEFAULT_RULE.equals(rule.getId()) &&
selectorData.getId().equals(rule.getSelectorId())));
+ mockedStatic.verifyNoInteractions();
+ verify(shenyuPluginChain, never()).execute(exchange);
+ }
+ }
+
+ @Test
+ public void executeRuleCacheHitShouldNotReMatchTest() {
+ List<ConditionData> conditionDataList =
Collections.singletonList(conditionData);
+ this.selectorData.setMatchMode(0);
+ this.selectorData.setMatchRestful(false);
+ this.selectorData.setLogged(true);
+ this.selectorData.setConditionList(conditionDataList);
+ this.selectorData.setContinued(true);
+ this.ruleData.setConditionDataList(conditionDataList);
+ this.ruleData.setMatchMode(0);
+ this.ruleData.setMatchRestful(false);
+ BaseDataCache.getInstance().cachePluginData(pluginData);
+ BaseDataCache.getInstance().cacheSelectData(selectorData);
+ BaseDataCache.getInstance().cacheRuleData(ruleData);
+
MatchDataCache.getInstance().cacheSelectorData(exchange.getRequest().getURI().getRawPath(),
selectorData, 1000, 10000L);
+
MatchDataCache.getInstance().cacheRuleData(exchange.getRequest().getURI().getRawPath(),
ruleData, 1000, 10000L);
+ try (MockedStatic<MatchStrategyFactory> mockedStatic =
Mockito.mockStatic(MatchStrategyFactory.class)) {
+ mockedStatic.when(() ->
MatchStrategyFactory.match(selectorData.getMatchMode(),
selectorData.getConditionList(), exchange)).thenReturn(true);
+ mockedStatic.when(() ->
MatchStrategyFactory.match(ruleData.getMatchMode(),
ruleData.getConditionDataList(), exchange)).thenReturn(true);
+ StepVerifier.create(testShenyuPlugin.execute(exchange,
shenyuPluginChain)).expectSubscription().verifyComplete();
+ verify(testShenyuPlugin).doExecute(exchange, shenyuPluginChain,
selectorData, ruleData);
+ mockedStatic.verifyNoInteractions();
Review Comment:
Same issue as above: `verifyNoInteractions()` is inconsistent with the two
`when(() -> MatchStrategyFactory.match(...))` stubbings, since stubbing itself
counts as an invocation. Prefer `mockedStatic.clearInvocations()` after
stubbing and before running `execute(...)`, or use `mockedStatic.verify(() ->
MatchStrategyFactory.match(...), never())` (for each signature) instead of
`verifyNoInteractions()`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
