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

Test-cases using serenity.batch commands are not being split up #2199

Closed
timothyjmtan opened this issue Aug 11, 2020 · 16 comments
Closed

Test-cases using serenity.batch commands are not being split up #2199

timothyjmtan opened this issue Aug 11, 2020 · 16 comments
Labels

Comments

@timothyjmtan
Copy link
Contributor

timothyjmtan commented Aug 11, 2020

Hello,

I am having some difficulties in running the JUnit test-cases in batches via the serenity.batch.count commands documented here:

Serenity Parallel Batches

My testing stack is as follows:
Jenkins -> Openshift pod (slave) -> AWS browser stack

Upon Jenkins job trigger, an openshift pod will spin up dynamically, and coordinate with the AWS stack to run the tests.

Expectation
By using serenity.batch.count=3, there should be 3 openshift pods spun up, and each will handle a test-suite corresponding to its batch number.

Observation
3 openshift pods spin up, and from console logs it is running the correct mvn command with the correct batch numbers:

Pod #1 - mvn clean verify -Dserenity.batch.number=1 -Dserenity.batch.count=3 --> runs 0 test-cases
Pod #2 - mvn clean verify -Dserenity.batch.number=2 -Dserenity.batch.count=3 --> runs 0 test-cases
Pod #3 - mvn clean verify -Dserenity.batch.number=3 -Dserenity.batch.count=3 --> runs 200 test-cases

However, 2 pods will execute 0 test-cases, and 1 pod will run the entire test-suite. This suggests that the splitting is not being handled properly somewhere.

No difference with the inclusion of the serenity.batch.strategy parameter.

It has the same behaviour when executing on plain windows machines as well.

I don't reckon I need to set the following in maven-failsafe-plugin in pom.xml, as its for subdivisions within a test machine.

        <parallel>classes</parallel>
        <threadCount>${parallel.tests}</threadCount>
        <forkCount>${parallel.tests}</forkCount>

Is there anywhere that I can take a look at to troubleshoot?

@timothyjmtan timothyjmtan changed the title Test-cases using serenity.batch commands are not being split up (openshift environment) Test-cases using serenity.batch commands are not being split up Aug 11, 2020
@wakaleo
Copy link
Member

wakaleo commented Aug 11, 2020

This would be a question for @nbarrett.

@nbarrett
Copy link
Contributor

nbarrett commented Aug 11, 2020

Okay @timothyjmtan, here are some thoughts...

  • When it comes to diagnosing a problem, my strong recommendation would be to keep the complexity to a minimum, so I would suggest removing the Openshift pod (slave) and AWS browser stack from the equation, until you understand and get the batching working properly. In order to achieve this, my suggestion would be to try and run these batches locally on your dev machine first to work out what's going on. For instance, I was a bit concerned about the very "round" number of 200 test-cases that were run in one of your batches. Real-life is often more random than that 😆
  • Fortunately, Serenity has the most excellent "dry run" mode which can be activated using -Dserenity.dry.run=true. This very quickly runs all tests but bypasses WebDriver navigation, so your suite can be run in just a few seconds.
  • I'm assuming that you have used the Screenplay pattern, but in dry run mode, only the "outer" Performables will be executed. This is also a very quick way of working out whether your entire test suite can even be run properly. I'm not sure whether you've had full, clean runs from your suite yet, so thought I'd mention this. Dry run can highlight some design problems in your test setup or Performables that will prompt some refactoring or redesign (e.g. class initialisation problems or a host of other things that need to be fixed during the suite refinement stage).
  • Moving onto the maven configuration, I would recommend that you have something along these lines to ensure that tests are ordered correctly. I've helped other people in the past with batching problems because they didn't have runOrder configured properly (please excuse my out of date versions, I've not used maven for about 4 years):
    <build>
        <plugins>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-surefire-plugin</artifactId>
                <version>2.19.1</version>
                <configuration>
                    <runOrder>alphabetical</runOrder>
                    <systemPropertyVariables>
                    </systemPropertyVariables>
                    <skipTests>${skip.surefire.tests}</skipTests>
                </configuration>
            </plugin>
            <plugin>
                <artifactId>maven-failsafe-plugin</artifactId>
                <version>2.19.1</version>
                <configuration>
                    <runOrder>alphabetical</runOrder>
                    <includes>
                        <include>com.blah.blah/**.java</include>
                    </includes>
                </configuration>
                <executions>
                    <execution>
                        <goals>
                            <goal>integration-test</goal>
                            <goal>verify</goal>
                        </goals>
                    </execution>
                </executions>
            </plugin>
            <plugin>
                <groupId>net.serenity-bdd.maven.plugins</groupId>
                <artifactId>serenity-maven-plugin</artifactId>
                <version>${serenity.maven.plugin.version}</version>
                <dependencies>
                    ...
                </dependencies>
            </plugin>
        </plugins>
    </build>
  • Another thing that I've found is useful with Junit test suites that are batched is to tag the tests so that you can see in the html report how the tests are balanced across the slices. Here's something you could add to your Test runner(s) to help provide insight:
@After
    public void tearDown() {
        tagTestWithSlice();
    }

    public static void tagTestWithSlice() {
        EnvironmentVariables environmentVariables = Injectors.getInjector().getInstance(EnvironmentVariables.class);
        TestTag testTag = TestTag.TestTagBuilder.withName(String.format("Slice %02d", Integer.valueOf(environmentVariables.getProperty(SERENITY_BATCH_NUMBER, "1")))).andType("Test Slice");
        StepEventBus.getEventBus().addTagsToCurrentTest(newArrayList(testTag));
    }

Hope this helps!

@timothyjmtan
Copy link
Contributor Author

timothyjmtan commented Aug 12, 2020

Hello @nbarrett, thanks for the in-depth suggestions!

Just some updates:

  • I have significantly reduced the overall complexity by running the command mvn clean verify -Dserenity.batch.count=4 -Dserenity.batch.number=1 on my local PC. My test.folder is pointing to a package containing only 4 tests (written in Screenplay pattern), so the expectation here is that it should only attempt to execute 1 test-case.

  • I have introduced -Dserenity.dry.run=true, and have checked that all 4 test-cases are running w/o issues using a vanilla mvn clean verify.

  • With the only change being the inclusion of the two additional parameters mvn clean verify -Dserenity.batch.count=4 -Dserenity.batch.number=1, I am getting 0 tests being executed.

  • No difference between -Dserenity.batch.count=4 and -Dserenity.batch.size=4. I read the source code and saw that the former is actually deprecated.

  • I have also tried incrementing the -Dserenity.batch.number=X to see if there were any differences. Batch numbers 1-3 yielded 0 tests, but serenity.batch.number=4 executed all 4 test-cases.

  • Have incrementally introduced the suggestions in the pom.xml configuration, but I do not see any difference in behaviour as described above.

  • I have also tried including the method described in the @After annotation. All 4 test-cases are showing tags of Slice 04 (Test Slice).

So it seems to be that there is something iffy with the batch allocation of the tests. Are there are any lower-level logs which I can access to see where the issue may be at? From where I currently stand, I'm not entirely sure if it is a maven or serenity issue...

@nbarrett
Copy link
Contributor

Hi again @timothyjmtan - well it does seem like you are being very methodical in your approach and doing all the right things. I now recall that I did have a very similar problem to you and raised #736 but ended up resolving it by fixing the maven plugin configuration as described in this note. Around this time, I started work on a batch refactoring PR which included:

  • addition of logging in BatchManagerProvider to help people diagnose why tests are being included/excluded
  • updated tests to use more modern Java constructs
  • refactored THUCYDIDES properties to SERENITY equivalents
  • fixed Demo projects and updated dependencies to enable them to be run to demonstrate the batching

However due to resolving my problem, there was no need at the time to make fixes changes to serenity. I think it might be a good idea if I bring this up to date and then submit it, then hopefully the logging will be available in the core product. What does that sound like?

@timothyjmtan
Copy link
Contributor Author

timothyjmtan commented Aug 12, 2020

Hello @nbarrett , yes I suppose that would be extremely useful for anyone who is running into issues using the batching functionality!

Just curious, is it a must to include the -Dparallel="classes" -Dparallel.count=4 -DuseUnlimitedThreads=true parameters for basic batching functionality to work? From my understanding these are just to enable concurrency testing for the nominated tests of a particular batch, so it's not required per se.

@nbarrett
Copy link
Contributor

I've never personally used any of those parameters, it should work with just the two we've already discussed. Okay will try and get that PR submitted later on 👍

@nbarrett
Copy link
Contributor

I've got one last thought.... when you mention the 4 tests in your diagnosis, how are they allocated? For instance are there 4 @Test methods or 4 classes? I would suggest splitting them up into separate classes as I think the batching doesn't go down to the test method level.

@timothyjmtan
Copy link
Contributor Author

Hello @nbarrett , it's 4 classes, each containing 1 @test annotation. As such I believe the batching should work... Hope that clarifies!

@timothyjmtan
Copy link
Contributor Author

timothyjmtan commented Aug 13, 2020

Howdy @nbarrett, after digging through the code, I think I have found the issue.

Based on my understanding of how the batching works within the BatchManager:

  • As each test-case is executed by the framework, it will be registered and the atomic integer testCaseCount will be incremented by 1.
  • After this increment, the method shouldExecuteThisTest() will be executed.
  • Based on the current value of testCaseCount.get(), the method will return a true or false depending on the outcome of the logic check

What I've noticed is that in my code, before any test-case is actually 'executed', the batch manager is parsing through every single test-case, and doing the testCaseCount increments. As such, when the test-case is actually being executed, the testCaseCount is already at the max value, i.e. the total number of test-cases, and as such the logic breaks.

With an example, my code is currently giving the following values:

Number of test-cases: 4
serenity.batch.number=1
serenity.batch.count=4

TestCaseA.java --> expected: testCaseCount=1 actual: testCaseCount=4
TestCaseB.java --> expected: testCaseCount=2 actual: testCaseCount=4
TestCaseC.java --> expected: testCaseCount=3 actual: testCaseCount=4
TestCaseD.java --> expected: testCaseCount=4 actual: testCaseCount=4

As such, batches 1-3 will always return 0 TCs to execute, and serenity.batch.number=4 will execute everything.

It is worth noting that I am actually using an older version of Serenity, i.e. 1.9.36. Comparing it to the latest version in github, I do not see any changes in logic, but perhaps there was one in serenity-core that modified this batching behaviour?

@nbarrett
Copy link
Contributor

nbarrett commented Aug 13, 2020

Hi @timothyjmtan - great diagnostics and very similar to what I did when I was exploring back here in #736. The BatchManager implementations do rely on being executed in the same sequence and in such a way that the atomic integer increments gradually throughout the execution of the suite and is not reset between each test case. For some reason it seems that some people's configuration (yours included), causes these conditions to not occur resulting in incorrect batching behaviour. I did spend some time on the PR last night and cleaned things up quite a bit and added logging, but I didn't make any changes to the batching algorithm itself. My preference would be to implement the same approach for batching as has been done for Cucumber, namely using MultiRunTestStatistics, as this works really well. The challenge I've yet to resolve is that this algorithm needs to build a list of all tests in the suite before any tests are run. I just need to work out how to hook this point in the lifecycle reliably. I'll do some more work on this later on and hopefully get that in the PR as well.

@timothyjmtan
Copy link
Contributor Author

timothyjmtan commented Aug 20, 2020

Hey @nbarrett , just to give another update. We've managed to pinpoint and resolve the issue.

We're using a parent-child pom model in our project, and it turns out that the parent pom's maven-failsafe plugin contained the following settings:

<parallel>classes</parallel>
<threadCountClasses>1</threadCountClasses>
<forkCount>${parallel.count}</forkCount>

Commenting these out will allow the batching to be done correctly.

However this gives us another issue where we are now not able to execute the tests within each batch in parallel (via forking). Is this a functionality that can be taken a look at and fixed?

@wakaleo
Copy link
Member

wakaleo commented Aug 20, 2020

Why not check the code and see what it is doing? If it uses thread-local counters then parallel execution for a given batch will not work.

@dicksonprof
Copy link

@wakaleo Does not seems like forking or parallel threading runs would work.
Is there any way to replication the CucumberSlicerRunner login into the Junit.
Some ideas could be writting a maven-plugin that runs at pre-integration-test to do the splititing and writting to a file.
Maybe using Reflection to scan @test and @RunWith(SerenityRunner), SerenityParameterizedRunner

After that the BatchManager reads this file during integrationTest phases. This will probably be more deterministic but the logic and computation may take some time and i think it should be able to support forking/threading

@nbarrett
Copy link
Contributor

Yes, that kind of thing was what I had in mind. I'll try and finish my PR this weekend.

@thePantz
Copy link
Contributor

thePantz commented Oct 6, 2020

Hello, I've just come across this problem. Has this been solved?

@nbarrett
Copy link
Contributor

nbarrett commented Oct 6, 2020

Sorry @thePantz - I now recall I promised to complete my PR but didn't find time yet. Will try and do soon!

@wakaleo wakaleo closed this as completed Feb 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

5 participants