Page MenuHomePhabricator
Differential D12346

[commtest] Fix flaky identity-tunnelbroker test
ClosedPublic
Actions

Authored by bartek on Fri, Jun 7, 1:07 AM.
Tags
None
Referenced Files
F2176939: D12346.id.diff
Wed, Jul 3, 5:59 AM
Unknown Object (File)
Sun, Jun 23, 1:51 PM
Unknown Object (File)
Sat, Jun 22, 11:50 AM
Unknown Object (File)
Fri, Jun 21, 6:01 AM
Unknown Object (File)
Thu, Jun 20, 3:33 AM
Unknown Object (File)
Wed, Jun 19, 5:22 PM
Unknown Object (File)
Tue, Jun 18, 8:50 PM
Unknown Object (File)
Mon, Jun 17, 5:05 PM
View All 18 Files
Subscribers
ashoat
tomek

Details

Reviewers
kamil
varun
Commits
rCOMM522d00e1ed6c: [commtest] Fix flaky identity-tunnelbroker test
Summary

This test stops working when I introduced random and unique device IDs.

What was wrong:

  1. It was using register_user_device(None, None) which always created a user with the same (constant) device ID. Tunnelbroker was treating this as it was always the same device. So multiple users were treated as one recipient.
  2. When fixed this, it turns out that RefreshKeysRequest is expected to be sent twice, but recently it was fixed to be sent only once. Previous behavior was mentioned in https://phab.comm.dev/D9577?id=32347#inline-60330 It was working before only because other tests also sent refresh requests that were enqueued in Tunnelbroker for this constant device ID.
Test Plan

Commtest - before and after random device IDs

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

bartek created this revision.Fri, Jun 7, 1:07 AM
bartek held this revision as a draft.
Herald added subscribers: tomek, ashoat. · View Herald TranscriptFri, Jun 7, 1:07 AM
bartek added a child revision: D12341: [commtest] Generate random client keys by default.Fri, Jun 7, 1:08 AM
Harbormaster failed remote builds in B29516: Diff 41089!Fri, Jun 7, 1:11 AM
bartek published this revision for review.Fri, Jun 7, 1:57 AM

Keyserver CI unrelated

services/commtest/tests/identity_tunnelbroker_tests.rs
76 ↗(On Diff #41089)

Not sure why we previously registered a device again.

Harbormaster completed remote builds in B29516: Diff 41089.Sun, Jun 9, 11:25 PM
kamil accepted this revision.Mon, Jun 10, 10:11 AM
This revision is now accepted and ready to land.Mon, Jun 10, 10:11 AM
Closed by commit rCOMM522d00e1ed6c: [commtest] Fix flaky identity-tunnelbroker test (authored by bartek). · Explain WhyWed, Jun 12, 7:31 AM
This revision was automatically updated to reflect the committed changes.
bartek added a commit: rCOMM522d00e1ed6c: [commtest] Fix flaky identity-tunnelbroker test.

Revision Contents
Changeset List

PathSize
services/
commtest/
tests/
47 lines

Diff 41233

View Options

services/commtest/tests/identity_tunnelbroker_tests.rs

Loading...