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

Upgrading mockito version in OpenSearch #1016

Closed
wants to merge 1 commit into from

Conversation

VachaShah
Copy link
Collaborator

Signed-off-by: Vacha Shah vachshah@amazon.com

Description

Upgrading the mockito-core version in OpenSearch from 1.9.5 to 3.11.2.

Issues Resolved

#1008

Check List

  • New functionality includes testing.
    • All tests pass
  • New functionality has been documented.
    • New functionality has javadoc added
  • Commits are signed per the DCO using --signoff

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Vacha Shah <vachshah@amazon.com>
@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Wrapper Validation success 95384d2

@opensearch-ci-bot
Copy link
Collaborator

✅   DCO Check Passed 95384d2

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Precommit success 95384d2

@adnapibar
Copy link
Contributor

start gradle check

@opensearch-ci-bot
Copy link
Collaborator

✅   Gradle Check success 95384d2
Log 354

Reports 354

@dblock
Copy link
Member

dblock commented Jul 27, 2021

I'm amazed that we did two major version bumps for mockito, and everything just worked!?

@VachaShah
Copy link
Collaborator Author

I'm amazed that we did two major version bumps for mockito, and everything just worked!?

Yeah the direct dependencies worked fine for the upgrade.

@vrozov
Copy link
Contributor

vrozov commented Jul 28, 2021

It does not look that the change updates mockito version globally on the OpenSearch project. The change is local to build-tools module. The mocking framework is provided by org.elasticsearch:securemock:1.2 that is based on the old 1.9.5 mockito-core version.

@VachaShah
Copy link
Collaborator Author

It does not look that the change updates mockito version globally on the OpenSearch project. The change is local to build-tools module. The mocking framework is provided by org.elasticsearch:securemock:1.2 that is based on the old 1.9.5 mockito-core version.

Yes the securemock update is part of issue #114. This PR is to upgrade the direct dependency for mockito.

@vrozov
Copy link
Contributor

vrozov commented Jul 28, 2021

IMO, it is confusing to have two different versions (2 major versions apart) in a single project. Will it be better to upgrade both dependencies at the same time or use the same securemock dependency in all modules?

@vrozov
Copy link
Contributor

vrozov commented Jul 28, 2021

The dependency seems to be used only by PluginBuildPluginTests where mockito is used simply to mock BwcVersions class without mocking any of its methods. It seems that such usage can be replaced with null.

@VachaShah
Copy link
Collaborator Author

IMO, it is confusing to have two different versions (2 major versions apart) in a single project. Will it be better to upgrade both dependencies at the same time or use the same securemock dependency in all modules?

You are right, we can do this as part of securemock. I will close this PR.

@VachaShah VachaShah closed this Jul 28, 2021
@VachaShah VachaShah deleted the upgrade_mockito branch August 17, 2021 16:58
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.

5 participants