-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Introduce first-class support for scenario tests #48
Comments
@sbrannen: do you prefer referencing the next step by name? i assume these references would be refactoring-safe, so no real problem here. but what about typos? (i guess just using numbered steps will be awkward if you introduce steps in the middle...) |
Is it in scope for Alpha1 (end of next week)? |
Thanks for writing down the concept and providing a detailed example. I think it is great and I also like the next attribute pointing to the methodName. Two reasons:
I still wonder if we want to point to the next or rather to the previous step? From a reader's perspective, I would like to have an idea what are the preconditions for this step? Especially as the annotation stands above the method declaration, I would suggest using a "previous" or "ancester" attribute. But it's just a feeling. Concerning the Alpha-M1: I don't think it will be part of it. We should probably move it to a different milestone. What do you think, @sbrannen ? |
Why not use preconditions as the concept to determine the order? A step can 2015-12-04 11:24 GMT+01:00 Stefan Bechtold [email protected]:
|
Yes, this will be a consequence of turning from "next" to "ancestor". Preconditions are great, but we need to be aware of the added complexity. Maybe we discuss this topic in person? Anyways, I like this feature. 👍 |
Referring a method by string sounds like way to do more mistakes and as @bechte commented it will not give IDE support when coding. I would suggest something like following final String LOGIN_ID = "login_id"; @step(id = LOGOUT_ID, waitFor = VISIT_PAGE) so the waitFor will wait till VISIT_PAGE is completed. That way executor should be able to plan concurrent executions as well. But again I really see the benefit of the original proposal |
Utilizing a simple state machine (using enums instead of strings) is an overkill, right? |
I was trying to say that I like the method name references. I would like to avoid introducing another ID for the tests. I think IDE vendors are great for picking up method names (outline view, etc.) and they will easily support refactorings and support test writers with auto-completion. This can all happen on static code analysis and, therefore, is something they can support right-away. At least as a test writer, I dont want to maintain the list of IDs. It's boilerplate anyways. We might think about support the test name:
and allow referencing the name as well. But for a first alpha, I would stick with the easy method name approach provided by @sbrannen . |
@jlink and @bechte, scheduling this issue for Alpha M1 was intentionally optimistic. Whether or not it's possible for Alpha M1 really depends on how far we get with the rewrite of the JUnit 5 engine -- for example, whether we have extension points for test instance lifecycle and ordering of tests. If we don't get far enough in Alpha M1, we will naturally push the implementation of this issue to a subsequent milestone. However, we need to make sure that everyone has this on his radar; otherwise, it will likely be more challenging to integrate this feature as an afterthought. |
If this wasn't first-class supported, how hard would it be to add via a David Saff |
@mmerdes, @bechte, @jlink, @dinesh707, Thanks for the feedback on ordering and dependencies! The reason I have thus far only proposed an To address the issues you have raised... Depending on how we decide to support ordering in general (i.e., for standard The difference is that an annotation like As for whether a step declares its dependency by referencing the previous step vs. the next step, I think it will become more clear what the better solution is once we begin to experiment more with this feature. With regard to supporting a precondition mechanism, that would certainly increase the flexibility of step execution within a scenario test, but I think we should first focus on a simpler approach before adding the level of complexity that would be required to support that correctly. Along the same line of thinking, for the initial draft of this feature I would recommend that we not concern ourselves with parallel execution of steps within a scenario test. |
With the current set of extension points, it would be impossible to implement a feature like this as an extension. However, that may soon change, depending on the outcome of the current effort to redesign the test discovery and test execution models. In general, our goal is to introduce as many extension points that we deem feasible. Thus, if we create the necessary extension points in the engine to make it possible to implement scenario tests as an extension, we would naturally opt to implement it as an extension even if the feature is available out-of-the-box. That's analogous to how we implemented |
Yes, I think that using or implementing a true state machine would be overkill for this feature. |
I think you meant: @Name("Step1")
@Step instead of: @Test(name = "Step1") Right? |
@sbrannen True 👍 - Thanks ;-) |
Here is an idea for an alternative way of defining test order, using the normal rules of code execution order: http://specsy.org/documentation.html#non-isolated-execution-model That way requires that tests can be nested and that tests are declared as method calls which take a lambda as parameter, so it can be hard to fit it together with the JUnit style of declaring tests as method declarations. |
@orfjackal, thanks for sharing the link to Specsy's Non-Isolated Execution Model.
Since the JDK does not guarantee that methods are returned in the order in which they are declared when using reflection, we have to introduce an explicit mechanism to control the order. So unless we introduce an alternative programming model based solely on lambdas (which has in fact been discussed), we unfortunately won't be able to support a programming model as simple as Specsy in this regard. |
Interesting discussion - I just wanted to throw in two alternative ideas that came to my mind: One alternative for referring to test methods via Strings or explicit lambdas could be method-references. Another idea is to create a proxied mock object of the current test class that would allow you to specify the @ScenarioSetup
public void simpleScenario(WebSecurityScenarioTest mock){
mock.visitPageRequiringAuthorizationWhileNotLoggedIn();
mock.login();
mock.visitSecondPageRequiringAuthorizationWhileLoggedIn();
mock.logout();
} |
Why not rename I would also change the Any test methods with Also having chained test methods finally gives us something like |
Sounsd like a good use case for a test engine of your own? I wouldn't like to overload the meaning of @test within the JUnit 5 engine.
|
In my view it wouldn't overload The second part of what I propose is a somewhat less verbose alternative to what @sbrannen proposed and would simply allow you to chain together test methods using |
Three things I would like to add to the discussion:
I really think we should take this issue and have a group discussion within the dev team in January. |
Hi, I'm encountering an error which directly references this issue by link. Gradle says:
I'm using Spring Cloud Contract (SCC), which places all the individual contract tests into a single class, relying on an inherited test base class for setup. I have no annotations on the test methods (SCC marks the methods with I'm surprised to see this junit failure message. If this is the wrong place to ask about this, I'm happy to ping Spring Contract. Since the error message references here, I thought I'd ask here first. |
Some more digging. This is definitely SCC, not JUnit, complaining: SCC believes this I'll talk to them. |
This issue looks almost completed: The only additional thing which is missing IMHO: skip the remaining tests after a failure. |
@panchenko I think you are right. Here is my first attempt I have been trying out (haven't tried with nested tests or anything complex): import org.junit.jupiter.api.MethodOrderer
import org.junit.jupiter.api.TestInstance
import org.junit.jupiter.api.TestMethodOrder
import org.junit.jupiter.api.extension.ConditionEvaluationResult
import org.junit.jupiter.api.extension.ExecutionCondition
import org.junit.jupiter.api.extension.ExtendWith
import org.junit.jupiter.api.extension.ExtensionContext
import org.junit.jupiter.api.extension.TestExecutionExceptionHandler
// https://github.com/junit-team/junit5/issues/48 - Introduce first-class support for scenario tests
// https://github.com/junit-team/junit5/issues/431 - Introduce mechanism for terminating Dynamic Tests early
private class StepwiseExtension : ExecutionCondition, TestExecutionExceptionHandler {
override fun handleTestExecutionException(context: ExtensionContext, throwable: Throwable) {
val namespace = namespaceFor(context)
val store = storeFor(context, namespace)
store.put(StepwiseExtension::class, context.displayName)
throw throwable
}
override fun evaluateExecutionCondition(context: ExtensionContext): ConditionEvaluationResult {
val namespace = namespaceFor(context)
val store = storeFor(context, namespace)
val value: String? = store.get(StepwiseExtension::class, String::class.java)
return if (value == null) {
ConditionEvaluationResult.enabled("No test failures in stepwise tests")
} else {
ConditionEvaluationResult.disabled("Stepwise test disabled due to previous failure in '$value'")
}
}
private fun namespaceFor(context: ExtensionContext): ExtensionContext.Namespace =
ExtensionContext.Namespace.create(StepwiseExtension::class, context.parent)
private fun storeFor(context: ExtensionContext, namespace: ExtensionContext.Namespace): ExtensionContext.Store =
context.parent.get().getStore(namespace)
}
@TestInstance(TestInstance.Lifecycle.PER_CLASS)
@TestMethodOrder(MethodOrderer.OrderAnnotation::class)
@ExtendWith(StepwiseExtension::class)
@Target(AnnotationTarget.CLASS)
@Retention(AnnotationRetention.RUNTIME)
annotation class Stepwise import org.junit.jupiter.api.Order
import org.junit.jupiter.api.Test
@Stepwise
internal class ScenarioExample {
@Test
@Order(1)
internal fun first() {
println("1")
}
@Test
@Order(6)
internal fun sixth() {
println("6")
throw IllegalStateException("Should be skipped")
}
@Test
@Order(5)
internal fun fifth() {
println("5")
throw IllegalStateException("Should be skipped")
}
@Test
@Order(3)
internal fun third() {
println("3")
}
@Test
@Order(2)
internal fun second() {
println("2")
}
@Test
@Order(4)
internal fun fourth() {
println("4")
throw IllegalArgumentException("FAILURE")
}
} Which fails on the |
@mkobit Thanks for sharing, that looks promising! It should use |
My two cents, since I just implemented a feature for scenario-oriented tests in my own testing tool (which I currently use with JUnit 4, but should migrate to JUnit 5 eventually). As a user, I would prefer to simply put Test (step) ordering should respect the textual order I write the tests in the test class. This can be implemented without much difficulty using ASM. (I just did it, hacking JUnit 4's |
That's related to the discussion here: #1919 (comment) |
Is it a goal of this feature to support mapping from a Gherkin feature file to the relevant scenario classes? |
No, this is about providing sth. similar to Spock's
AFAIK Cucumber already provides a custom TestEngine so integration should no longer be an issue or am I missing sth.? |
Extension provided by @mkobit works fine for me. Here is Java version: package pl.marcinchwedczuk.javafx.validation.demo.utils;
import org.junit.jupiter.api.extension.ConditionEvaluationResult;
import org.junit.jupiter.api.extension.ExecutionCondition;
import org.junit.jupiter.api.extension.ExtensionContext;
import org.junit.jupiter.api.extension.TestExecutionExceptionHandler;
// https://github.com/junit-team/junit5/issues/48 - Introduce first-class support for scenario tests
class StopOnFirstFailureExtension implements ExecutionCondition, TestExecutionExceptionHandler {
@Override
public void handleTestExecutionException(ExtensionContext context, Throwable throwable) throws Throwable {
try {
ExtensionContext.Namespace namespace = namespaceFor(context);
ExtensionContext.Store store = storeFor(context, namespace);
store.put(StopOnFirstFailureExtension.class, context.getDisplayName());
} catch (Exception e) {
e.printStackTrace();
}
throw throwable;
}
@Override
public ConditionEvaluationResult evaluateExecutionCondition(ExtensionContext context) {
var namespace = namespaceFor(context);
var store = storeFor(context, namespace);
var value = store.get(StopOnFirstFailureExtension.class, String.class);
if (value == null) {
return ConditionEvaluationResult.enabled("No test failures in stepwise tests");
} else {
return ConditionEvaluationResult.disabled(
"Stepwise test disabled due to previous failure in '" + value + "'");
}
}
private ExtensionContext.Namespace namespaceFor(ExtensionContext context) {
return ExtensionContext.Namespace.create(StopOnFirstFailureExtension.class, context.getParent());
}
private ExtensionContext.Store storeFor(ExtensionContext context, ExtensionContext.Namespace namespace) {
return context.getParent().get().getStore(namespace);
}
}
|
@nipafx Could this be a feature for JUnit Pioneer? |
For anyone stumbling upon this, I've found the Test Robot Pattern a great way of expressing "steps" or "scenarios" inside a test in a readable manner. My current team uses it in an Android project to great effect, and the pattern also applies to things like Selenium tests. |
It seems there's a relatively simple way to achieve this using already existing extensible mechanisms so I would tend not to. 🙂 |
I do not understand why don't you want to add it directly to JUnit? Or do you want to test it in JUnit Pioneer and then move it to JUnit? Or are there another reasons? Because I think this is one of the most wanted feature. |
This issue has been automatically marked as stale because it has not had recent activity. Given the limited bandwidth of the team, it will be automatically closed if no further activity occurs. Thank you for your contribution. |
Note that the workaround offered by StopOnFirstFailureExtension is quite different with inter-test dependency. The latter should work like this: when running a step3 test, its dependence tests would run as well. |
For anyone landing on this page, looking for a complete example of a |
Proposal
The current proposal is to introduce the following:
@ScenarioTest
: class-level annotation used to denote that a test class contains steps that make up a single scenario test.@Step
: method-level annotation used to denote that a test method is a single step within the scenario test.The
@Step
annotation will need to provide an attribute that can be used to declare the next step within the scenario test. Steps will then be ordered according to the resulting dependency graph and executed in exactly that order.For example, a scenario test could look similar to the following:
Related Issues
The text was updated successfully, but these errors were encountered: