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 spec iterations to be skipped #1008

Closed
renatoathaydes opened this issue Jul 5, 2019 · 12 comments · Fixed by #1442
Closed

Allow spec iterations to be skipped #1008

renatoathaydes opened this issue Jul 5, 2019 · 12 comments · Fixed by #1442

Comments

@renatoathaydes
Copy link
Contributor

renatoathaydes commented Jul 5, 2019

Feature request

Spock does not currently support natively skipping a single iteration of an examples-based feature. The workaround mentioned in the documentation involves using JUnit's AssumptionViolatedException to handle such cases.

Spock already correctly reports such iterations as "skipped" to the JUnit Notifier, but not to Spock's own IRunListener (which, incidentally, causes a bug in spock-reports as it cannot know that a particular iteration of a feature was skipped).

It would be ideal to be able to use a Spock, backend-test-framework-independent way of doing this. Specifically, Spock's IRunListener should have a method, void iterationSkipped( IterationInfo iteration ), analogous to the existing void specSkipped( SpecInfo spec ).

Proposed solution 1

I would suggest adding a method to Specification like void skipIf(boolean) and/or assumeThat(boolean), that causes the current feature/iteration to be skipped when necessary.

Proposed solution 2

It might be more Spock-like to use a precondition block instead:

precondition: 'must be on Windows'
OS.isWindows()

Example

Here's a minimal specification demonstrating how feature iterations can be skipped today, but only when using JUnit APIs.

import org.junit.Assume
import spock.lang.Specification
import spock.lang.Unroll

class AssumptionViolatedTest extends Specification {
    @Unroll
    def "Can ignore test if AssumptionViolationException is thrown"() {
        setup: 'check some precondition'
        Assume.assumeFalse( i == 2 )
        expect:
        i > 0
        where:
        i << [ 1, 2, 3 ]
    }
}

With the first proposed solution, it would look like this:

import spock.lang.Specification
import spock.lang.Unroll

class AssumptionViolatedTest extends Specification {
    @Unroll
    def "Can ignore test if precondition is not satisfied"() {
        setup: 'check some precondition'
        assumeThat( i == 2 )

        expect:
        i > 0
        where:
        i << [ 1, 2, 3 ]
    }
}

With the second proposal:

import spock.lang.Specification
import spock.lang.Unroll

class AssumptionViolatedTest extends Specification {
    @Unroll
    def "Can ignore test if precondition is not satisfied"() {
        precondition: 'check some precondition'
        i == 2

        expect:
        i > 0
        where:
        i << [ 1, 2, 3 ]
    }
}

Thank you for providing the community with a best-in-class testing framework, and hope that this small addition can help make Spock become even better.

@Vampire
Copy link
Member

Vampire commented May 5, 2020

With #1085 being merged, you can now skip single iterations using @Requires, @IgnoreIf and @PendingFeatureIf, depending on data variables, is that sufficient for you?

@renatoathaydes
Copy link
Contributor Author

From my perspective, the important is actually this:

... specifically, Spock's IRunListener should have a method, void iterationSkipped( IterationInfo iteration ), analogous to the existing void specSkipped( SpecInfo spec ).

The way you suggest to skip iterations seems fine, but extensions need to be able to "detect" when an iteration was skipped... I couldn't find that addition in the changes you've linked, do you know if that has been added already?

@Vampire
Copy link
Member

Vampire commented May 6, 2020

I don't think so, no.
Iterations are actually not skipped, they are aborted with the exception.
I think if you use the exception to abort a non data-driven feature, you will probably have the same problem.
The relevant part from Spock 1.3:

    if (exception instanceof AssumptionViolatedException) {
      // Spock has no concept of "violated assumption", so we don't notify Spock listeners
      // do notify JUnit listeners unless it's a data-driven iteration that's reported as one feature
    } else {
      masterListener.error(error);
    }

and from Spock 2.0

    if (exception instanceof TestAbortedException || exception instanceof TestSkippedException) {
      // Spock has no concept of "aborted tests", so we don't notify Spock listeners
    } else {
      masterListener.error(transformedError);
    }

My PR just changed that you do not need to manually throw that exception, but can use the annotations just like for features or specs.

@renatoathaydes
Copy link
Contributor Author

renatoathaydes commented May 6, 2020

That code snippet you posted above is exactly the reason why spock-reports cannot know when an iteration is skipped. It relies on a JUnit-specific method to do that, currently. But now that it's possible to skip iterations via Spock itself, I would say Spock should notify its listeners when that happens.

In other words, this assertion is no longer correct, right?

// Spock has no concept of "aborted tests"

As it does now when an iteration is "aborted" due to the use of the @Requires, @IgnoreIf and @PendingFeatureIf annotations.

@renatoathaydes
Copy link
Contributor Author

That comment needs to be rephrased:

if (exception instanceof TestAbortedException || exception instanceof TestSkippedException) {
      // Spock has no concept of "aborted tests", so we don't notify Spock listeners
}

It would have to say Spock has no concept of "aborted tests" OR "skipped tests" if you wanted it to match the code... but that would be incorrect, as Spock does have the concept of skipped specifications, and now, you're introducing the concept of "skipped feature iteration"... so I really think that IRunListener needs to have a new method, iterationSkipped, because of the new feature to skip per iteration.

@mcichonqa
Copy link

Hello, renatoathaydes and Vampire, I read your two proposed solution and I have two questions.

It is possible to create static method like TestContext.IgnoreTest("Ignored message") which invoke void iterationSkipped? If you implement one of 2 ideas ignoring tests will only be possible from the level of the test itself, but what about some helper classes, where we can check conditions configuration etc for tests? Maybe I miss understanding but I think this ignore feature for iteration should be exists for every place in the code.

The second question is whether it would be possible to check several conditions before ignoring the test e.g.
assumeThat (i == 2)
assumeThat (i == 10)

Using annotations to ignore tests is ok, but it seems to me that being able to ignore the test elsewhere than in the test gives you more flexibility.
For example:

    def setup(){
    ConfigurationHelper.isShouldBeRun(new RestRequest())
}

    @Unroll
    def Test1() {
        given:
        println(i)

        where:
        i << [ 1, 2, 3 ]
    }
}

class ConfigurationHelper{
      static def isShouldBeRun(RestRequest request){
         def response = request.sendRequest(request)
         if(response.Status != 200)
              TestContext.Ignore("Ignore message")
}
    }

@Vampire
Copy link
Member

Vampire commented May 25, 2020

The annotation and also the assume... methods are only syntactic sugar for throwing a TestAbortedException actually.
You can use assume... methods or throw TestAbortedException wherever you want and as often as you like and the test will be aborted if one of the conditions hold / exception is thrown.
This issues is more about the Spock-own listeners not being called in such events.

@mcichonqa
Copy link

Ok, Assume works perfectly, rtretyak has good solution for that with interceptor to catch this exception. Interceptors are awesome.
Sorry for offtop, but I cant find any information about test status from spock tests and I set it manualy...
It is possible to get test status as variable after execution?

@shenhong29
Copy link

I like to ask two questions about this issue:

  1. Is Spock going to fix the issue any time soon?
  2. Is there a recommended work around for this issue?
    Thank you very much for your help in advance.

kriegaex referenced this issue in kriegaex/spock Mar 11, 2022
'@stepwise' can now be applied to feature methods. The effects are:
  1. the feature be switched to ExecutionMode.SAME_THREAD, similarly to
     how whole specs are switched to the same execution mode when using
     the annotation on class level.
  2. After the first error or failure occurs in any iteration, all
     subsequent iterations are going to be skipped.

Fixes #1008.
@kriegaex
Copy link
Contributor

PR #1442 uses another approach: Simply permit @Stepwise for methods, add an error listener which skips remaining iterations, if it registers any failure.

kriegaex referenced this issue in kriegaex/spock Mar 11, 2022
'@stepwise' can now be applied to feature methods. The effects are:
  1. the feature be switched to ExecutionMode.SAME_THREAD, similarly to
     how whole specs are switched to the same execution mode when using
     the annotation on class level.
  2. After the first error or failure occurs in any iteration, all
     subsequent iterations are going to be skipped.

Fixes #1008.
leonard84 pushed a commit that referenced this issue Mar 21, 2022
`@Stepwise` can now be applied to feature methods. The effects are:
  1. the feature be switched to `ExecutionMode.SAME_THREAD`, similarly to
     how whole specs are switched to the same execution mode when using
     the annotation on class level.
  2. After the first error or failure occurs in any iteration, all
     subsequent iterations are going to be skipped.

Fixes #1008
@kriegaex
Copy link
Contributor

kriegaex commented Apr 6, 2022

@renatoathaydes, @leonard84: I just stumbled upon this issue again and not am not so sure anymore if my change actually solves this issue. There was no feedback by Renato, so maybe I did not think about this enough when writing in the commit message that this issue is closed by my PR.

@Stepwise for data-driven features actually solves the case that the user wants all remaining iterations to be skipped after the first failure, because it does not make sense to try another iteration, maybe because a shared resource is unavailable.

This issue, reading it again, seems to be a slightly different use case, namely the ability to skip any iteration based on a condition. For example, of 5 iterations the condition could skip the 2nd and 4th for some reason, but execute all others. This requirement is not solved by @Stepwise, because in that case iterations 3 through 5 would be skipped after a failure in 2.

Should the issue be re-opened?

@kriegaex
Copy link
Contributor

kriegaex commented Apr 6, 2022

Answering my own question:

Should the issue be re-opened?

No, because this was implemented in another way in #1360, see also #1454 (comment).

R.I.P., issue 1008. 😉

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 a pull request may close this issue.

5 participants