Test the interceptor lifecycle

[Vampire]

No description provided.

[Vampire]

Current dependencies on/for this PR:

• master
  • PR Fix docs how to register all possible interceptors #1824
    • PR Test the interceptor lifecycle #1668 👈
      • PR Add SpecInfo#getAll...Methods() methods #1770
        • PR Fix the lifecycle of simple features #1675
          • PR Verify and fix interceptor contexts  #1676
            • PR Verify the feature is given to method invocations #1677
              • PR Verify the iteration is given to method invocations #1678
                • PR Verify the instances are given correctly to method invocations #1679
                  • PR Verify the target is given correctly to method invocations #1680
                  • PR Verify the method is given to method invocations #1681
                  • PR Verify the method reflection is given to method invocations #1682
                  • PR Verify the method name is given to method invocations #1683
                  • PR Verify the specification is given to specification context #1684
                  • PR Verify the feature is given to specification context #1685
                  • PR Verify the iteration is given to specification context #1686
      • PR Add SpecInfo#getAllSharedInitializerMethods() #1669
        • PR Add SpecInfo#getAllSetupSpecMethods() #1670
          • PR Add SpecInfo#getAllInitializerMethods() #1671
            • PR Add SpecInfo#getAllSetupMethods() #1672
              • PR Add SpecInfo#getAllCleanupMethods() #1673
                • PR Add SpecInfo#getAllCleanupSpecMethods() #1674

This stack of pull requests is managed by Graphite.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (1316827) 80.43% compared to head (586f1c1) 80.39%.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #1668      +/-   ##
============================================
- Coverage     80.43%   80.39%   -0.04%     
+ Complexity     4234     4233       -1     
============================================
  Files           434      434              
  Lines         13279    13279              
  Branches       1688     1688              
============================================
- Hits          10681    10676       -5     
- Misses         1960     1964       +4     
- Partials        638      639       +1     

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

[leonard84]

🤔 From a reporting POV it isn't ideal that you get the same spec name twice here, as one of the invoked methods is from the SuperSpec, although it is correct that the the feature belongs to the SubSpec. The same applied to the other methods.

[Vampire]

I agree, but that is nothing I change here.
I just add a test that verifies the current state, like I also do not fix the simple feature lifecycle here, but in a separate upstack PR.

So this simply is, what you get from IMethodInvocation#spec#name currently.

Maybe it should generally be reviewed whether the delivered information makes sense in all cases.

But actually I don't see how "one of the invoked methods is from the SuperSpec" here.
The super spec does not have any setup method.
It is just the super specs "wrapper around all setup methods" that is called for the setup phase of the sub spec.

If you have setup method in both sub spec and super spec like in the other feature, it looks like this:

setup start (SubSpec.subFeature1[#0])
  setup start (SubSpec.subFeature1[#0])
    setup method start (SuperSpec.subFeature1[#0].setup())
      fixture method start (SuperSpec.subFeature1[#0].setup())
      fixture method end (SuperSpec.subFeature1[#0].setup())
    setup method end (SuperSpec.subFeature1[#0].setup())
  setup end (SubSpec.subFeature1[#0])
  setup method start (SubSpec.subFeature1[#0].setup())
    fixture method start (SubSpec.subFeature1[#0].setup())
    fixture method end (SubSpec.subFeature1[#0].setup())
  setup method end (SubSpec.subFeature1[#0].setup())
setup end (SubSpec.subFeature1[#0])

But it might also be questionable whether this is correct.
Should it really say the spec is super, which matches the actual method called,
but the feature is the one from the sub spec which does not match the feature?

Maybe it would make more sense if spec here is indeed the sub spec, matching the feature you get, and if you want to know from where the method is from, you use method.parent?

I'm not sure how it best should be to be honest. For "shared initializer" for example, you get sub and super, not twice sub like here.

[Vampire]

Or maybe it makes sense that for the actual "...method" one the IMethodInvocation.spec gives the one matching that method as it is the main target in view currently, and if you want the features spec, you use feature.parent.

[Vampire]

Actually, in my other upstack PR I add tests for whether the fields are set at all or not,
but they don't verify to what they are set and whether the spec is consistent with the feature and so on. I guess another PR could introduce such checks later on and fix some of this if it is not like intended.

[Vampire]

I at least improved the reporting in the method-interceptor cases above now, so that the features are not attributed to the wrong specification.

[leonard84]

I was not saying that we should change anything on the IMethodInvocation side, but it would be possible to change

specInfo.specsBottomToTop*.addSetupInterceptor {
        proceed(it, 'setup', "$it.spec.name.$it.feature.name[#$it.iteration.iterationIndex]")
      }

to

specInfo.specsBottomToTop.each { spec -> spec.addSetupInterceptor {
        proceed(it, 'setup', "$spec.name.$it.feature.name[#$it.iteration.iterationIndex]")
      }
}

[leonard84]

I just add a test that verifies the current state, like I also do not fix the simple feature lifecycle here, but in a separate upstack PR.

So, the intention is to visualize which interceptors are called in what order. Given that the information in IMethodInvocation is no always sufficient for this, I think it makes sense to change the interceptor registrations to include the spec, like I've shown above.

[Vampire]

Oh, now I got it, yeah, makes absolute sense. Fixed now.