Move automatic status updates timeout to status-go #13602
Conversation
Jenkins BuildsClick to see older builds (31)
|
Parveshdhull
force-pushed
the
move-status-updates-timeout-to-status-go
branch
2 times, most recently
from
c7f8f61
to
f6511f6
Compare
Parveshdhull
force-pushed
the
move-status-updates-timeout-to-status-go
branch
2 times, most recently
from
3016754
to
fc1558b
Compare
Parveshdhull
changed the title
Parveshdhull
force-pushed
the
move-status-updates-timeout-to-status-go
branch
from
fc1558b
to
cc754c3
Compare
90% of end-end tests have passedFailed tests (9)Click to expandClass TestCommandsMultipleDevicesMerged:
Class TestPairingSyncMultipleDevicesMerged:
Class TestGroupChatMultipleDeviceMerged:
Passed tests (78)Click to expandSingle device tests:
Class TestOnboardingOneDeviceMerged:
Class TestKeycardTxOneDeviceMerged:
Class TestEnsStickersMultipleDevicesMerged:
Class TestWalletManagementDeviceMerged:
Class TestOneToOneChatMultipleSharedDevices:
Class TestPublicChatMultipleDeviceMerged:
Class TestPairingSyncMultipleDevicesMerged:
Class TestContactBlockMigrateKeycardMultipleSharedDevices:
Class TestRestoreOneDeviceMerged:
Class TestPublicChatBrowserOneDeviceMerged:
Class TestSendTxDeviceMerged:
|
95% of end-end tests have passedFailed tests (4)Click to expandClass TestPublicChatBrowserOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevices:
Class TestPairingSyncMultipleDevicesMerged:
Passed tests (83)Click to expandClass TestOneToOneChatMultipleSharedDevices:
Class TestRestoreOneDeviceMerged:
Class TestPublicChatBrowserOneDeviceMerged:
Class TestEnsStickersMultipleDevicesMerged:
Class TestGroupChatMultipleDeviceMerged:
Class TestPublicChatMultipleDeviceMerged:
Class TestPairingSyncMultipleDevicesMerged:
Class TestCommandsMultipleDevicesMerged:
Class TestSendTxDeviceMerged:
Class TestOnboardingOneDeviceMerged:
Class TestWalletManagementDeviceMerged:
Class TestKeycardTxOneDeviceMerged:
Class TestContactBlockMigrateKeycardMultipleSharedDevices:
|
Parveshdhull
force-pushed
the
move-status-updates-timeout-to-status-go
branch
2 times, most recently
from
81cb124
to
feb1dd0
Compare
| @@ -75,4 +75,5 @@ | |||
| "wallet" (ethereum.subscriptions/new-wallet-event cofx (js->clj event-js :keywordize-keys true)) | |||
| "local-notifications" (local-notifications/process cofx (js->clj event-js :keywordize-keys true)) | |||
| "community.found" (link.preview/cache-community-preview-data (js->clj event-js :keywordize-keys true)) | |||
| "status.updates.timedout" (visibility-status-updates/handle-visibility-status-updates cofx (js->clj event-js :keywordize-keys true)) | |||
There was a problem hiding this comment.
typo ? or intentional ? timeDout
There was a problem hiding this comment.
Hi, Thank you for reviewing PR.
yes, intentional. Status-go is signaling for timed out status updates.
100% of end-end tests have passedPassed tests (87)Click to expandClass TestKeycardTxOneDeviceMerged:
Class TestGroupChatMultipleDeviceMerged:
Class TestOnboardingOneDeviceMerged:
Class TestPairingSyncMultipleDevicesMerged:
Class TestPublicChatMultipleDeviceMerged:
Class TestContactBlockMigrateKeycardMultipleSharedDevices:
Class TestPublicChatBrowserOneDeviceMerged:
Class TestSendTxDeviceMerged:
Class TestEnsStickersMultipleDevicesMerged:
Class TestRestoreOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevices:
Class TestWalletManagementDeviceMerged:
Class TestCommandsMultipleDevicesMerged:
|
|
@Parveshdhull thank you for PR! It works great, except for one issue that I, unfortunately, can't find the exact steps to reproduce, but which I've stumbled upon a couple of times during testing. I don't think it's a blocker for PR, so just want to let you know about it. The problem is this: Suspected steps:
Expected result: D1 is offline Also, I suspect that it can be related to changing the mode of online status updates, i.e. changing the mode from "automatic" to "always online" and back, and under this condition, the problem was reproduced. But still not sure as can't reproduce it in this way. Summarizing the above: the issue is not a blocker for PR, since it's not reproducible constantly and the online status updates after relogin, so it can be merged. Tested:
|
Parveshdhull
force-pushed
the
move-status-updates-timeout-to-status-go
branch
3 times, most recently
from
ffaa855
to
b67ae44
Compare
|
Hi @qoqobolo, Thank you very much for testing the PR. After several tries and analyzing several scenarios, I found the issue. Reproduction Steps:
So, the issue was if on the app start device didn't find any status update to expire, it will again check after 5 minutes and will expire new status updates according to that time. The issue is fixed now. This issue was really interesting, must have been very hard to find 🙏. |
Parveshdhull
force-pushed
the
move-status-updates-timeout-to-status-go
branch
from
b67ae44
to
7fbf996
Compare
Aaahh I should have waited for 5 seconds and close device 2 again while device 1 is closed too! |
|
pr status: waiting for status-go testing |
Parveshdhull
force-pushed
the
move-status-updates-timeout-to-status-go
branch
from
7fbf996
to
c554342
Compare
fixes: #13601
status-go PR: status-im/status-go#2757
Testing Request:
Please check if status updates, (especially automatic status updates) are working as expected and nothing is broken.
status: waiting for status-go testing