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

Upgrade mockito #7577

Merged
merged 4 commits into from
Jun 12, 2023
Merged

Upgrade mockito #7577

merged 4 commits into from
Jun 12, 2023

Conversation

vishesh92
Copy link
Member

@vishesh92 vishesh92 commented Jun 1, 2023

Description

This PR upgrades the mockito version and removes powermockito from utils module.

Moving to Mockito to mock static methods (available since 3.4.0) instead of powermockito since it hasn't had any release since 2020 and is not compatible with newer versions of mockito.

I will create separate PRs for other modules overtime and eventually remove powermockito.

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

@harikrishna-patnala
Copy link
Contributor

@blueorangutan package

@blueorangutan
Copy link

@harikrishna-patnala a [SF] Jenkins job has been kicked to build packages. It will be bundled with
SystemVM template(s). I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [SF]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6193

@codecov
Copy link

codecov bot commented Jun 1, 2023

Codecov Report

Merging #7577 (d87c62a) into main (8d6241f) will increase coverage by 0.00%.
The diff coverage is n/a.

❗ Current head d87c62a differs from pull request most recent head 99afb91. Consider uploading reports for the commit 99afb91 to get more accurate results

@@            Coverage Diff            @@
##               main    #7577   +/-   ##
=========================================
  Coverage     12.96%   12.96%           
- Complexity     8998     8999    +1     
=========================================
  Files          2728     2728           
  Lines        256667   256667           
  Branches      40028    40028           
=========================================
+ Hits          33276    33282    +6     
+ Misses       219210   219205    -5     
+ Partials       4181     4180    -1     

see 6 files with indirect coverage changes

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

@@ -113,7 +113,7 @@
<cs.junit.dataprovider.version>1.13.1</cs.junit.dataprovider.version>
<cs.junit.jupiter.version>5.9.1</cs.junit.jupiter.version>
<cs.guava-testlib.version>18.0</cs.guava-testlib.version>
<cs.mockito.version>3.2.4</cs.mockito.version>
Copy link
Contributor

@harikrishna-patnala harikrishna-patnala Jun 1, 2023

Choose a reason for hiding this comment

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

we may also need the following change, so mockstatic() can be used with Mockito itself.

- <artifactId>mockito-core</artifactId>
+ <artifactId>mockito-inline</artifactId>

Let's first run tests and see nothing is broken

Copy link
Contributor

Choose a reason for hiding this comment

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

@harikrishna-patnala tests have been run in the github acrions. Do you need to see anything else?

Copy link
Member Author

Choose a reason for hiding this comment

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

@harikrishna-patnala Let me make this change. Then we can incrementally remove PowerMockito.

@vishesh92 vishesh92 marked this pull request as ready for review June 5, 2023 19:25
Copy link
Contributor

@DaanHoogland DaanHoogland left a comment

Choose a reason for hiding this comment

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

clgtm

@@ -0,0 +1 @@
mock-maker-inline
Copy link
Member

Choose a reason for hiding this comment

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

@vishesh92 is this boilerplate or does it serve any purpose? If you want an empty file you can touch the file and git add.

Copy link
Member Author

Choose a reason for hiding this comment

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

We will need to add this file to module when we remove powermock. It specifies the MockMaker to use in that module for running tests.

more info here: https://github.com/apache/cloudstack/pull/7577/files#r1217685958

@rohityadavcloud rohityadavcloud added this to the 4.19.0.0 milestone Jun 6, 2023
@rohityadavcloud
Copy link
Member

@blueorangutan package

@blueorangutan
Copy link

@rohityadavcloud a [LL] Jenkins job has been kicked to build packages. It will be bundled with KVM, XenServer and VMware SystemVM templates. I'll keep you posted as I make progress.

@blueorangutan
Copy link

Packaging result [LL]: ✔️ el7 ✔️ el8 ✔️ el9 ✔️ debian ✔️ suse15. SL-JID 6099

@DaanHoogland DaanHoogland merged commit 5fda9c3 into apache:main Jun 12, 2023
@vishesh92 vishesh92 deleted the upgrade-mockito branch March 25, 2024 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants