Skip to content

MockFactory via ServiceLoader - #1746

Merged
leonard84 merged 2 commits into
spockframework:masterfrom
AndreasTu:MockFactory-via-ServiceLoader
Sep 15, 2023
Merged

MockFactory via ServiceLoader#1746
leonard84 merged 2 commits into
spockframework:masterfrom
AndreasTu:MockFactory-via-ServiceLoader

Conversation

@AndreasTu

Copy link
Copy Markdown
Member

Added API IMockMaker to make the mock creation pluggable via the ServiceLoader pattern.
The MockMakerRegistry provides access to the different IMockMaker.

This fixes #1220

@codecov

codecov Bot commented Aug 7, 2023

Copy link
Copy Markdown

Codecov Report

Patch coverage: 96.26% and project coverage change: +0.46% 🎉

Comparison is base (b749611) 79.87% compared to head (2423c59) 80.34%.

Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1746      +/-   ##
============================================
+ Coverage     79.87%   80.34%   +0.46%     
- Complexity     4100     4192      +92     
============================================
  Files           425      431       +6     
  Lines         12969    13157     +188     
  Branches       1644     1670      +26     
============================================
+ Hits          10359    10571     +212     
+ Misses         1998     1972      -26     
- Partials        612      614       +2     
Files Changed Coverage Δ
...pock-core/src/main/java/spock/mock/MockingApi.java 2.88% <ø> (ø)
...pockframework/mock/runtime/ByteBuddyMockMaker.java 80.00% <80.00%> (ø)
...va/org/spockframework/mock/runtime/IMockMaker.java 92.50% <92.50%> (ø)
...ckframework/mock/runtime/MockCreationSettings.java 95.45% <95.45%> (ø)
...spockframework/mock/runtime/MockMakerRegistry.java 97.84% <97.84%> (ø)
...rc/main/java/org/spockframework/mock/MockUtil.java 100.00% <100.00%> (ø)
...ckframework/mock/runtime/ByteBuddyMockFactory.java 91.78% <100.00%> (+0.23%) ⬆️
.../spockframework/mock/runtime/CglibMockFactory.java 60.00% <100.00%> (+60.00%) ⬆️
...rg/spockframework/mock/runtime/CglibMockMaker.java 100.00% <100.00%> (ø)
...spockframework/mock/runtime/GroovyMockFactory.java 72.97% <100.00%> (-0.72%) ⬇️
... and 9 more

... and 4 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@AndreasTu

AndreasTu commented Aug 7, 2023

Copy link
Copy Markdown
Member Author

I chose the name IMockMaker similar as in Mockito, because the I found the word Factory clases with the existing JavaMockFactory/GroovyMockFactory.

Missing stuff & discussions:

  • name of the property mockMaker: in Mock()/Stub()/Spy() API
  • Configuration of the MockMaker
  • The hard coded priorities for the internal IMockMaker
  • Documentation
  • Proof of concept, if the new API could now integrate the Mockito inline-mock-maker

@leonard84 leonard84 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This looks promising. Here are my first comments. I haven't yet thought about every aspect.

@spockframework/supporter any comments on the usability side?

Comment thread spock-core/src/main/java/org/spockframework/mock/runtime/CgLibMockMaker.java Outdated
Comment thread spock-core/src/main/java/org/spockframework/mock/runtime/MockMakerRegistry.java Outdated
Comment thread spock-core/src/main/java/org/spockframework/mock/runtime/MockMakerRegistry.java Outdated
Comment thread spock-core/src/main/java/org/spockframework/mock/runtime/MockMakerRegistry.java Outdated
@AndreasTu
AndreasTu requested a review from leonard84 August 7, 2023 18:47
@AndreasTu
AndreasTu force-pushed the MockFactory-via-ServiceLoader branch from c4a7139 to c5cb8cc Compare August 9, 2023 12:06
@AndreasTu

Copy link
Copy Markdown
Member Author

The PR #1753 contains the code to implement final class/final method mocking with Mockito.
The Pr implements an IMockMaker using Mockito API.

@leonard84
leonard84 requested a review from Vampire August 9, 2023 20:44
@AndreasTu
AndreasTu force-pushed the MockFactory-via-ServiceLoader branch from c5cb8cc to 821af01 Compare August 11, 2023 09:13
@AndreasTu
AndreasTu marked this pull request as ready for review August 11, 2023 09:22
@AndreasTu
AndreasTu force-pushed the MockFactory-via-ServiceLoader branch 2 times, most recently from 9b9e96d to f9a3e66 Compare August 13, 2023 14:05
@AndreasTu

Copy link
Copy Markdown
Member Author

The PR #1756 contains the code to support static method mocking with the mockito-inline-mock-maker introduced in #1753.

@AndreasTu
AndreasTu force-pushed the MockFactory-via-ServiceLoader branch 4 times, most recently from 4b7ce46 to 9f665ac Compare August 13, 2023 20:45
@AndreasTu

Copy link
Copy Markdown
Member Author

@leonard84 I have changed the access to the MockMakerRegistry to RunContext.get().getMockMakerRegistry().
And I have changed the configuration of the preferredMockMaker to a ConfigurationObject named MockMakerRegistryConfiguration.

Usage sample:

//spock-configuration-file
mockMaker {
  preferredMockMaker = "your-preferred-mock-maker"
}

@AndreasTu
AndreasTu force-pushed the MockFactory-via-ServiceLoader branch from 9f665ac to 2231a34 Compare August 13, 2023 21:14
@AndreasTu AndreasTu self-assigned this Aug 14, 2023

@Vampire Vampire left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks good to me in general.
Just some detail points, and a lot of improvement suggestions. :-)

Comment thread docs/extensions.adoc Outdated
Comment thread docs/extensions.adoc Outdated
Comment thread docs/extensions.adoc Outdated
Comment thread docs/extensions.adoc Outdated
Comment thread docs/extensions.adoc Outdated
Comment thread spock-core/src/main/java/org/spockframework/mock/runtime/IMockMaker.java Outdated
Comment thread spock-core/src/main/java/org/spockframework/mock/runtime/MockMakerRegistry.java Outdated
Comment thread docs/extensions.adoc Outdated
Comment thread docs/extensions.adoc Outdated
Comment thread spock-core/src/main/java/org/spockframework/runtime/RunContext.java Outdated
@AndreasTu
AndreasTu requested review from Vampire and leonard84 August 22, 2023 19:34

@Vampire Vampire left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks already very good imho.
Just some more details. :-)

Comment thread docs/extensions.adoc Outdated
Comment thread docs/extensions.adoc Outdated
Comment thread docs/interaction_based_testing.adoc Outdated
Comment thread spock-core/src/main/java/org/spockframework/mock/runtime/IMockMaker.java Outdated
@Vampire

Vampire commented Aug 23, 2023

Copy link
Copy Markdown
Member

Mea culpa, tests were broken due to bad review, I fixed them

@AndreasTu
AndreasTu requested a review from Vampire August 23, 2023 12:21
@AndreasTu

Copy link
Copy Markdown
Member Author

@Vampire I hope I have nothing forgotten.
GitHub gets a bit tricky and sluggish, with so much stuff going on in a PR.

Comment thread docs/extensions.adoc Outdated
Comment thread spock-core/src/main/java/org/spockframework/util/ObjectUtil.java
@AndreasTu
AndreasTu requested a review from Vampire August 23, 2023 13:05
@Vampire

Vampire commented Aug 23, 2023

Copy link
Copy Markdown
Member

I hope I have nothing forgotten.

I think from my side areis currently only twoone points left.
The two conversations I re-opened.
#1746 (comment) where @leonard84 should reply
and #1746 (comment).

@AndreasTu
AndreasTu force-pushed the MockFactory-via-ServiceLoader branch from 4a186c8 to ac2fae6 Compare September 1, 2023 16:50
leonard84
leonard84 previously approved these changes Sep 15, 2023

@leonard84 leonard84 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@AndreasTu all looks good now, I tried to rebase it so that I can directly merge it, but the resulting merge and changes were non-trivial. If you be so kind to resolve them, either by rebase or merge (they will be squash merged in the end).

Also it would be nice, if you could write an entry for the release notes.

Vampire
Vampire previously approved these changes Sep 15, 2023
Added API IMockMaker to make the mock creation pluggable
via the ServiceLoader pattern.
The MockMakerRegistry provides access to the different IMockMaker.

Added IMockMakerSettings to provide a way to pass additional type-safe arguments to the IMockMaker.

This fixes spockframework#1220

Co-authored-by: Leonard Brünings <lord_damokles@gmx.net>
Co-authored-by: Björn Kautler <Bjoern@Kautler.net>
@AndreasTu
AndreasTu dismissed stale reviews from Vampire and leonard84 via d28862c September 15, 2023 17:52
@AndreasTu
AndreasTu force-pushed the MockFactory-via-ServiceLoader branch from 0473ec8 to d28862c Compare September 15, 2023 17:52
@AndreasTu

Copy link
Copy Markdown
Member Author

@leonard84 I have rebased the PR.

@leonard84
leonard84 enabled auto-merge (squash) September 15, 2023 18:01
@leonard84

Copy link
Copy Markdown
Member

@AndreasTu thanks for all the work

@leonard84
leonard84 merged commit 42f730d into spockframework:master Sep 15, 2023
@AndreasTu
AndreasTu deleted the MockFactory-via-ServiceLoader branch September 15, 2023 19:00
@AndreasTu AndreasTu added this to the 2.4 milestone Feb 18, 2024
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.

MockFactory via ServiceLoader

3 participants