Fix the IStoreProvider impl. in SpecContext...

[leonard84]

The SpecificationContext always belongs to a single spec instance,
so it is either the shared Specification instance or it belongs to
the iteration. There is no Feature-level instance, so we don't need
to use a stack.

[Vampire]

What happens on feature level then?
Do you get the specification level store?
It would be sad to not have a feature level store that is valid for all iterations.
And if this is really the case, then the documentation also needs to be adapted.
But it would really be nicer if a feature-level store would exist.
Or did I get the text and changes wrong?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.87%. Comparing base (b57816a) to head (24bc224).
Report is 1 commits behind head on master.

Additional details and impacted files

@@             Coverage Diff              @@
##             master    #2071      +/-   ##
============================================
- Coverage     81.88%   81.87%   -0.01%     
+ Complexity     4687     4686       -1     
============================================
  Files           452      452              
  Lines         14666    14658       -8     
  Branches       1845     1845              
============================================
- Hits          12009    12001       -8     
  Misses         1976     1976              
  Partials        681      681              

Files with missing lines	Coverage Δ
...org/spockframework/runtime/PlatformSpecRunner.java	93.14% <100.00%> (-0.14%)	⬇️
...g/spockframework/runtime/SpecificationContext.java	100.00% <100.00%> (ø)

[leonard84]

There is a feature-level store, but you can only access it directly via a feature interceptor. In the spec itself, you can't directly access the feature store. However, the stores are hierarchic, so it will ask its parents if it can't find an item. You can also call getParent() from the iteration to get to the feature level.

[Vampire]

Ah, so in a feature interceptor you have to do invocation.getStore to get the feature-level store which works as the invocation gets it from SpockExecutionContext.
And in a specification itself you would need to do specificationContext.getStore and there of course get the iteration-level one.

On feature-level you currently "could" do invocation.instance.specificationContext.getStore(...) which does not work properly due to the spec context being the shared one, just like it did not work for the currentFeature and instead you have to do invocation.getStore(...).

So just like you can "again" not get the currentFeature in the feature interceptors spec context you can with this change also not get the store anymore, but have to retrieve it from the invocation to get the feature level store.

If I understood it right now, I think the change is fine so far.

A protection like for SpecificationContext#getCurrentFeature that throws an exception for this probably does not make sense, as you can get a valid store, it might just not be the one you expect if you do it from a feature interceptor. Maybe it would at least make sense to add a warning to the docs that clearly states that you get the spec-level store if you get it from the spec context there and instead should use the invocation?

[Vampire]

Hm, actually right now I still wonder how the problem happened and whether there is still something wrong.
Even with the stack being used on the shared spec context for the feature-level.
Shouldn't there still only be as many pops as previous pushes had happened?

[Vampire]

Ah, no, there then was the problem that the stack was not thread-safe and the pushes and pops happened on varying threads during parallel execution, all fine without the stack.

[Vampire]

Did you see and discard my comment about doc, or missed it? :-)