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

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

Closed
0pdd opened this issue Jan 29, 2018 · 34 comments
Closed

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

0pdd opened this issue Jan 29, 2018 · 34 comments
Labels

Comments

@0pdd
Copy link
Collaborator

0pdd commented Jan 29, 2018

The puzzle 482-5994e3af from #482 has to be resolved:

// @todo #482:30min add multi-threaded tests which test that the lock syncs the
// access to the next method against next and hasNext calls and calls to the
// hasNext method against next calls.

The puzzle was created by Sven Diedrichsen on 25-Jan-18.

Estimate: 30 minutes, role: IMP.

If you have any technical questions, don't ask me, submit new tickets instead. The task will be "done" when the problem is fixed and the text of the puzzle is removed from the source code. Here is more about PDD and about me.

@0crat
Copy link
Collaborator

0crat commented Jan 29, 2018

@yegor256/z please, pay attention to this issue

@0crat 0crat removed their assignment Jan 29, 2018
@0crat 0crat added the scope label Jan 29, 2018
@0crat
Copy link
Collaborator

0crat commented Jan 29, 2018

Job #585 is now in scope, role is DEV

@0crat
Copy link
Collaborator

0crat commented Feb 1, 2018

The job #585 assigned to @Icok/z. The budget is 30 minutes, see §4. Please, read §8 and §9. If the task is not clear, read this and this.

@0crat
Copy link
Collaborator

0crat commented Feb 6, 2018

@Icok/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.

@nick318
Copy link

nick318 commented Feb 8, 2018

@svendiedrichsen To follow the discussion we had in #607, how my test for hasNext should look like, if such tests it is all about percentage of failure. As you said from 40% to 4.8%. How can we assert such percents?

So far I wrote one test for next method: https://gist.github.com/nick318/543bf5e2aea11c7050a3286d6f4f24ee

And another one for hasNext:
https://gist.github.com/nick318/fd41d239dd02222f1ac851db680aa3a9

My question is, what assert can I use to verify hasNext method?

@svendiedrichsen
Copy link
Contributor

@nick318 Sorry, but the discussion on #607 was about the necessaty of synchronizing the hasNext call. My suggestion to you was about how you can proof that it is useful.

The tests we need are totally different. We need to proof that

  1. calls to the next method block calls to the hasNext and next from another thread.
  2. call to the hasNext method block only calls from the next and not calls to the hasNext from another thread.
    Simply speaking. We need to test that the synchronization really does do the job that it is supposed to do and not calculate any kind of failure ratio.

@nick318
Copy link

nick318 commented Feb 8, 2018

@svendiedrichsen Thanks for the fast response. Do I go in a right direction with first method?

@svendiedrichsen
Copy link
Contributor

svendiedrichsen commented Feb 8, 2018

@nick318 Not quiet. You can't really tell from the result of your test if it worked by accident.
Maybe you pass in a Lock into the iterator, call writeLock, spawn one extra thread to call next, call next in the test and than release the lock. That way you could see that the test gets the first value and the extra thread gets the second.
I couldn't do in proper OOP style myself. Thats why I left the puzzle in the code.

@nick318
Copy link

nick318 commented Feb 8, 2018

@svendiedrichsen But inside, iterator will unlock my lock which I made in the test. Since there is code:
this.lock.writeLock().unlock();

@svendiedrichsen
Copy link
Contributor

svendiedrichsen commented Feb 8, 2018

@nick318 It won't. Because you should have called writelock().lock() outside the iterator first which will cause the call writelock().lock() inside the iterator to block until you have unlocked outside.
Remember to pass into the iterator a lock and will use outside to block the iterator.

@0crat
Copy link
Collaborator

0crat commented Feb 9, 2018

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

@nick318
Copy link

nick318 commented Feb 9, 2018

@0crat refuse

@0crat
Copy link
Collaborator

0crat commented Feb 9, 2018

You need to ask somebody to invite you: https://github.com/zerocracy/datum#invite-only

@nick318
Copy link

nick318 commented Feb 10, 2018

@0crat waiting

@0crat
Copy link
Collaborator

0crat commented Feb 10, 2018

You need to ask somebody to invite you: https://github.com/zerocracy/datum#invite-only

@0crat
Copy link
Collaborator

0crat commented Feb 11, 2018

The user @Icok/z resigned from #585, please stop working

@0crat
Copy link
Collaborator

0crat commented Feb 13, 2018

The job #585 assigned to @driver733/z, here is why. The budget is 30 minutes, see §4. Please, read §8 and §9. If the task is not clear, read this and this.

@driver733
Copy link
Contributor

@0crat waiting

@0crat
Copy link
Collaborator

0crat commented Feb 15, 2018

@0crat waiting (here)

@driver733 The impediment for #585 was registered successfully by @driver733/z

@0pdd
Copy link
Collaborator Author

0pdd commented Feb 20, 2018

The puzzle 482-5994e3af has disappeared from the source code, that's why I closed this issue.

@0crat
Copy link
Collaborator

0crat commented Feb 20, 2018

@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
Copy link

@driver733 According to our QA Rules:

Some of ticket messages must mention all pull requests where the problem was actually fixed.

Please provide a link to corresponding PR in any of your messages.

@driver733
Copy link
Contributor

driver733 commented Feb 20, 2018

@ypshenychka
It has been mentioned here.
#585 (reference)

What is the reason to mention it again in messages? We reference issues in PR and commits.
@yegor256 Please correct me if I am wrong.

@ypshenychka
Copy link

@nick318 This reference is not enough. It can be hard to find the real PR among other references on commits, different referenced issues etc.
So please stick to the rules and provide a link in any of your messages, so we could finish with this job.

@driver733
Copy link
Contributor

@ypshenychka Fixed.
PR - #639.

@ypshenychka
Copy link

@driver733 Thanks.

@ypshenychka
Copy link

@0crat quality acceptable

@0crat
Copy link
Collaborator

0crat commented Feb 20, 2018

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

@0crat
Copy link
Collaborator

0crat commented Feb 20, 2018

@0crat quality acceptable (here)

@ypshenychka There is an unrecoverable failure on my side. Please, submit it here:

com.zerocracy.farm.reactive.StkGroovy[125] java.lang.IllegalStateException: java.lang.reflect.InvocationTargetException in com/zerocracy/stk/pm/cost/make_payment.groovy
sun.reflect.GeneratedMethodAccessor61[-1] java.lang.reflect.InvocationTargetException: null
com.zerocracy.pmo.banks.Paypal[122] java.io.IOException: Failed to pay $8.00 to driver733@me.com with memo "Payment for gh:yegor256/cactoos#585 (30 minutes): Order was finished, quality was "acceptable"": java.io.IOException Failed to pay through PayPal: [ERROR APPLICATION The payment can not be processed because no payment source is available]
com.zerocracy.pmo.banks.Paypal[202] java.io.IOException: Failed to pay through PayPal: [ERROR APPLICATION The payment can not be processed because no payment source is available]

1.0-SNAPSHOT /cc @yegor256

@0crat
Copy link
Collaborator

0crat commented Feb 20, 2018

Order was finished, quality was "acceptable": +30 points just awarded to @driver733/z, total is +290

@0pdd
Copy link
Collaborator Author

0pdd commented Feb 22, 2018

@0pdd all puzzles are solved

3 similar comments
@0pdd
Copy link
Collaborator Author

0pdd commented Feb 22, 2018

@0pdd all puzzles are solved

@0pdd
Copy link
Collaborator Author

0pdd commented Feb 23, 2018

@0pdd all puzzles are solved

@0pdd
Copy link
Collaborator Author

0pdd commented Feb 23, 2018

@0pdd all puzzles are solved

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

No branches or pull requests

6 participants