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

Allow to create Groovy spies with existing instances #1825

Merged
merged 1 commit into from
Mar 17, 2024

Conversation

Vampire
Copy link
Member

@Vampire Vampire commented Nov 8, 2023

No description provided.

Copy link
Member Author

Vampire commented Nov 8, 2023

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @Vampire and the rest of your teammates on Graphite Graphite

Copy link

codecov bot commented Nov 8, 2023

Codecov Report

Attention: Patch coverage is 40.00000% with 6 lines in your changes are missing coverage. Please review.

Project coverage is 80.33%. Comparing base (2c7db77) to head (18556f9).
Report is 51 commits behind head on master.

❗ Current head 18556f9 differs from pull request most recent head af671b7. Consider uploading reports for the commit af671b7 to get more accurate results

Files Patch % Lines
...pock-core/src/main/java/spock/mock/MockingApi.java 0.00% 4 Missing ⚠️
...in/java/org/spockframework/lang/SpecInternals.java 66.66% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@             Coverage Diff              @@
##             master    #1825      +/-   ##
============================================
- Coverage     80.44%   80.33%   -0.12%     
+ Complexity     4337     4309      -28     
============================================
  Files           441      439       -2     
  Lines         13534    13484      -50     
  Branches       1707     1702       -5     
============================================
- Hits          10888    10832      -56     
  Misses         2008     2008              
- Partials        638      644       +6     

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

@Vampire Vampire force-pushed the vampire/groovy-spies-on-instances branch from a6b71e1 to aa2ae90 Compare November 9, 2023 12:11
@Vampire Vampire force-pushed the vampire/groovy-spies-on-instances branch from aa2ae90 to 42a42b8 Compare November 13, 2023 00:50
@Vampire Vampire force-pushed the vampire/groovy-spies-on-instances branch from 42a42b8 to bed0214 Compare November 27, 2023 21:59
Base automatically changed from add-missing-closure-hints to master November 27, 2023 23:18
@Vampire Vampire force-pushed the vampire/groovy-spies-on-instances branch 4 times, most recently from bdf291f to e6a1853 Compare December 1, 2023 01:21
@Vampire Vampire force-pushed the vampire/groovy-spies-on-instances branch 2 times, most recently from 1a546b9 to 18556f9 Compare December 18, 2023 12:50

class GroovySpies extends Specification {
def "can spy on concrete instances"() {
def person = GroovySpy(new Person())
Copy link
Member

Choose a reason for hiding this comment

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

Is this testing that it works? When you don't customize the instance, how would you notice the difference to the parameter being ignored?

Copy link
Member Author

Choose a reason for hiding this comment

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

Afair I just ported over the tests from JavaSpies where it is done the same.

Copy link
Member Author

Choose a reason for hiding this comment

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

Also, I'm not sure what you mean.
Calling the method on the Spy calls through to the original object.
What should be customized here?
That the Spy can intercept and change things is tested further down.

Copy link
Member

Choose a reason for hiding this comment

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

I was talking about creating Person with non-default constructor values. Otherwise, it would behave the same as GroovySpy().

Copy link
Member Author

Choose a reason for hiding this comment

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

What would that change?
The tested method is not influenced by the constructor arguments or any property of the class.
Again, I just ported over the test from JavaSpies, assuming that whoever invented them had a reason to write them like that.

@Vampire Vampire force-pushed the vampire/groovy-spies-on-instances branch 2 times, most recently from 0139d22 to 473ef9f Compare March 10, 2024 16:20
@Vampire Vampire force-pushed the vampire/groovy-spies-on-instances branch from 473ef9f to 15c42b0 Compare March 16, 2024 16:37
@Vampire Vampire force-pushed the vampire/groovy-spies-on-instances branch from 15c42b0 to af671b7 Compare March 17, 2024 19:06
@leonard84 leonard84 merged commit 66e55b2 into master Mar 17, 2024
38 checks passed
@leonard84 leonard84 deleted the vampire/groovy-spies-on-instances branch March 17, 2024 21:15
@leonard84 leonard84 added this to the 2.4-M3 milestone Mar 17, 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.

None yet

2 participants