From 611cf26fbafd6d1cf1c2589e324b6343aca6f1ca Mon Sep 17 00:00:00 2001 From: Artem Zinnatullin Date: Sat, 14 Oct 2017 20:18:16 -0700 Subject: [PATCH 1/4] Fix incorrect test parallelWritesWithoutTransaction. --- .../storio2/sqlite/integration/BaseTest.java | 5 ++ .../integration/NotifyAboutChangesTest.java | 14 +++- .../sqlite/integration/RxQueryTest.java | 25 +++++-- storio-test-common/build.gradle | 1 + .../pushtorefresh/storio2/test/Repeat.java | 13 ++++ .../storio2/test/RepeatRule.java | 36 ++++++++++ .../storio2/test/RepeatRuleTest.java | 71 +++++++++++++++++++ 7 files changed, 157 insertions(+), 8 deletions(-) create mode 100644 storio-test-common/src/main/java/com/pushtorefresh/storio2/test/Repeat.java create mode 100644 storio-test-common/src/main/java/com/pushtorefresh/storio2/test/RepeatRule.java create mode 100644 storio-test-common/src/test/java/com/pushtorefresh/storio2/test/RepeatRuleTest.java diff --git a/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/BaseTest.java b/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/BaseTest.java index e499d7d25..6868952ca 100644 --- a/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/BaseTest.java +++ b/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/BaseTest.java @@ -14,6 +14,8 @@ import com.pushtorefresh.storio2.sqlite.operations.put.PutResults; import org.junit.Before; +import org.junit.Rule; +import org.junit.rules.Timeout; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; import org.robolectric.RuntimeEnvironment; @@ -30,6 +32,9 @@ @Config(constants = BuildConfig.class, sdk = 21) public abstract class BaseTest { + @Rule + public Timeout timeout = Timeout.seconds(30); + @NonNull protected StorIOSQLite storIOSQLite; diff --git a/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/NotifyAboutChangesTest.java b/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/NotifyAboutChangesTest.java index 1484262b6..9a8411dc8 100644 --- a/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/NotifyAboutChangesTest.java +++ b/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/NotifyAboutChangesTest.java @@ -6,7 +6,10 @@ import com.pushtorefresh.storio2.sqlite.BuildConfig; import com.pushtorefresh.storio2.sqlite.Changes; import com.pushtorefresh.storio2.sqlite.StorIOSQLite; +import com.pushtorefresh.storio2.test.Repeat; +import com.pushtorefresh.storio2.test.RepeatRule; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; @@ -28,6 +31,9 @@ @Config(constants = BuildConfig.class, sdk = 21) public class NotifyAboutChangesTest extends BaseTest { + @Rule + public RepeatRule repeat = new RepeatRule(); + @Test public void notifyAboutChange() { TestSubscriber testObserver = new TestSubscriber(); @@ -197,11 +203,12 @@ public void run() { } @Test + @Repeat(times = 20) public void shouldReceiveOneNotificationWithAllAffectedTablesInTransactionWithMultipleThreads() throws InterruptedException { final String table1 = "test_table1"; final String table2 = "test_table2"; - final int numberOfThreads = 100; + final int numberOfThreads = 50; final TestSubscriber testSubscriber = new TestSubscriber(); @@ -239,7 +246,7 @@ public void run() { // Steady! startAllThreadsLock.countDown(); // Go! - assertThat(allThreadsFinishedLock.await(20, SECONDS)).isTrue(); + assertThat(allThreadsFinishedLock.await(25, SECONDS)).isTrue(); // While we in transaction, no changes should be sent. assertThat(testSubscriber.getOnNextEvents()).hasSize(0); @@ -247,6 +254,7 @@ public void run() { lowLevel.endTransaction(); testSubscriber.assertNoErrors(); + List actualChanges = testSubscriber.getOnNextEvents(); assertThat(actualChanges).hasSize(1); assertThat(actualChanges.get(0).affectedTables()).containsOnly("test_table1", "test_table2"); @@ -298,4 +306,4 @@ public void run() { testSubscriber.assertNoErrors(); testSubscriber.assertNoValues(); } -} \ No newline at end of file +} diff --git a/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/RxQueryTest.java b/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/RxQueryTest.java index 026ff78fa..26ab59378 100644 --- a/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/RxQueryTest.java +++ b/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/RxQueryTest.java @@ -6,7 +6,10 @@ import com.pushtorefresh.storio2.sqlite.Changes; import com.pushtorefresh.storio2.sqlite.queries.Query; import com.pushtorefresh.storio2.test.AbstractEmissionChecker; +import com.pushtorefresh.storio2.test.Repeat; +import com.pushtorefresh.storio2.test.RepeatRule; +import org.junit.Rule; import org.junit.Test; import org.junit.runner.RunWith; import org.robolectric.RobolectricTestRunner; @@ -55,6 +58,9 @@ public void call(List users) { } } + @Rule + public RepeatRule repeat = new RepeatRule(); + @Test public void insertEmission() { final List initialUsers = putUsersBlocking(10); @@ -151,19 +157,20 @@ public void deleteEmission() { } @Test - public void parallelWritesWithoutTransaction() { - final int numberOfParallelWorkers = 50; + @Repeat(times = 20) + public void parallelPutWithoutGlobalTransaction() { + final int numberOfParallelPuts = 50; TestSubscriber testSubscriber = new TestSubscriber(); storIOSQLite .observeChangesInTable(TweetTableMeta.TABLE) - .take(numberOfParallelWorkers) + .take(numberOfParallelPuts) .subscribe(testSubscriber); final CountDownLatch countDownLatch = new CountDownLatch(1); - for (int i = 0; i < numberOfParallelWorkers; i++) { + for (int i = 0; i < numberOfParallelPuts; i++) { final int copyOfCurrentI = i; new Thread(new Runnable() { @Override @@ -188,7 +195,15 @@ public void run() { testSubscriber.awaitTerminalEvent(); testSubscriber.assertNoErrors(); - assertThat(testSubscriber.getOnNextEvents()).hasSize(numberOfParallelWorkers); + + // Put operation creates short-term transaction which might result in merge of some notifications. + // So we have two extreme cases: + // - no merged notifications → isEqualTo(numberOfParallelPuts) + // - all notifications merged → isEqualTo(1) + // Obviously truth is somewhere between those (depends on CPU of machine that runs test). + assertThat(testSubscriber.getOnNextEvents().size()) + .isLessThanOrEqualTo(numberOfParallelPuts) + .isGreaterThanOrEqualTo(1); } @Test diff --git a/storio-test-common/build.gradle b/storio-test-common/build.gradle index 5080ba9d6..5267f6fbb 100644 --- a/storio-test-common/build.gradle +++ b/storio-test-common/build.gradle @@ -23,6 +23,7 @@ dependencies { provided libraries.supportAnnotations provided libraries.rxJava + compile libraries.junit compile libraries.mockitoCore testCompile libraries.junit diff --git a/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/Repeat.java b/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/Repeat.java new file mode 100644 index 000000000..35d7f0820 --- /dev/null +++ b/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/Repeat.java @@ -0,0 +1,13 @@ +package com.pushtorefresh.storio2.test; + +import java.lang.annotation.Retention; +import java.lang.annotation.Target; + +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.RetentionPolicy.RUNTIME; + +@Retention(RUNTIME) +@Target(METHOD) +public @interface Repeat { + int times(); +} diff --git a/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/RepeatRule.java b/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/RepeatRule.java new file mode 100644 index 000000000..cc551a0ae --- /dev/null +++ b/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/RepeatRule.java @@ -0,0 +1,36 @@ +package com.pushtorefresh.storio2.test; + +import android.support.annotation.NonNull; + +import org.junit.rules.TestRule; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +public class RepeatRule implements TestRule { + + @Override + @NonNull + public Statement apply(@NonNull final Statement test, @NonNull final Description description) { + final Repeat repeat = description.getAnnotation(Repeat.class); + + if (repeat != null) { + return new Statement() { + @Override + public void evaluate() throws Throwable { + final int times = repeat.times(); + + if (times < 1) { + throw new IllegalArgumentException("Repeat times should be >= 1, times = " + times); + } + + for (int i = 0; i < times; i++) { + System.out.println(description.getMethodName() + " iteration " + (i + 1)); + test.evaluate(); + } + } + }; + } else { + return test; + } + } +} diff --git a/storio-test-common/src/test/java/com/pushtorefresh/storio2/test/RepeatRuleTest.java b/storio-test-common/src/test/java/com/pushtorefresh/storio2/test/RepeatRuleTest.java new file mode 100644 index 000000000..06db29361 --- /dev/null +++ b/storio-test-common/src/test/java/com/pushtorefresh/storio2/test/RepeatRuleTest.java @@ -0,0 +1,71 @@ +package com.pushtorefresh.storio2.test; + +import org.junit.Test; +import org.junit.runner.Description; +import org.junit.runners.model.Statement; + +import static org.assertj.core.api.Java6Assertions.assertThat; +import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.never; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +public class RepeatRuleTest { + + private Statement test = mock(Statement.class); + private Description description = mock(Description.class); + private Repeat repeat = mock(Repeat.class); + + @Test + public void repeats10Times() throws Throwable { + when(description.getAnnotation(Repeat.class)).thenReturn(repeat); + when(repeat.times()).thenReturn(10); + + new RepeatRule().apply(test, description).evaluate(); + + verify(test, times(10)).evaluate(); + } + + @Test + public void noAnnotation() throws Throwable { + when(description.getAnnotation(Repeat.class)).thenReturn(null); + + new RepeatRule().apply(test, description).evaluate(); + + verify(test).evaluate(); + } + + @Test + public void lessThan1Time() throws Throwable { + when(description.getAnnotation(Repeat.class)).thenReturn(repeat); + when(repeat.times()).thenReturn(0); + + try { + new RepeatRule().apply(test, description).evaluate(); + fail(); + } catch (IllegalArgumentException expected) { + assertThat(expected).hasMessage("Repeat times should be >= 1, times = 0"); + } + } + + @Test + public void lazyWithoutAnnotation() throws Throwable { + when(description.getAnnotation(Repeat.class)).thenReturn(null); + + new RepeatRule().apply(test, description); + + verify(test, never()).evaluate(); + } + + @Test + public void lazyWithAnnotation() throws Throwable { + when(description.getAnnotation(Repeat.class)).thenReturn(repeat); + when(repeat.times()).thenReturn(10); + + new RepeatRule().apply(test, description); + + verify(test, never()).evaluate(); + } +} From 65d38601debe76c3832e3250e6e419c17f21e539 Mon Sep 17 00:00:00 2001 From: Artem Zinnatullin Date: Sat, 14 Oct 2017 20:34:50 -0700 Subject: [PATCH 2/4] Dynamically compute number of threads for tests. --- .../sqlite/integration/NotifyAboutChangesTest.java | 3 ++- .../storio2/sqlite/integration/RxQueryTest.java | 3 ++- .../storio2/test/ConcurrencyTesting.java | 12 ++++++++++++ 3 files changed, 16 insertions(+), 2 deletions(-) create mode 100644 storio-test-common/src/main/java/com/pushtorefresh/storio2/test/ConcurrencyTesting.java diff --git a/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/NotifyAboutChangesTest.java b/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/NotifyAboutChangesTest.java index 9a8411dc8..5cf5efae9 100644 --- a/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/NotifyAboutChangesTest.java +++ b/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/NotifyAboutChangesTest.java @@ -6,6 +6,7 @@ import com.pushtorefresh.storio2.sqlite.BuildConfig; import com.pushtorefresh.storio2.sqlite.Changes; import com.pushtorefresh.storio2.sqlite.StorIOSQLite; +import com.pushtorefresh.storio2.test.ConcurrencyTesting; import com.pushtorefresh.storio2.test.Repeat; import com.pushtorefresh.storio2.test.RepeatRule; @@ -208,7 +209,7 @@ public void shouldReceiveOneNotificationWithAllAffectedTablesInTransactionWithMu final String table1 = "test_table1"; final String table2 = "test_table2"; - final int numberOfThreads = 50; + final int numberOfThreads = ConcurrencyTesting.optimalTestThreadsCount(); final TestSubscriber testSubscriber = new TestSubscriber(); diff --git a/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/RxQueryTest.java b/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/RxQueryTest.java index 26ab59378..2eb2782bf 100644 --- a/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/RxQueryTest.java +++ b/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/RxQueryTest.java @@ -6,6 +6,7 @@ import com.pushtorefresh.storio2.sqlite.Changes; import com.pushtorefresh.storio2.sqlite.queries.Query; import com.pushtorefresh.storio2.test.AbstractEmissionChecker; +import com.pushtorefresh.storio2.test.ConcurrencyTesting; import com.pushtorefresh.storio2.test.Repeat; import com.pushtorefresh.storio2.test.RepeatRule; @@ -159,7 +160,7 @@ public void deleteEmission() { @Test @Repeat(times = 20) public void parallelPutWithoutGlobalTransaction() { - final int numberOfParallelPuts = 50; + final int numberOfParallelPuts = ConcurrencyTesting.optimalTestThreadsCount(); TestSubscriber testSubscriber = new TestSubscriber(); diff --git a/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/ConcurrencyTesting.java b/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/ConcurrencyTesting.java new file mode 100644 index 000000000..a15e70e4b --- /dev/null +++ b/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/ConcurrencyTesting.java @@ -0,0 +1,12 @@ +package com.pushtorefresh.storio2.test; + +public class ConcurrencyTesting { + + private ConcurrencyTesting() { + throw new IllegalStateException("No instances!"); + } + + public static int optimalTestThreadsCount() { + return Runtime.getRuntime().availableProcessors() * 2; + } +} From b98f2b1487d968c7c815329f927647e5f4aeda2d Mon Sep 17 00:00:00 2001 From: Artem Zinnatullin Date: Sat, 14 Oct 2017 20:49:59 -0700 Subject: [PATCH 3/4] Fix take(). --- .../sqlite/integration/RxQueryTest.java | 27 ++++++++++--------- .../storio2/test/ConcurrencyTesting.java | 2 +- 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/RxQueryTest.java b/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/RxQueryTest.java index 2eb2782bf..f6de6dddb 100644 --- a/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/RxQueryTest.java +++ b/storio-sqlite/src/test/java/com/pushtorefresh/storio2/sqlite/integration/RxQueryTest.java @@ -159,42 +159,45 @@ public void deleteEmission() { @Test @Repeat(times = 20) - public void parallelPutWithoutGlobalTransaction() { - final int numberOfParallelPuts = ConcurrencyTesting.optimalTestThreadsCount(); + public void concurrentPutWithoutGlobalTransaction() throws InterruptedException { + final int numberOfConcurrentPuts = ConcurrencyTesting.optimalTestThreadsCount(); TestSubscriber testSubscriber = new TestSubscriber(); storIOSQLite .observeChangesInTable(TweetTableMeta.TABLE) - .take(numberOfParallelPuts) .subscribe(testSubscriber); - final CountDownLatch countDownLatch = new CountDownLatch(1); + final CountDownLatch concurrentPutLatch = new CountDownLatch(1); + final CountDownLatch allPutsDoneLatch = new CountDownLatch(numberOfConcurrentPuts); + + for (int i = 0; i < numberOfConcurrentPuts; i++) { + final int iCopy = i; - for (int i = 0; i < numberOfParallelPuts; i++) { - final int copyOfCurrentI = i; new Thread(new Runnable() { @Override public void run() { try { - countDownLatch.await(); + concurrentPutLatch.await(); } catch (InterruptedException e) { throw new RuntimeException(e); } storIOSQLite .put() - .object(Tweet.newInstance(null, 1L, "Some text: " + copyOfCurrentI)) + .object(Tweet.newInstance(null, 1L, "Some text: " + iCopy)) .prepare() .executeAsBlocking(); + + allPutsDoneLatch.countDown(); } }).start(); } - // Release the KRAKEN! - countDownLatch.countDown(); + // Start concurrent Put operations. + concurrentPutLatch.countDown(); - testSubscriber.awaitTerminalEvent(); + assertThat(allPutsDoneLatch.await(25, SECONDS)).isTrue(); testSubscriber.assertNoErrors(); // Put operation creates short-term transaction which might result in merge of some notifications. @@ -203,7 +206,7 @@ public void run() { // - all notifications merged → isEqualTo(1) // Obviously truth is somewhere between those (depends on CPU of machine that runs test). assertThat(testSubscriber.getOnNextEvents().size()) - .isLessThanOrEqualTo(numberOfParallelPuts) + .isLessThanOrEqualTo(numberOfConcurrentPuts) .isGreaterThanOrEqualTo(1); } diff --git a/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/ConcurrencyTesting.java b/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/ConcurrencyTesting.java index a15e70e4b..95928c69c 100644 --- a/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/ConcurrencyTesting.java +++ b/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/ConcurrencyTesting.java @@ -7,6 +7,6 @@ private ConcurrencyTesting() { } public static int optimalTestThreadsCount() { - return Runtime.getRuntime().availableProcessors() * 2; + return Math.max(Runtime.getRuntime().availableProcessors() * 2, 4); } } From c4415efb0b72e487c9cc2f7851e5706f8c896f67 Mon Sep 17 00:00:00 2001 From: Artem Zinnatullin Date: Sat, 14 Oct 2017 21:07:59 -0700 Subject: [PATCH 4/4] Test ConcurrencyTesting. --- .../storio2/test/ConcurrencyTesting.java | 2 +- .../storio2/test/ConcurrencyTestingTest.java | 24 +++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) create mode 100644 storio-test-common/src/test/java/com/pushtorefresh/storio2/test/ConcurrencyTestingTest.java diff --git a/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/ConcurrencyTesting.java b/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/ConcurrencyTesting.java index 95928c69c..6bd1e6e71 100644 --- a/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/ConcurrencyTesting.java +++ b/storio-test-common/src/main/java/com/pushtorefresh/storio2/test/ConcurrencyTesting.java @@ -3,7 +3,7 @@ public class ConcurrencyTesting { private ConcurrencyTesting() { - throw new IllegalStateException("No instances!"); + throw new IllegalStateException("No instances please."); } public static int optimalTestThreadsCount() { diff --git a/storio-test-common/src/test/java/com/pushtorefresh/storio2/test/ConcurrencyTestingTest.java b/storio-test-common/src/test/java/com/pushtorefresh/storio2/test/ConcurrencyTestingTest.java new file mode 100644 index 000000000..2dd1c46da --- /dev/null +++ b/storio-test-common/src/test/java/com/pushtorefresh/storio2/test/ConcurrencyTestingTest.java @@ -0,0 +1,24 @@ +package com.pushtorefresh.storio2.test; + +import com.pushtorefresh.private_constructor_checker.PrivateConstructorChecker; + +import org.junit.Test; + +import static org.assertj.core.api.Java6Assertions.assertThat; + +public class ConcurrencyTestingTest { + + @Test + public void optimalTestThreadsCountIsAtLeast4() { + assertThat(ConcurrencyTesting.optimalTestThreadsCount()).isGreaterThanOrEqualTo(4); + } + + @Test + public void constructorMustBePrivateAndThrowException() { + PrivateConstructorChecker + .forClass(ConcurrencyTesting.class) + .expectedTypeOfException(IllegalStateException.class) + .expectedExceptionMessage("No instances please.") + .check(); + } +}