Skip to content

Discard unnecessary state in ConfineMetaClassChangesInterceptor#1460

Merged
leonard84 merged 2 commits intospockframework:masterfrom
mattmoss:master
May 4, 2022
Merged

Discard unnecessary state in ConfineMetaClassChangesInterceptor#1460
leonard84 merged 2 commits intospockframework:masterfrom
mattmoss:master

Conversation

@mattmoss
Copy link
Contributor

If @ConfineMetaClassChanges is used on a data-driver feature method, the
ConfineMetaClassChangesInterceptor is shared between iterations of the
method. Clear the originalMetaClasses field of the interceptor after
each invocation to avoid populating the list with duplicate references
to original meta-classes.

The previous implementation was not defective per se, but this fixes a
potential efficiency issue (determined by how many meta-classes are
stored and how many iterations of the feature method are run).

@leonard84
Copy link
Member

Looking at the original code, originalMetaClasses doesn't have to be a field but could be a local variable, then we wouldn't have to clear it at the end.

@mattmoss
Copy link
Contributor Author

Looking at the original code, originalMetaClasses doesn't have to be a field but could be a local variable, then we wouldn't have to clear it at the end.

Agreed, good catch. Will revise.

Matthew Moss added 2 commits May 4, 2022 11:38
If @ConfineMetaClassChanges is used on a data-driver feature method, the
ConfineMetaClassChangesInterceptor is shared between iterations of the
method. Clear the originalMetaClasses field of the interceptor after
each invocation to avoid populating the list with duplicate references
to original meta-classes.

The previous implementation was not defective, per se, but this fixes a
potential efficiency issue (determined by how many meta-classes are
stored and how many iterations of the feature method are run).
@codecov
Copy link

codecov bot commented May 4, 2022

Codecov Report

Merging #1460 (f67481d) into master (a73fa94) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##             master    #1460   +/-   ##
=========================================
  Coverage     79.59%   79.59%           
  Complexity     4010     4010           
=========================================
  Files           404      404           
  Lines         12563    12563           
  Branches       1641     1641           
=========================================
  Hits           9999     9999           
  Misses         1969     1969           
  Partials        595      595           
Impacted Files Coverage Δ
...on/builtin/ConfineMetaClassChangesInterceptor.java 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a73fa94...f67481d. Read the comment docs.

@leonard84 leonard84 changed the title Reset state after intercept of feature method Discard unnecessary state in ConfineMetaClassChangesInterceptor May 4, 2022
@leonard84 leonard84 merged commit 5a5e575 into spockframework:master May 4, 2022
@leonard84
Copy link
Member

thanks @mattmoss

leonard84 added a commit to kriegaex/spock that referenced this pull request May 21, 2022
* master:
  Bump org.gradle.test-retry from 1.3.2 to 1.4.0
  Make EnableSharedInjection public (spockframework#1472)
  Remove runtime dependency on Jetbrains Annotations (spockframework#1468)
  Add Dependabot configuration
  Use static imports for well known methods of Collections and Arrays
  Refactor SpecInfo extract shared logic into collectSpecHierarchy
  Discard unnecessary state in ConfineMetaClassChangesInterceptor (spockframework#1460)
  Update Groovy to 4.0.2
  Update Gradle plugins
  Update Gradle to 7.4.2
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.

2 participants