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

Fix fragile tests for asynchronous events [SPR-17211] #21744

Closed
spring-projects-issues opened this issue Aug 24, 2018 · 5 comments
Closed

Fix fragile tests for asynchronous events [SPR-17211] #21744

spring-projects-issues opened this issue Aug 24, 2018 · 5 comments
Assignees
Labels
in: test type: task
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

@spring-projects-issues spring-projects-issues commented Aug 24, 2018

Korovin Anatoliy opened SPR-17211 and commented

I tried to run all tests several times (in the spring-context module), and sometimes I got an error, sometimes not.

Some of these errors:

  • org.springframework.scheduling.concurrent.AbstractSchedulingTaskExecutorTests#submitCallableWithGetAfterShutdown
  • org.springframework.scheduling.concurrent.AbstractSchedulingTaskExecutorTests#submitFailingListenableCallable

I fixed it and also made a refactoring some of async tests..

I will soon create a PR.


Affects: 5.1 RC2

Attachments:

Referenced from: pull request #1941, and commits ab086f4

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 24, 2018

Sam Brannen commented

Just an idea...

Instead of reinventing the wheel here with the proposed AsyncAssert class, why not used a dedicated asynchronous assertion library such as Awaitility?

Juergen Hoeller, what do you think about introducing a test dependency on Awaitility for this purpose?

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 24, 2018

Korovin Anatoliy commented

I'm absolutely agree, this is the first thing I tried to do, but I was very surprised that the code was not working with it.

In the test AbstractSchedulingTaskExecutorTests sometimes there are fails when I run all child tests.

I plan to reproduce this problem on a simple example and commit a bug-report to the Awaitility.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 26, 2018

Juergen Hoeller commented

Indeed, it would be nice to reuse Awaitility there... We might also temporarily disable those tests for the time being if this still takes a while.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Aug 26, 2018

Korovin Anatoliy commented

it is good, I can make it in this PR, today..

I found a problem when using the awaitility, this is not a bug in the utility, as I understood the problem is that the awaitility catches all uncaught throwables in all threads and propagates them to the awaiting thread.

In my opinion this is not an expected behavior, because when you check a test without the awaitility, for example by the Thread.sleep(1000) you dont get any exceptions and successfuly pass the test. And when you use the awaitility you can get a failed test because another thread throws an exception.

But the awaitility provide API to disabled this behavior, so I can solve this problem.

@spring-projects-issues
Copy link
Collaborator Author

@spring-projects-issues spring-projects-issues commented Sep 4, 2018

Sam Brannen commented

This has been resolved in master. See related commits in the sidebar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test type: task
Projects
None yet
Development

No branches or pull requests

2 participants