Page MenuHomePhabricator
Create Task
Maniphest T75833

Flow: Off-by-one timestamp failures in tests
Closed, ResolvedPublic
Actions

Description

There are two timestamp-related off-by-one errors, both intermittent. It's not clear yet if they're related:

https://gerrit.wikimedia.org/r/#/c/172925/ had a failure on patch set 9.

03:08:27 1) Flow\Tests\Api\ApiFlowViewTopicListTest::testTocOnly
03:08:27 TOC-only output for "updated" order
03:08:27 Failed asserting that two arrays are equal.
03:08:27 --- Expected
03:08:27 +++ Actual
03:08:27 @@ @@
03:08:27                              'fwd' => Array (
03:08:27 -                                'url' => '/wiki/index.php?title=Talk:Flow_QA&topiclist_offset-dir=fwd&topiclist_limit=2&topiclist_offset=20141123030821&topiclist_sortby=updated'
03:08:27 +                                'url' => '/wiki/index.php?title=Talk:Flow_QA&topiclist_offset-dir=fwd&topiclist_limit=2&topiclist_offset=20141123030822&topiclist_sortby=updated'
03:08:27                                  'title' => 'fwd'
03:08:27                              )
03:08:27                          )
03:08:27                      )
03:08:27                      'errors' => Array ()
03:08:27                      'type' => 'topiclist'
03:08:27                  )
03:08:27              )
03:08:27              'status' => 'ok'
03:08:27          )
03:08:27      )
03:08:27  )

I then did a recheck and patch set 9 then worked.

Patch set 7 of https://gerrit.wikimedia.org/r/#/c/170997/ had another failure, also a timestamp off-by-one:

03:52:54 1) Flow\Tests\Model\UUIDTest::testConversionToTimestamp
03:52:54 Failed asserting that '20141125035244' matches expected '20141125035243'.
03:52:54 
03:52:54 /srv/ssd/jenkins-slave/workspace/mwext-Flow-testextension/src/extensions/Flow/tests/phpunit/Model/UUIDTest.php:160
03:52:54 /srv/ssd/jenkins-slave/workspace/mwext-Flow-testextension/src/tests/phpunit/MediaWikiTestCase.php:141

Again, it went away on recheck.

Details

SubjectRepoBranchLines +/-
Fix intermittent off by one issuesmediawiki/extensions/Flowmaster+3 -3
Customize query in gerrit

Related Objects

Event Timeline

• Mattflaschen-WMF created this task.Nov 25 2014, 5:18 AM
• Mattflaschen-WMF raised the priority of this task from to Medium.
• Mattflaschen-WMF updated the task description. (Show Details)
• Mattflaschen-WMF added a project: StructuredDiscussions.
• Mattflaschen-WMF changed Security from none to None.
• Mattflaschen-WMF added a subscriber: EBernhardson.
• Mattflaschen-WMF subscribed.
EBernhardson added a comment.EditedNov 25 2014, 6:55 AM

I think these are both caused by the timestamp ticking over from one second to the next.

In the ViewTopicList case we are sourcing the timestamp from the latest revision id, but the timestamp in the url comes from the workflow. The workflow timestamp is assigned in TopicBlock::commit() just before writing things out, there is the potential for a few ms between creating the revision and commiting it. It makes sense for the workflow to use the exact same timestamp, so https://gerrit.wikimedia.org/r/175664 makes that happen. As long as we are making timestamps match I also submitted https://gerrit.wikimedia.org/r/175665 to match a revisions moderation timestamp to the revision id. In both these cases though, i almost wonder if we should be storing id's instead of timestamps.

In the UUIDTest its directly comparing wfTimestampNow() to a uuid, another opportunity for the timestamps to be created on either side of the second tick. I don't have much of a solution for this atm, we could move uid generation behind a mockable interface and use consistent timestamps on the unit test side through HistoricalUIDGenerator

• Mattflaschen-WMF assigned this task to EBernhardson.Nov 25 2014, 7:08 PM
• Mattflaschen-WMF added a comment.Nov 25 2014, 7:16 PM
In T75833#783480, @EBernhardson wrote:

In the ViewTopicList case we are sourcing the timestamp from the latest revision id, but the timestamp in the url comes from the workflow.

From the topic workflow?

EBernhardson added a comment.Nov 25 2014, 7:18 PM

Yes the topic workflow. When you have a numerical offset like in this test that means we are sorting by the workflow_last_updated_timestamp of the related topic workflows.

• Mattflaschen-WMF added a project: Collaboration-Team-Triage.Feb 24 2015, 9:24 PM
• Mattflaschen-WMF moved this task from Untriaged to Team discussion on the Collaboration-Team-Triage board.
• Mattflaschen-WMF raised the priority of this task from Medium to High.Mar 3 2015, 10:32 PM

This really needs to be fixed.

Krinkle subscribed.Mar 3 2015, 10:37 PM

This is currently causing mediawiki-core integration tests to fail and block commits in other repositories being merged from time to time. This should be fixed within a day. Alternatively, I'll disable the relevant test for the time being. No worries :)

Krinkle added a project: Continuous-Integration-Infrastructure.Mar 3 2015, 10:38 PM
Krinkle moved this task from Untriaged to In-progress on the Continuous-Integration-Infrastructure board.
• Mattflaschen-WMF added a comment.Mar 3 2015, 11:13 PM

Per our IRC conversation: For the testConverionToTimestamp, I think we can just do:

UUID::create( 'somehex' )->getTimestamp()

which will have a deterministic output, and still test our code (we're not trying to test core's use of the time).

gerritbot subscribed.Mar 3 2015, 11:37 PM

Change 194226 had a related patch set uploaded (by EBernhardson):
Fix intermittent off by one issues

https://gerrit.wikimedia.org/r/194226

gerritbot added a project: Patch-For-Review.Mar 3 2015, 11:37 PM
hashar removed a project: Continuous-Integration-Infrastructure.Mar 4 2015, 10:42 AM
gerritbot added a comment.Mar 4 2015, 8:36 PM

Change 194226 merged by jenkins-bot:
Fix intermittent off by one issues

https://gerrit.wikimedia.org/r/194226

EBernhardson mentioned this in rEFLW0fb2edab871e: Fix intermittent off by one issues.Mar 4 2015, 8:41 PM
• Mattflaschen-WMF closed this task as Resolved.Mar 4 2015, 8:47 PM
EBernhardson mentioned this in rMEXT1c0419333f1e: Updated mediawiki/extensions Project: mediawiki/extensions/Flow….Mar 11 2015, 6:28 AM
• DannyH moved this task from Team discussion to Resolved on the Collaboration-Team-Triage board.Apr 1 2015, 7:25 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits