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

JBPM-4586 - Removal of loops with calls to Thread.sleep(). #243

Closed
wants to merge 3 commits into from

Conversation

baldimir
Copy link
Member

}
// Timer in the process takes 1000ms, so after 2 seconds, there should be 2 process IDs in the list.
// 2100ms is here because it is important to be sure that 2 seconds have passed.
Thread.sleep(2100);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on the previous code, shouldn't the wait be 2500ms? Or maybe the 1200ms is wrong and the process indeed takes 1000ms?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made some changes to this. Take a look if it is OK. The main difference is that I created a constant that defines extra time needed for Thread.sleep() calls so the extra sleep time should be easily configurable in the future. Now it is 250ms. I think that this should be enough now to cover all tests that are in StartEventTest class because now there are no loops with Thread.sleep().

@mswiderski
Copy link
Contributor

just tried to merge this but several builds fails with this:
Failed tests:
StartEventTest.testTimerStartCron:227->Assert.assertEquals:542->Assert.assertEquals:555->Assert.assertEquals:118->Assert.failNotEquals:743->Assert.fail:88 expected:<5> but was:<6>
StartEventTest.testTimerStartCron:227->Assert.assertEquals:542->Assert.assertEquals:555->Assert.assertEquals:118->Assert.failNotEquals:743->Assert.fail:88 expected:<5> but was:<6>

@baldimir
Copy link
Member Author

Hmm... Strange. I tested it several times. OK I will recheck it on Monday.

@baldimir
Copy link
Member Author

Changed the cron test so it waits approximately the same time as it was before (5s as was before, but without Thread.sleep in cycle). Tested it again, it should work.

@mswiderski
Copy link
Contributor

merged into master, thanks @baldimir !

@mswiderski mswiderski closed this Apr 20, 2015
@baldimir baldimir deleted the StartEventTestFix branch May 29, 2018 14:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants