Skip to content

Re-enable testGetProtocolAdaptor - #5435

Open
sharanggupta wants to merge 1 commit into
apache:developfrom
sharanggupta:reenable-protocol-plugin-factory-test
Open

sharanggupta wants to merge 1 commit into
apache:developfrom
sharanggupta:reenable-protocol-plugin-factory-test

Conversation

@sharanggupta

Copy link
Copy Markdown

Motivation

testGetProtocolAdaptor has been commented out since #5206 because it replaced the static final PROTOCOL_ADAPTOR_MAP by reflection, which the JDK rejects.

Modifications

Put the mock adaptor into the existing map and remove it in a finally, so the cached-adaptor and SPI-loading paths of ProtocolPluginFactory.getProtocolAdaptor are tested again.

Documentation

  • Does this pull request introduce a new feature? (no)

The test was commented out in apache#5206 because it replaced the static
final PROTOCOL_ADAPTOR_MAP by reflection, which the JDK rejects. Put
the mock adaptor into the existing map instead.

Signed-off-by: Sharang Gupta <sharang@sharanggupta.dev>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Welcome to the Apache EventMesh community!!
This is your first PR in our project. We're very excited to have you onboard contributing. Your contributions are greatly appreciated!

Please make sure that the changes are covered by tests.
We will be here shortly.
Let us know if you need any help!

Want to get closer to the community?

WeChat Assistant WeChat Public Account Slack
Join Slack Chat

Mailing Lists:

Name Description Subscribe Unsubscribe Archive
Users User support and questions mailing list Subscribe Unsubscribe Mail Archives
Development Development related discussions Subscribe Unsubscribe Mail Archives
Commits All commits to repositories Subscribe Unsubscribe Mail Archives
Issues Issues or PRs comments and reviews Subscribe Unsubscribe Mail Archives

This branch has not been deployed

No deployments
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.

1 participant