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.

Sign up for GitHub

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

Jump to bottom

[UNDERTOW-1889] switch to mockito #1576

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[UNDERTOW-1889] switch to mockito #1576

wants to merge 1 commit into from

Conversation

baranowb
Copy link
Contributor

@baranowb baranowb commented Apr 8, 2024

@baranowb baranowb added enhancement Enhances existing behaviour or code under verification Currently being verified (running tests, reviewing) before posting a review to contributor waiting CI check Ready to be merged but waiting for CI check labels Apr 8, 2024
@baranowb baranowb requested a review from fl4via April 8, 2024 09:17
@baranowb baranowb removed the waiting CI check Ready to be merged but waiting for CI check label Apr 11, 2024
rhusar
rhusar suggested changes Apr 19, 2024
View reviewed changes
core/pom.xml
Comment on lines +131 to +132
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

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

This makes no sense - don't add a dependency that is never used. If a subsequent change will require this dependency, the onus will be on them.

Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @rhusar - the org.easymock dependency should be removed if it is not used at all.

ropalka
ropalka requested changes Aug 6, 2024
View reviewed changes
core/pom.xml
Comment on lines +131 to +132
<groupId>org.mockito</groupId>
<artifactId>mockito-core</artifactId>
Copy link
Contributor

Choose a reason for hiding this comment

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

I agree with @rhusar - the org.easymock dependency should be removed if it is not used at all.

@ropalka ropalka added the waiting PR update Awaiting PR update(s) from contributor before merging label Aug 6, 2024
@fl4via
Copy link
Member

fl4via commented Oct 21, 2024

I agree that we need to have dependencies that are only being used.
My two cents, @baranowb : if you plan to add a mock of a final class, add it to the same PR. If not, close this PR for now and reopen it when you are ready to submit a testcase that mocks a final class.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rhusar rhusar rhusar requested changes

@ropalka ropalka ropalka requested changes

@fl4via fl4via Awaiting requested review from fl4via

Assignees
No one assigned
Labels
enhancement Enhances existing behaviour or code under verification Currently being verified (running tests, reviewing) before posting a review to contributor waiting PR update Awaiting PR update(s) from contributor before merging
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@baranowb @fl4via @ropalka @rhusar