-
Notifications
You must be signed in to change notification settings - Fork 642
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
Conversation
Field modifiersField = Field.class.getDeclaredField("modifiers"); | ||
modifiersField.setAccessible(true); | ||
modifiersField.setInt(field, field.getModifiers() & ~Modifier.FINAL); | ||
field.set(null, mockProtocolAdaptorMap); |
There was a problem hiding this comment.
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
属性值以后,最终是否应该再将属性值改回原值?否则是不是就影响了该属性的正常使用?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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); | ||
} |
There was a problem hiding this comment.
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’是指什么?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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()
最前面,而没必要单独定义一个方法?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
Codecov Report
@@ 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
... and 11 files with indirect coverage changes 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
Fixes #4515
Modifications
Add test case for ProtocolPluginFactory.