Enable implicit non-top-level conditions (#1160)

[Vampire]

Fixes #1160

[Vampire]

• #2250 👈 (View in Graphite)
• master

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

[Vampire]

While fixing the verify methods also stumbled over the sole line that needs to be changed to enable non-top-level implicit conditions. Turns out this was more an arbitrary restriction that was very easy to change and also only one of our tests needed adaption because it relied on impicit condition being non-effective inside an if. :-)

[Vampire]

Ok, two tests. Good that we have the !! available now. :-)

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 82.07%. Comparing base (158e2e4) to head (b38799d).

Additional details and impacted files

@@            Coverage Diff            @@
##             master    #2250   +/-   ##
=========================================
  Coverage     82.07%   82.07%           
+ Complexity     4754     4753    -1     
=========================================
  Files           465      465           
  Lines         14872    14873    +1     
  Branches       1877     1877           
=========================================
+ Hits          12206    12207    +1     
  Misses         1978     1978           
  Partials        688      688           

Files with missing lines	Coverage Δ
...kframework/compiler/AbstractDeepBlockRewriter.java	98.75% <100.00%> (ø)
...org/spockframework/compiler/DeepBlockRewriter.java	98.19% <100.00%> (ø)
...g/spockframework/compiler/NoSpecialMethodCall.java	54.16% <100.00%> (+1.99%)	:arrow_up:

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[AndreasTu]

Very nice, but should we maybe wait for or plan a Spock 3 version? I think this change in 2.4 will hinder the adoption of the new version. And the 2.3 is quite old by now.

So I would be for delaying that PR until we released 2.4 finally and then we plan a 3.0 as next version.

[leonard84]

While I also want to get rid of this confusing behavior, I think we should postpone it until Spock 3.0, as the breaking change is quite significant.

[Vampire]

Up to @leonard84 I'd say. I don't care when it comes, I just stumbled over the line and thought I'll create a PR right away before I forget the details again, as I know Leonard wants to have this change.

In my personal opinion I think the change is not that much breaking. I'd expect that most cases where this is breaking existing code is, where it should always better failed because someone checked a condition on non-top-level and thought the test is green while it is actually red.

And the cases where this is not the case, but the behavior used like in the two cases we had in the codebase, you can easily use !! to disable the implicit assertion.

We were seldomly shy to do breaking changes, why become shy now? :-)

If you really think it is too breaking to bring it in 2.4 now, no problem, just delay it until later. It just then needs another 3 years until it reaches the users. (I consider "normal" users here that do not use an M-release but only final releases). 🤷‍♂️

As I said, I just thought I'll do the changes now that I have the details and easy solution in mind. and did it in a way it could still make it into 2.4 (regarding the release notes changes).