#585 - Multi-threaded tests for SyncIterator class #639

driver733 · 2018-02-14T00:42:47Z

As per #585

I have written three multi-threaded tests for the SyncIterator class.
The first test deals with two concurrent iterator.next() method calls and tests that the calls are able to produce the correct output even when they are accessed concurrently.
The second test deals with two concurrent methods calls (iterator.next() and iterator.hasNext()).
The last added test has two concurrent iterator.hasNext() calls.

I have also added the Concurrent inner class that tests the supplied Runnables for concurrency problems. I have constructed this class based on the information found here.

0crat · 2018-02-14T00:43:14Z

Job #639 is now in scope, role is REV

driver733 · 2018-02-14T00:45:21Z

src/test/java/org/cactoos/iterator/SyncIteratorTest.java

+                    Matchers.hasItem("b")
+                )
+            );
+            MatcherAssert.assertThat(


@amihaiemil Two matchers have to be used here, because the compiler complains about using Object arrays in matchers (contaisInAnyOrder with different object types). (produces a compiler warning)

codecov-io · 2018-02-14T00:52:12Z

Codecov Report

Merging #639 into master will increase coverage by 0.4%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master     #639     +/-   ##
===========================================
+ Coverage     69.96%   70.37%   +0.4%     
- Complexity     1001     1022     +21     
===========================================
  Files           213      220      +7     
  Lines          3300     3335     +35     
  Branches        192      190      -2     
===========================================
+ Hits           2309     2347     +38     
+ Misses          944      941      -3     
  Partials         47       47

Impacted Files	Coverage Δ	Complexity Δ
src/main/java/org/cactoos/io/TeeOutputStream.java	`52.63% <0%> (-15.79%)`	`3% <0%> (-1%)`
src/main/java/org/cactoos/iterable/Reversed.java	`66.66% <0%> (-13.34%)`	`2% <0%> (-1%)`
src/main/java/org/cactoos/io/TeeInputStream.java	`56.52% <0%> (-13.05%)`	`6% <0%> (-1%)`
src/main/java/org/cactoos/scalar/ItemAt.java	`84.44% <0%> (-0.34%)`	`19% <0%> (+1%)`
...main/java/org/cactoos/iterator/StickyIterator.java
src/main/java/org/cactoos/scalar/SumOfScalar.java	`100% <0%> (ø)`	`3% <0%> (?)`
src/main/java/org/cactoos/map/Grouped.java	`100% <0%> (ø)`	`2% <0%> (?)`
src/main/java/org/cactoos/iterator/IteratorOf.java	`100% <0%> (ø)`	`5% <0%> (?)`
src/main/java/org/cactoos/scalar/Constant.java	`100% <0%> (ø)`	`2% <0%> (?)`
...c/main/java/org/cactoos/scalar/SumOfIntScalar.java	`100% <0%> (ø)`	`2% <0%> (?)`
... and 7 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eff4166...41da93f. Read the comment docs.

0crat · 2018-02-14T01:05:57Z

This pull request #639 is assigned to @amihaiemil /z, here is why. The budget is 15 minutes, see §4. Please, read §27 and when you decide to accept the changes, inform @yegor256 /z (the architect) right in this ticket. If you decide that this PR should not be accepted ever, also inform the architect.

amihaiemil · 2018-02-16T16:40:05Z

@driver733 please, add a proper title and description to this PR. What is it about, what tests were added etc.

amihaiemil

@driver733 looks ok, just one comment :)

amihaiemil · 2018-02-16T16:47:21Z

src/test/java/org/cactoos/iterator/SyncIteratorTest.java

+         */
+        private final CountDownLatch done;
+
+        Concurrent(final List<? extends Runnable> runnables) {


@driver733 I think here, instead of a List, you could accept Runnable... and then in the tests you won't have to instantiate Runnables, add them to a List and use that List. You can directly instantiate Concurrent and give the Runnables directly as lambdas.

@amihaiemil I agree with you. I will make this change now.

driver733 · 2018-02-16T16:52:40Z

@amihaiemil I have updated the title and the description of the PR.

driver733 · 2018-02-16T17:20:20Z

@amihaiemil Done. (#639 (comment))

amihaiemil · 2018-02-16T17:21:19Z

@rultor good to merge

rultor · 2018-02-16T17:22:32Z

@rultor good to merge

@amihaiemil Thanks for your request. @yegor256 Please confirm this.

0crat · 2018-02-19T01:06:46Z

@amihaiemil /z this job was assigned to you 5 days ago. It will be taken away from you soon, unless you close it, see §8. Read this and this, please.

amihaiemil · 2018-02-19T08:15:53Z

@yegor256 ping, don't forget about this one. Also, I have several other PRs which are waiting arch approval :)

yegor256 · 2018-02-19T18:50:21Z

src/test/java/org/cactoos/iterator/SyncIteratorTest.java

 import java.util.concurrent.locks.ReentrantReadWriteLock;
+import junit.framework.TestCase;


@driver733 we don't use this. Use Hamcrest instead, like all other tests are doing

yegor256 · 2018-02-19T18:50:33Z

@driver733 see above

driver733 · 2018-02-19T19:22:55Z

@yegor256 Done.

yegor256 · 2018-02-19T19:26:22Z

src/test/java/org/cactoos/iterator/SyncIteratorTest.java

+    /**
+     * Tests runnables for concurrency issues.
+     */
+    private final class Concurrent {


@driver733 I have a big suspicion that you are reinventing here what we already have in RunInThreads. Can you please check?

yegor256 · 2018-02-19T19:26:58Z

@driver733 @amihaiemil can you please check how RunInThreads is used in other tests? It seems that in this PR we are reinventing the same functionality.

driver733 · 2018-02-19T20:21:51Z

@amihaiemil
@yegor256
ping

amihaiemil

@driver733 looks nice :)

amihaiemil · 2018-02-20T12:56:12Z

@yegor256 ping

yegor256 · 2018-02-20T13:19:14Z

@rultor merge

rultor · 2018-02-20T13:29:21Z

@rultor merge

@yegor256 OK, I'll try to merge now. You can check the progress of the merge here

rultor · 2018-02-20T13:38:56Z

@rultor merge

@yegor256 Done! FYI, the full log is here (took me 9min)

0crat · 2018-02-20T13:39:11Z

@ypshenychka /z please review this job, as in §30; the job will be fully closed and all payments will be made when the quality review is completed

ypshenychka · 2018-02-20T15:21:57Z

@amihaiemil According to our QA Rules:

The code reviewer found at least three problems in the code.

Only one issue was found during code review. Please confirm that you'll try to find at least three problems while future reviews.

ypshenychka · 2018-02-20T15:23:01Z

@driver733 According to our QA Rules:

Messages in a ticket always start with a name of a user they are addressed to.

Please correct your code review comment by indicating an addressee in the beginning.

amihaiemil · 2018-02-20T16:15:54Z

@ypshenychka I confirm

ypshenychka · 2018-02-20T16:19:43Z

@amihaiemil Thanks!

amihaiemil · 2018-02-21T17:09:03Z

@ypshenychka I think the author made the changes here -- addressed the comments :)

driver733 · 2018-02-21T17:13:51Z

@ypshenychka Done.

ypshenychka · 2018-02-21T19:17:34Z

@driver733 Thank you!

ypshenychka · 2018-02-21T19:17:54Z

@0crat quality acceptable

0crat · 2018-02-21T19:20:30Z

QA review completed: +15 points just awarded to @ypshenychka /z, total is +510

0crat · 2018-02-21T19:21:18Z

Order was finished, quality was "acceptable": +15 points just awarded to @amihaiemil /z, total is +905

0crat · 2018-02-21T19:21:30Z

@0crat quality acceptable (here)

@ypshenychka You can't do that, unless you have one of these roles: ARC, PO. Your current roles are: QA.

0crat · 2018-02-21T19:21:38Z

Payment to ARC for a closed pull request, as in §28: +15 points just awarded to @yegor256 /z, total is +10619

0crat · 2018-02-28T17:55:42Z

This pull request #639 is assigned to @neonailol /z, here is why. The budget is 15 minutes, see §4. Please, read §27 and when you decide to accept the changes, inform @yegor256 /z (the architect) right in this ticket. If you decide that this PR should not be accepted ever, also inform the architect.

yegor256 · 2018-02-28T19:34:12Z

@0crat refuse

yegor256 · 2018-02-28T19:34:16Z

@0crat out

0crat · 2018-02-28T19:45:05Z

@0crat refuse (here)

@yegor256 The user @neonailol /z resigned from #639, please stop working. Reason for job resignation: Order was cancelled

0crat · 2018-02-28T19:45:24Z

@0crat out (here)

@yegor256 The job #639 is now out of scope

0crat · 2018-02-28T19:45:32Z

@0crat out (here)

@yegor256 Job #639 was not in scope

Mikhail Yakushin added 2 commits February 13, 2018 19:08

#585 - Completed.

6f44f67

#585 - Refactored method to a class.

9d6c61b

0crat added the scope label Feb 14, 2018

driver733 commented Feb 14, 2018

View reviewed changes

#585 - Fix.

6746546

driver733 changed the title ~~#585~~ #585 - Multi-threaded tests for SyncIterator class Feb 16, 2018

amihaiemil reviewed Feb 16, 2018

View reviewed changes

#585 - Refactored list to varargs in ctor.

d3ba2fd

yegor256 reviewed Feb 19, 2018

View reviewed changes

#585 - Refactored TestCase to MatcherAssert

2bc525e

yegor256 reviewed Feb 19, 2018

View reviewed changes

#585 - Refactored class to RunsInThreads.

41da93f

amihaiemil reviewed Feb 20, 2018

View reviewed changes

rultor merged commit 41da93f into yegor256:master Feb 20, 2018

driver733 mentioned this pull request Feb 20, 2018

SyncIteratorTest.java:33-35: add multi-threaded tests... #585

Closed

0crat removed the scope label Feb 28, 2018

		import java.util.concurrent.locks.ReentrantReadWriteLock;
		import junit.framework.TestCase;

#585 - Multi-threaded tests for SyncIterator class #639

#585 - Multi-threaded tests for SyncIterator class #639

Conversation

driver733 commented Feb 14, 2018 • edited

0crat commented Feb 14, 2018

driver733 Feb 14, 2018 • edited

Choose a reason for hiding this comment

codecov-io commented Feb 14, 2018 • edited

Codecov Report

0crat commented Feb 14, 2018

amihaiemil commented Feb 16, 2018

amihaiemil left a comment

Choose a reason for hiding this comment

amihaiemil Feb 16, 2018 • edited

Choose a reason for hiding this comment

driver733 Feb 16, 2018

Choose a reason for hiding this comment

driver733 commented Feb 16, 2018

driver733 commented Feb 16, 2018

amihaiemil commented Feb 16, 2018

rultor commented Feb 16, 2018

0crat commented Feb 19, 2018

amihaiemil commented Feb 19, 2018

yegor256 Feb 19, 2018

Choose a reason for hiding this comment

yegor256 commented Feb 19, 2018

driver733 commented Feb 19, 2018

yegor256 Feb 19, 2018

Choose a reason for hiding this comment

yegor256 commented Feb 19, 2018

driver733 commented Feb 19, 2018

amihaiemil left a comment

Choose a reason for hiding this comment

amihaiemil commented Feb 20, 2018

yegor256 commented Feb 20, 2018

rultor commented Feb 20, 2018

rultor commented Feb 20, 2018

0crat commented Feb 20, 2018

ypshenychka commented Feb 20, 2018

ypshenychka commented Feb 20, 2018

amihaiemil commented Feb 20, 2018

ypshenychka commented Feb 20, 2018

amihaiemil commented Feb 21, 2018

driver733 commented Feb 21, 2018

ypshenychka commented Feb 21, 2018

ypshenychka commented Feb 21, 2018

0crat commented Feb 21, 2018

0crat commented Feb 21, 2018

0crat commented Feb 21, 2018

0crat commented Feb 21, 2018

0crat commented Feb 28, 2018

yegor256 commented Feb 28, 2018

yegor256 commented Feb 28, 2018

0crat commented Feb 28, 2018

0crat commented Feb 28, 2018

0crat commented Feb 28, 2018

driver733 commented Feb 14, 2018 •

edited

driver733 Feb 14, 2018 •

edited

codecov-io commented Feb 14, 2018 •

edited

amihaiemil Feb 16, 2018 •

edited