Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ISSUE#4515] Add test case for ProtocolPluginFactory. #4516

Merged
merged 5 commits into from
Oct 30, 2023

Conversation

yanrongzhen
Copy link
Contributor

@yanrongzhen yanrongzhen commented Oct 26, 2023

Fixes #4515

Modifications

Add test case for ProtocolPluginFactory.

Field modifiersField = Field.class.getDeclaredField("modifiers");
modifiersField.setAccessible(true);
modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL);
field.set(null, mockProtocolAdaptorMap);
Copy link
Member

@pandaapo pandaapo Oct 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After you modify the value of the private static final property of ProtocolPluginFactory in this way, should you eventually change the value back to the original value? If not, will it affect the normal use of this attribute?


你这样修改了ProtocolPluginFactory类的private static final属性值以后,最终是否应该再将属性值改回原值?否则是不是就影响了该属性的正常使用?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although it does not affect normal use, I handled this with @AfterEach.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You still haven't fixed the issue I mentioned in your new submission. After you changed the final attribute value to mockProtocolAdaptorMap, it ultimately did not change back to its original value. Affects normal use.


你在新的提交中,并没有修复我说的问题。该final属性值被改成mockProtocolAdaptorMap后,最终没有改回原值。影响正常使用。

public void testGetProtocolAdaptorWhenMapNotEmpty() {
ProtocolAdaptor<ProtocolTransportObject> adaptor = ProtocolPluginFactory.getProtocolAdaptor(PROTOCOL_TYPE_NAME);
Assertions.assertEquals(adaptor.getClass(), MockProtocolAdaptorImpl.class);
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by 'MapNotEmpty' in this test? And what does the other 'BothEmpty' mean?


这个测试你所说的‘MapNotEmpty’是指什么?以及另一个的‘BothEmpty’是指什么?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It has been changed to a clearer name.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the testing logic of this method be merged to the beginning of testGetProtocolAdaptorWithAdaptorMap() without the need to define a separate method?


这个方法的测试逻辑,是不是可以合并到testGetProtocolAdaptorWithAdaptorMap()最前面,而没必要单独定义一个方法?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@codecov
Copy link

codecov bot commented Oct 27, 2023

Codecov Report

Merging #4516 (db536e5) into master (ae76d75) will decrease coverage by 0.07%.
Report is 2 commits behind head on master.
The diff coverage is 41.37%.

❗ Current head db536e5 differs from pull request most recent head 3fac2a3. Consider uploading reports for the commit 3fac2a3 to get more accurate results

@@             Coverage Diff              @@
##             master    #4516      +/-   ##
============================================
- Coverage     15.97%   15.90%   -0.07%     
+ Complexity     1549     1545       -4     
============================================
  Files           727      730       +3     
  Lines         28872    28891      +19     
  Branches       2744     2743       -1     
============================================
- Hits           4613     4596      -17     
- Misses        23776    23813      +37     
+ Partials        483      482       -1     
Files Coverage Δ
...java/org/apache/eventmesh/filter/PatternEntry.java 85.71% <ø> (ø)
...entmesh/filter/condition/AnythingButCondition.java 20.00% <100.00%> (ø)
.../eventmesh/filter/condition/ConditionsBuilder.java 80.95% <100.00%> (ø)
...he/eventmesh/filter/condition/ExistsCondition.java 75.00% <ø> (ø)
...e/eventmesh/filter/condition/NumericCondition.java 44.44% <ø> (ø)
...he/eventmesh/filter/condition/PrefixCondition.java 100.00% <100.00%> (ø)
...eventmesh/filter/condition/SpecifiedCondition.java 100.00% <ø> (ø)
...he/eventmesh/filter/condition/SuffixCondition.java 100.00% <ø> (ø)
...a/org/apache/eventmesh/filter/pattern/Pattern.java 65.38% <ø> (ø)
.../eventmesh/filter/patternbuild/PatternBuilder.java 72.28% <ø> (ø)
... and 10 more

... and 11 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@xwm1992 xwm1992 merged commit a091017 into apache:master Oct 30, 2023
7 checks passed
@xwm1992 xwm1992 added this to the 1.10 milestone Dec 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Enhancement] Add test case for ProtocolPluginFactory.
4 participants