Skip to content

Add Support for Cucumber using @split[feature:treatment] tags #247

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

Merged
merged 4 commits into from
Jul 26, 2021

Conversation

aslakhellesoy
Copy link

@aslakhellesoy aslakhellesoy commented Jul 13, 2021

Hi Splitsters,

This PR adds a small utility that would make it easier to use Split from Cucumber Scenarios. Here is a short example:

Feature: Make Coffee

  @split[cappuccino:on]
  Scenario: verify functionality with cappuccino feature
    Given ...
    When ...
    Then ...

  @split[cappuccino:off]
  Scenario: verify functionality without cappuccino feature
    Given ...
    When ...
    Then ...

In order to enable the integration with Split, a Before Hook must be defined:

import io.cucumber.java.Before;
import io.split.client.testing.SplitClientForTest;
import io.split.client.testing.cucumber.CucumberSplit;

public class StepDefinitions {
    private final SplitClientForTest splitClient = new SplitClientForTest();

    @Before
    public void configureSplit(Scenario scenario) {
        CucumberSplit.configureSplit(splitClient, scenario);
    }
}

I hope you'll consider adding this to the io.split.client:java-client-testing library!

Many thanks,

Aslak Hellesøy
Creator and lead developer of Cucumber Open

<dependency>
<groupId>io.cucumber</groupId>
<artifactId>cucumber-java</artifactId>
<version>6.10.4</version>
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should probably be test scope, no?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably

Copy link
Author

@aslakhellesoy aslakhellesoy Jul 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've thought a bit more about this. I think there are two options here - compile scope or provided scope. (See maven docs for details).

The junit dependency is already compile scope, so using the same scope for cucumber-java and cucumber-junit would be consistent with that.

Alternatively, making it provided scope would indicate that dependent modules would have to explicitly add the dependencies themselves. I think that's ok too, if you don't want consumers of the library transitively download cucumber unless they opt into it with an explicit dependency in their own pom/gradle file.

I'll defer to you to decide what's most appropriate.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If someone is already using this framework (our Java-testing) without cucumber and I merge this PR. Regardless of the provided or not, I believe the build will fail for not finding the dependency in the classpath, right?

I need time to sync internally but if that's the case we may need to split this into a separate submodule.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the build will fail for not finding the dependency in the classpath, right?

No it won't.

In Java, classes are only loaded when they are referenced by another class. Classes are not loaded by mere presence in a jar file.

No other classes in this library reference io.split.client.testing.cucumber.CucumberSplit, so it won't be loaded.

@@ -0,0 +1,19 @@
@split[cappuccino:off]
@split[dollars:on]
Feature: Split
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind educating me how the unit test binds to the split listed in this file?

also, is the idea to have a single descritptor file or one per test class or test case?

Copy link
Author

@aslakhellesoy aslakhellesoy Jul 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what you mean by unit test or descriptor file. I'll clarify some of the terminology used by Cucumber (which is closer to an acceptance testing tool than a unit testing tool).

A .feature file (sometimes called gherkin document) contains multiple scenarios (sometimes called examples).
Each Scenario is an acceptance test and consists of the following parts:

  • One or more Steps (Given, When, Then).
  • Step Definitions, which are Java methods annotated with @Given, When or @Then.

Cucumber creates a new instance of every class with step definitions before the scenario runs. This is similar to how JUnit creates a new instance of a test class for every @Test method it executes. However, the step definition is not a test - think of it as a method that is "invoked" by one of the scenario's steps.

For each step in the scenario, Cucumber finds a matching step definition (based on the expression on the annotation) and invokes the method.

Step definition classes aren't scoped to a particular .feature file. The steps in a .feature file can invoke step definitions from any step definition class.

Scenarios can have tags. The code in this library looks for tags of the format @split[feature:treatment] and uses that to register the appropriate treatments. This is done on a new instance of SplitClientForTest for every scenario.

public static void configureSplit(SplitClientForTest splitClient, Scenario scenario) {
Collection<String> tags = scenario.getSourceTagNames();
for (String tag : tags) {
Matcher matcher = SPLIT_TAG_PATTERN.matcher(tag);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question here:

  • if more than 1 tag matches this, do you end up overwriting the registered (feature, treatment) pair?

Copy link
Author

@aslakhellesoy aslakhellesoy Jul 17, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This is intentional.

Tags that are placed at the top level Feature level are inherited by all the Scenarios. The getSourceTagNames() method returns a Collection of the tags from both the Feature and the current Scenario. The inherited Feature tags are always before the Scenario tags in the collection (it would have been more appropriate to declare the return type as List rather than Collection since it's ordered).

We're taking advantage of this here to allow users to define "default" treatments at the feature level, and (optionally) override them at the scenario level. I have illustrated this in the last scenario where the default dollars=off treatment from the feature level is overridden to dollars=on for that particular scenario.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've pushed some more changes to (hopefully) illustrate better how this works.

@patricioe
Copy link
Contributor

@aslakhellesoy The only dependency with Split testing module is on the SplitClientForTest correct? just triple checking.

@aslakhellesoy
Copy link
Author

@aslakhellesoy The only dependency with Split testing module is on the SplitClientForTest correct? just triple checking.

That is correct.

The only class I have added to the module is io.split.client.testing.cucumber.CucumberSplit. As you can see from its import statements, its dependencies are:

import io.cucumber.java.Scenario;
import io.split.client.testing.SplitClientForTest;

import java.util.Collection;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

@patricioe
Copy link
Contributor

hey @aslakhellesoy are you able to allow me to edit your fork pull request? I'd like to create a new module so you have complete isolation for iterations and so I can run a git checkout on this branch to do a release.

image

@patricioe patricioe changed the base branch from development to cucumber July 26, 2021 20:55
@patricioe patricioe merged commit b83e182 into splitio:cucumber Jul 26, 2021
@patricioe patricioe mentioned this pull request Jul 26, 2021
@patricioe
Copy link
Contributor

Let's continue in #248

@aslakhellesoy
Copy link
Author

I've now moved this code to https://github.com/cucumber/split-java/ and added some documentation. Please remove the code I have contributed now that Cucumber-Split has a new home.

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 this pull request may close these issues.

2 participants