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

Spr-17211 - async tests refactoring #1941

Closed
wants to merge 8 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@antkorwin
Contributor

antkorwin commented Aug 24, 2018

I fixed the problem with a fragile tests for asynchronous events.

Also I add the AsyncAssert utility for write an asynchronous test with a better performance than test which use a Thread.sleep(1000)

I've a next result for pass of changed tests on my computer:
before ~30 sec.
after ~12 sec.

@pivotal-issuemaster

This comment has been minimized.

pivotal-issuemaster commented Aug 24, 2018

@antkorwin Please sign the Contributor License Agreement!

Click here to manually synchronize the status of this Pull Request.

See the FAQ for frequently asked questions.

@pivotal-issuemaster

This comment has been minimized.

pivotal-issuemaster commented Aug 24, 2018

@antkorwin Thank you for signing the Contributor License Agreement!

@sbrannen

Thanks for the PR.

I've requested a few changes to reduce the noise in this PR.

So let's see how it looks after you've made those changes.

import org.springframework.context.ApplicationContext;
import org.springframework.tests.sample.beans.TestBean;
import org.springframework.util.asyncassert.AsyncAssert;
import java.util.concurrent.TimeUnit;

This comment has been minimized.

@sbrannen

sbrannen Aug 24, 2018

Member

Please undo all changes to import ordering and use of import wildcards in this PR.

See the following for details: https://github.com/spring-projects/spring-framework/wiki/Code-Style#import-statements

@@ -78,40 +79,35 @@ public void asyncMethods() throws Exception {
try {
asyncTest.returnSomething(0).get();
fail("Should have thrown ExecutionException");
}
catch (ExecutionException ex) {
} catch (ExecutionException ex) {

This comment has been minimized.

@sbrannen

sbrannen Aug 24, 2018

Member

Please undo all removal of newlines before catch statements.

See the following for details: https://github.com/spring-projects/spring-framework/wiki/Code-Style#braces

* @param unit unit of time
* @return AsyncAssert with new settings of polling.
*/
public AsyncAssert polling(long amount, TimeUnit unit) {

This comment has been minimized.

@sbrannen

sbrannen Aug 24, 2018

Member

Please use java.time.Duration instead of implementing a custom type.

* @param unit unit of time
* @return AsyncAssert with new settings of timeout
*/
public AsyncAssert timeout(long amount, TimeUnit unit) {

This comment has been minimized.

@sbrannen

sbrannen Aug 24, 2018

Member

Please use java.time.Duration instead of implementing a custom type.

import static org.junit.Assert.assertEquals;
/**
* Created on 23.08.2018.

This comment has been minimized.

@sbrannen

sbrannen Aug 24, 2018

Member

Please remove all such "Created on" JavaDoc entries in this PR.

@antkorwin

This comment has been minimized.

Contributor

antkorwin commented Aug 27, 2018

@sbrannen , I corrected all issues in PR, also I imported the Awaitility in dependencies and use it (with the permission of @jhoeller ).

@sbrannen sbrannen self-assigned this Aug 27, 2018

@sbrannen

This comment has been minimized.

Member

sbrannen commented Aug 27, 2018

@sbrannen , I corrected all issues in PR, also I imported the Awaitility in dependencies and use it (with the permission of @jhoeller ).

Sounds good.

I'll try to get this merged into master soon.

@sbrannen

This comment has been minimized.

Member

sbrannen commented Sep 4, 2018

This has been merged into master in ab086f4.

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment