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

MINOR: Upgrade mockito test dependencies #12460

Merged
merged 1 commit into from Aug 9, 2022
Merged

MINOR: Upgrade mockito test dependencies #12460

merged 1 commit into from Aug 9, 2022

Conversation

dplavcic
Copy link
Contributor

@dplavcic dplavcic commented Jul 29, 2022

Changes

Why is this change needed?

Update 1

According to the Mockito documentation :

Although it is possible to verify a stubbed invocation, usually it's just redundant. Let's say you've stubbed foo.bar(). If your code cares what foo.bar() returns then something else breaks(often before even verify() gets executed). If your code doesn't care what get(0) returns then it should not be stubbed.

While working on the Replace EasyMock and PowerMock with Mockito for StreamsMetricsImplTest I noticed that described behavior wasn't applied when you create a new mock like this.

        final Metrics metrics = mock(Metrics.class);
        when(metrics.metric(metricName)).thenReturn(null);

        ... invoke SUT

        verify(metrics).metric(metricName); // this should be redundant (according to docs)

After further investigation I figured out that described behaviour wasn't implemented untilv4.6.1.

With this change we are now able to mock objects like this:

   Foo explicitStrictMock = mock(Foo.class, withSettings().strictness(Strictness.STRICT_STUBS));

Update 2

It looks like I can accomplish the same thing by using the @RunWith(MockitoJUnitRunner.StrictStubs.class) instead of the @RunWith(MockitoJUnitRunner.class) so mockito dependency version update is not mandatory, but it would be nice to stay up-to-date and use the latest version (it's up to MR reviewer to decide if we are going to merge this now, or just close the MR and update mockito version later).

Committer Checklist (excluded from commit message)

  • Verify design and implementation
  • Verify test coverage and CI build status
  • Verify documentation (including upgrade notes)

@dplavcic dplavcic marked this pull request as draft July 29, 2022 18:09
@dplavcic dplavcic marked this pull request as ready for review July 30, 2022 08:25
@ijuma
Copy link
Contributor

ijuma commented Aug 9, 2022

I'm guessing the 4.5.1 to 4.6.0 fix doesn't affect us since we're still on 4.4.0:

Regression? Strictness set in @MockitoSettings ignored after upgrade from 4.5.1 to 4.6.0 mockito/mockito#2656

Copy link
Contributor

@ijuma ijuma left a comment

Choose a reason for hiding this comment

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

LGTM, thanks.

@ijuma ijuma merged commit eeee8e2 into apache:trunk Aug 9, 2022
ijuma added a commit to confluentinc/kafka that referenced this pull request Aug 10, 2022
…(10 August 2022)

Trivial conflict in gradle/dependencies.gradle due to the newer Netty
version in confluentinc/kafka.

* apache-github/trunk:
MINOR: Upgrade gradle to 7.5.1 and bump other build/test dependencies
(apache#12495)
KAFKA-14140: Ensure an offline or in-controlled-shutdown replica is
not eligible to join ISR in ZK mode (apache#12487)
  KAFKA-14114: Add Metadata Error Related Metrics
MINOR: BrokerMetadataSnapshotter must avoid exceeding batch size
(apache#12486)
  MINOR: Upgrade mockito test dependencies (apache#12460)
KAFKA-14144:; Compare AlterPartition LeaderAndIsr before fencing
partition epoch (apache#12489)
KAFKA-14134: Replace EasyMock with Mockito for WorkerConnectorTest
(apache#12472)
  MINOR: Update scala version in bin scripts to 2.13.8 (apache#12477)
KAFKA-14104; Add CRC validation when iterating over Metadata Log
Records (apache#12457)
  MINOR: add :server-common test dependency to :storage (apache#12488)
  KAFKA-14107: Upgrade Jetty version for CVE fixes (apache#12440)
  KAFKA-14124: improve quorum controller fault handling (apache#12447)
ijuma added a commit to franz1981/kafka that referenced this pull request Aug 12, 2022
* apache-github/trunk: (447 commits)
  KAFKA-13959: Controller should unfence Broker with busy metadata log (apache#12274)
  KAFKA-10199: Expose read only task from state updater (apache#12497)
  KAFKA-14154; Return NOT_CONTROLLER from AlterPartition if leader is ahead of controller (apache#12506)
  KAFKA-13986; Brokers should include node.id in fetches to metadata quorum (apache#12498)
  KAFKA-14163; Retry compilation after zinc compile cache error (apache#12507)
  Remove duplicate common.message.* from clients:test jar file (apache#12407)
  KAFKA-13060: Replace EasyMock and PowerMock with Mockito in WorkerGroupMemberTest.java (apache#12484)
  Fix the rate window size calculation for edge cases (apache#12184)
  MINOR: Upgrade gradle to 7.5.1 and bump other build/test dependencies (apache#12495)
  KAFKA-14140: Ensure an offline or in-controlled-shutdown replica is not eligible to join ISR in ZK mode (apache#12487)
  KAFKA-14114: Add Metadata Error Related Metrics
  MINOR: BrokerMetadataSnapshotter must avoid exceeding batch size (apache#12486)
  MINOR: Upgrade mockito test dependencies (apache#12460)
  KAFKA-14144:; Compare AlterPartition LeaderAndIsr before fencing partition epoch (apache#12489)
  KAFKA-14134: Replace EasyMock with Mockito for WorkerConnectorTest (apache#12472)
  MINOR: Update scala version in bin scripts to 2.13.8 (apache#12477)
  KAFKA-14104; Add CRC validation when iterating over Metadata Log Records (apache#12457)
  MINOR: add :server-common test dependency to :storage (apache#12488)
  KAFKA-14107: Upgrade Jetty version for CVE fixes (apache#12440)
  KAFKA-14124: improve quorum controller fault handling (apache#12447)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants