Got this in http://qa.drupal.org/pifr/test/480823 (will trigger a re-test later and then it might no longer be visible)

Failed asserting that 16.96 is less than 15. Other TimerUnitTest.php 33 Drupal\Tests\Component\Utility\TimerUnitTest->testTimer()

CommentFileSizeAuthor
#18 1960032.timer-test.18.patch1.76 KBarlinsandbulte
#16 1960032.timer-test.16.patch1.79 KBarlinsandbulte
#14 1960032.timer-test.14.patch1.75 KBalexpott
#4 1960032.timer-test.4.patch1.74 KBalexpott
#3 timer-test-1960032-3.patch2.38 KBtim.plunkett
#3 timer-test-1960032-3.patch2.38 KBtim.plunkett
#3 timer-test-1960032-3.patch2.38 KBtim.plunkett
#3 timer-test-1960032-3.patch2.38 KBtim.plunkett
#3 timer-test-1960032-3.patch2.38 KBtim.plunkett
#3 timer-test-1960032-3.patch2.38 KBtim.plunkett
#3 timer-test-1960032-3.patch2.38 KBtim.plunkett
#3 timer-test-1960032-3.patch2.38 KBtim.plunkett
#3 timer-test-1960032-3.patch2.38 KBtim.plunkett
#3 timer-test-1960032-3.patch2.38 KBtim.plunkett
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented

The result should have been ~10 milliseconds, not 16.96.

The original tests (before #1935970: Convert timer_* to a utility class and convert tests to phpunit) were waiting 4 *seconds*, and it was dropped to 5 milliseconds. Is that really reasonable to expect?

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Hm. I guess that when running 8 tests in parallel, it can sometimes happen that you have to wait a few milliseconds more than you wanted.

I'd suggest to simply double those values.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
CreditAttribution: tim.plunkett commented
Status: Active » Needs review
FileSize
timer-test-1960032-3.patch2.38 KB
timer-test-1960032-3.patch2.38 KB
timer-test-1960032-3.patch2.38 KB
timer-test-1960032-3.patch2.38 KB
timer-test-1960032-3.patch2.38 KB
timer-test-1960032-3.patch2.38 KB
timer-test-1960032-3.patch2.38 KB
timer-test-1960032-3.patch2.38 KB
timer-test-1960032-3.patch2.38 KB
timer-test-1960032-3.patch2.38 KB
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
FileSize
1960032.timer-test.4.patch1.74 KB

There has only been one reported sighting of this failure...

Looking at the tests in \Drupal\Tests\Component\Utility\TimerUnitTest it seems we're testing the resolution of usleep as well as the ability of the Timer utility class. Perhaps just removing $this->assertLessThan assertions and doing the tests along the lines in the patch attached is sufficient.

ParisLiakos’s picture

ParisLiakos CreditAttribution: ParisLiakos commented

another one
http://qa.drupal.org/pifr/test/482078
Failed asserting that 21.72 is less than 20. Other TimerUnitTest.php 33 Drupal\Tests\Component\Utility\TimerUnitTest->testTimer()

msonnabaum’s picture

msonnabaum CreditAttribution: msonnabaum commented

I changed it to 5ms because that's where it was stable after a couple hundred times of running it locally for myself. That may be too low if testbot is running a high loadavg. I think it's fine to raise it some, just not where it was.

msonnabaum’s picture

msonnabaum CreditAttribution: msonnabaum commented

I suppose we could do what's in #4. If that's a good enough test though, I dont know that we need 4 assertions.

IMO raising the interval from 5ms to 50ms should do the trick, but I'm not necessarily opposed to #4.

Berdir’s picture

Berdir
Primary language German
Location Switzerland
CreditAttribution: Berdir commented

Just had another one of those, "Failed asserting that 20.14 is less than 20. Other TimerUnitTest.php 33 Drupal\Tests\Component\Utility\TimerUnitTest->testTimer()".

chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Reviewed & tested by the community

Blargh, we aren't exactly interested in the instruction speed of Timer::read. Ripping out the top is fine.

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented
Status: Reviewed & tested by the community » Fixed

Woohoo! Thanks for the fast turnaround on this one, folks. I agree those extra assertions seem superfluous.

Committed and pushed to 8.x. Thanks!

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.

Heine’s picture

Heine CreditAttribution: Heine commented
Status: Closed (fixed) » Active

On todays checkout I still get frequent fails such as

"PHPUnit_Framework_ExpectationFailedException : Timer measured at least 5 milliseconds of sleeping while running.
Failed asserting that 4.83 is equal to 5 or is greater than 5.

Windows 8, phpunit via phpstorm, PHP 5.4.4 cli.

amateescu’s picture

amateescu CreditAttribution: amateescu commented

I get the same kind of failures on the exact same setup, but W7 instead of 8.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott commented
Status: Active » Needs review
FileSize
1960032.timer-test.14.patch1.75 KB

Window's users how does this work for you?

amateescu’s picture

amateescu CreditAttribution: amateescu commented
Status: Needs review » Reviewed & tested by the community

Wfm, yes :)

arlinsandbulte’s picture

arlinsandbulte CreditAttribution: arlinsandbulte commented
Status: Reviewed & tested by the community » Needs review
FileSize
1960032.timer-test.16.patch1.79 KB

Missing punctuation made the comments REALLY hard to read & understand.
So, I just fixed the comments & did some minor re-wording to them.
Still RTBC if tests pass.

Status: Needs review » Needs work

The last submitted patch, 1960032.timer-test.16.patch, failed testing.

arlinsandbulte’s picture

arlinsandbulte CreditAttribution: arlinsandbulte commented
Status: Needs work » Needs review
FileSize
1960032.timer-test.18.patch1.76 KB

Let's try this again,

arlinsandbulte’s picture

arlinsandbulte CreditAttribution: arlinsandbulte commented
Status: Needs review » Reviewed & tested by the community

That's better.
Back to RTBC as I only slightly changed the comments.

arlinsandbulte’s picture

arlinsandbulte CreditAttribution: arlinsandbulte commented
Status: Reviewed & tested by the community » Fixed

Looks like #18 was committed by webchick, but not marked fixed here.
Commit: http://drupalcode.org/project/drupal.git/commit/2d32931

Another critical off the list!

webchick’s picture

webchick
she/they
Primary language English
Location Vancouver 🇨🇦
CreditAttribution: webchick commented

Oops. :D Forgot I did that. ;) Yay!

Automatically closed -- issue fixed for 2 weeks with no activity.