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 for concurrent test execution in the TestContext framework [SPR-5863] #10532

Closed
spring-issuemaster opened this issue Jun 24, 2009 · 22 comments

Comments

@spring-issuemaster
Copy link
Collaborator

commented Jun 24, 2009

Kristian Rosenvold opened SPR-5863 and commented

Status Quo

Newer versions of JUnit support concurrent test execution; however, the Spring TestContext Framework is not designed for concurrency.

Proposed Solution

The enclosed fixes sharpen focus on concurrency (including making mutable state much more distinct from immutable state), increasing separation between data for each test method run and the class they are being run on.

The patch consists of a failing test (patch 1) and a fix (patch 2), including several new tests. If you choose to apply the patch with the failing test, you must revert this before applying the fix (failing test is contained in fix patch). The included failing test may not produce concurrency issues (fail) in all cases and on all hardware platforms. They have been known to fail consistently on 3 different machines, usually upon first run.

Details of the Patch

The patch contains minor changes to the ContextLoader interface. The most significant changes have been made to the TestContextManager and TestContext classes.

Additionally upon completing the functionality, I had multiple deadlocks in the JVM when running my real test suite. I solved this by using a Java 5 ReentrantReadWriteLock in the RequestAttributes.getSessionMutex() method. It really looks to me like the creation of this mutex should be moved to one of the loader filters, since it's always created as of this patch.

Additionally, the patch contains a MockContextLoader that transfers attributes between threads. I'd really like you guys to check that code out before accepting it; there may be other smarter ways of doing this. It's only a part of the test code-base, but once it's included it sets a standard.

Real-life Tests

The patch has been applied to a local version of Spring 3 that has been running stably with multi-core machines and multi-CPU servers too. We have been running a continuous build using parallel classes, methods, and a combination of both. This is a full-scale build that was adaptable to multi-threaded test execution. The application under test uses lots of web-scopes, etc.

Proposed Documentation

Parallel Test Execution

From version 3.0, Spring supports parallel test execution in the Spring TestContext Framework. Executing builds in parallel with JUnit is only supported in later versions of JUnit, and it is recommended to use at least JUnit 4.6 for this feature. Please also note that there's no guarantee your tests will run properly in parallel; a number of general concurrency issues have to be taken into account when executing tests in parallel. Your runner can usually let you choose between classes, methods, and a combination of both. Classes are usually the easiest to get working; "a combination of both" is the hardest. All three modes are supported.


Affects: 3.0 M3

Attachments:

Issue Links:

  • #15263 Debug logging for DependencyInjectionTestExecutionListener breaks injection for parallel tests ("is duplicated by")
  • #18684 Problems running integration tests in parallel using SpringClassRule and SpringMethodRule ("is duplicated by")
  • #19549 using ParallelComputer to test spring test cases got error ("is duplicated by")
  • #17028 Improve thread safety in Spring JUnit integration ("is duplicated by")
  • #13582 Test execution issue: for "shared" test contexts in multithreading
  • #13499 @DirtiesContext does not destroy all cached singleton beans
  • #15166 Do not serialize ApplicationContext creation in the TestContext framework
  • #12343 Use soft or weak references for context caching in the TestContext framework
  • #12710 Limit size of context cache in the TestContext framework

Referenced from: commits e822e4c, 3dc6f11, fbfad86, 3e96cab, 2699504, a10a8e5, ec7aefa

42 votes, 40 watchers

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 24, 2009

Kristian Rosenvold commented

This is the failing test in isolation, can be applied to trunk of spring m3

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 24, 2009

Kristian Rosenvold commented

The complete patch. Apply this to a CLEAN svn up; do not apply the failing-test patch first. Merged well at rev 1417.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 13, 2009

Kristian Rosenvold commented

I have come to understand that I can split this patch in two; I can separate the concurrency issue into a separate issue, thereby reducing the overall footprint of the patch slightly.

The concurrency issue only arises when you have concurrent construction of session scoped artifacts within the same session. In real life this is probably only an issue for quite few applications, and the chances of this occuring are slim. When running parallel tests, this happens all of the time, but can be solved by introducing a non-blocking session scope from within the MockContextLoader instead.

Is there any chance I can make you reconsider the main concurrency fix for 3.0 if I split the patch it in two ? (I'm probably a few months late with this suggestion ;)
The patch for the main concurrency fixes would only affect spring-test.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 4, 2009

Kristian Rosenvold commented

Enclosed is an updated version of the concurrency patches that apply cleanly on trunk release 2051. Apply patches within the org.springframework.test module, using patch -p1

Due to the mentioned workaround for deadlock issue wrt Session object, I will be making a separate issue for this problem.

This patch now consists of three files with "2051" in the name. The two first are the main fix for concurrency issues. These two first patches do not support the use of "DirtiesContext" in a concurrent environment. DirtiesContext is supported with the third patch, although this third patch is more of an idea and you may consider accepting this too.

The original "failing test" can still be used as evidence.

The patch is being maintained at http://github.com/krosenvold/spring-test

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 2, 2010

Kristian Rosenvold commented

Just for the record, all the relevant bits and pieces necessary to support this patch are now released (aspectj 1.6.6), junit 4.8.1 (works from 4.7 but important concurrency bugs are fixed in 4.8.1). maven surefire2.5 support running with concurrent junit. This patch has also been running stably for quite some time in a number of projects.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 17, 2010

Kristian Rosenvold commented

3428* patch revised for 3.0.3 trunk at r3428

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Dec 13, 2011

Frank Prumbaum commented

So is there any issue why it isn't fixed until 3.2m1?
With 3.1.RC2 I still seem to have concurrency issues and running
the Spring tests in parallel mode seems still to be no option. Is
this correct or is this issues description outdated (since it has not
been revised for quite a long time now)?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 29, 2012

Kristian Rosenvold commented

I have not bothered resubmitting this patch based on trunk since it's been more or less permanently on the "move forward" list in the issue tracker. This is probably because the patch contains an interface change.

I think it should be possible to re-work the patch to not contain any breaking changes, especially if this increases the likelihood of getting the patch accepted. The patch is certainly quite stale as applied on current trunk. Let me know if reworking the patch will increase the probability of this getting applied to >0, which seems to be its current value.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented May 28, 2012

Sam Brannen commented

Hi guys,

Due to the increasing popularity of this issue, I'm assigning the fix version to 3.2 M2, and I'll take a look at in the M2 time frame.

Regards,

Sam

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 7, 2013

Juergen Hoeller commented

Kristian,

We are currently working on some remaining TestContext framework enhancements for 3.2.2, and have been considering this request as well. We do realize the importance of parallel test execution and will definitely address this over the next few months. Note that we got 3.2.2 upcoming in early March as well as 4.0 M1 in late April.

It would be great if you could submit a pull request on GitHub since that's much easier to consume for us these days. We might not be able to accept the full set of changes for 3.2.2 due to backwards compatibility issues but are definitely up for doing a thorough revision for the 4.0 line then - even for 4.0 M1 in April already.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 28, 2013

Ian Brandt commented

The http://github.com/krosenvold/spring-test link above is broken. I'm guessing that https://github.com/krosenvold/org.springframework.test is its replacement? There is no pull request from that repo however.

Would this fix include context caching across parallel tests? If I recall correctly when I last tried to use JUnit 4.8 parallel testing with Spring 3.0.7 integration tests I ran into two issues: deadlocks as mentioned above, and identical context locations not being reused across tests. No inheritance or profiles were being used. The tests were run via the Maven Failsafe plugin with the default fork mode of "once".

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 14, 2016

Alexander Arutuniants commented

Is there any known easy workaround to be able to run parallel test for today?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 22, 2016

Petar Tahchiev commented

41 votes and still no workaround... Sigh...

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 3, 2016

Sam Brannen commented

FYI: initial work on this issue has been pushed to master for inclusion in Spring Framework 5.0.

The following commits aim to address concurrency issues within the TestContextManager and DefaultTestContext.

Note that concurrency issues with regard to the loading and eviction of ApplicationContexts have not yet beed addressed. However, having said that, it may be the case that recent changes to the context caching support in spring-test combined with the aforementioned improvements to TestContextManager and DefaultTestContext may already get you quite far in terms of being able to execute tests in parallel.

So, with that in mind, we would be grateful if people could start trying out parallel test execution in Spring Framework 5.0 snapshots.

Cheers,

Sam

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 3, 2016

Sam Brannen commented

Update

I have copied over Kristian Rosenvold's failing tests from https://github.com/krosenvold/org.springframework.test and applied that to master (i.e., Spring Framework 5.0).

This work can be seen in my personal branch here: https://github.com/sbrannen/spring-framework/commits/SPR-5863-krosenvold-github

His custom ContextCleaner no longer works, so I simply replaced that with calls to ContextCacheTestUtils.resetContextCache(). If I recall correctly, that is all that's required to get the code compiling.

I then replaced:

@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration(
    locations = "model/applicationContext-concurrency-simple.xml",
    loader = MockContextLoader.class)
public class SpringJUnit4ClassRunnerEnvironmentAssumptionsTests { /* ... */ }

... with:

@RunWith(SpringJUnit4ClassRunner.class)
@ContextConfiguration("model/applicationContext-concurrency-simple.xml")
@WebAppConfiguration
public class SpringJUnit4ClassRunnerEnvironmentAssumptionsTests { /* ... */ }

Note that his custom MockContextLoader was replaced by the now built-in support for @WebAppConfiguration.

With those minor changes, all of his concurrency tests now pass on master.

Soooooo, it's looking like we are in much better shape now in terms of parallel test execution! (y)

But the Spring Team would greatly appreciate it if people in the community could report back on their experiences with recent Spring Framework 5.0 snapshots! ;)

Cheers,

Sam

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 3, 2016

Sam Brannen commented

Kristian Rosenvold, since I unfortunately cannot merge your work without it going through the official channels (i.e., Pivotal CLA, etc.)...

Would you mind submitting a pull request on GitHub that only includes the failing tests (similar to what I've done in my SPR\-5863-krosenvold-github branch)?

Thanks,

Sam

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 5, 2016

Sam Brannen commented

Kristian Rosenvold, please disregard my request for you to create a PR. I have come up with an alternative means to complete our test suite coverage of concurrent test execution, and I will push that to master in the near future.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 5, 2016

Sam Brannen commented

For those interested in the details, the following additional refinements have been pushed to master:

Furthermore, 2699504 replaces the need for the tests originally implemented by Kristian Rosenvold.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 5, 2016

Sam Brannen commented

A new "Parallel test execution" section has been added to the "Testing" chapter of the reference manual in commit 3e96cab.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Sep 5, 2016

Sam Brannen commented

Resolving this issue as complete.

For a list of general guidelines and unsupported use cases, please consult the new Parallel test execution section in the Testing chapter of the reference manual.

Please give Spring Framework 5.0 snapshots a try and let us know how parallel test execution works for you.

Thanks!

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 25, 2016

Andreas Höhmann commented

This fix was not merged back to 3.2.17.RELEASE right?

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Oct 25, 2016

Juergen Hoeller commented

This isn't even in a 4.x release... It's 5.0 only, available as of 5.0 M2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.