Test filter subscribe order fix #18769
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Jenkins BuildsClick to see older builds (21)
|
27% of end-end tests have passedFailed tests (34)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePR:
Class TestDeepLinksOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (13)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
|
|
@vitvly thank you for the PR. Unfortunately data reliability issues still exist so most of e2e are failed. Below I describe couple of examples. 1. Contact request (CR) sender does not receive a signal about CR has been accepted by receiverSteps:
Expected result: User A receives notification about acceptance of his CR by User B. User B appears in contact list of User A. Actual result: User B has not appeared in contact list of User A |
2. Community status has not been updated from Pending to Joined for community request senderPreconditions: User A is an admin of non token gated community which does not require admin's manual approval (hereinafter 'Community') Steps:
Expected result: Community status should change to Joined for User B. Actual result: Community status remains in Pending for User B. |
3. Message reaction (emoji) is not shown for receiver in 1-1 chatPreconditions: User A and User B are messaging in 1-1 chat
Expected result: User B observes the reaction Actual result: User B received a message but does not see reaction added to the message by User A |
@cammellos @vitvly guys, I have messed up with sender's log. I have updated them in the issue. Sorry for wasting your time. |
|
Quick update: after debugging together with @chaitanyaprem the problem with peer exchange has been identified. In case of "accept contact request" message, multiple attempts to send it produced the following log: Important to node that |
|
Some notes for contact request delivery troubleshooting:
Kudos to @chaitanyaprem for insights throughout the process. |
83% of end-end tests have passedFailed tests (7)Click to expandClass TestCommunityMultipleDeviceMerged:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (40)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestDeepLinksOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Class TestGroupChatMultipleDeviceMergedNewUI:
|
@vitvly I had a doubt and had taken a look at the logs again and noticed that peer-exchange issue noticed is not the cause of message delivery failure in this case. 
Looking at contentTopic I don't know how the contentTopic mapping is done for various flows, and hence cannot debug further beyond this. |
@chaitanyaprem Pavlo updated the logs after our initial investigation, the hash you mention ( This is the correct hash: |
Ah, that makes sense then. But on retry, the hash would have changed. The new hash is The delay is due to the retry after first failure. |
|
Hey, @vitvly @chaitanyaprem! Yesterday I re-ran e2e in this PR. The results are much better then in the previous run (83% passed comparing to 27% in previous run). Could it be the case that some changes were committed on Waku side which improved the situation? There are still existing failures related data delivery issues (will post the examples with logs later today) but overall the situation looks better. |
|
@chaitanyaprem you're right. I am still wondering what are other failed attempts then - they refer to the same contentTopic |
I don't think any changes got committed on waku side in the last few days. But i am not sure any changes in status-go happened between the runs that has caused improvement. Wrt failures, still trying to figure out if there is an issue in the way we get peers from peer-exchange(which is causing this failure). |
|
Suspicion: Will investigate logs to confirm if the peerIDs are of nodes from previous tests which is why they are no longer available. |
vitvly
force-pushed
the
feat/filter-subscribes-order
branch
from
082ff22
to
d7f605f
Compare
0% of end-end tests have passedFailed tests (47)Click to expandClass TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestDeepLinksOneDevice:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
|
|
The latest e2e run has failed because of the following reason: UI gets stuck on keys saving step. TestOneToOneChatMultipleSharedDevicesNewUi_geth0.log
telegram-cloud-document-2-5354898850228226284.mp4 |
That is because the changes ins status-go seem incomplete. |
|
thanks @chaitanyaprem ! couldn't quite figure out what was the problem. Just updated here to point to your fix. |
|
thank you @vitvly @chaitanyaprem! I have triggered e2e run, let's wait the results. |
@vitvly I have manually checked the latest PR build and see that the error is still there. |
I have no idea why it is crashing, maybe something else in status-go is dependent on discovery probably. |
Trying to debug this by running status-desktop locally. |
Not sure of cause of crash but was able to run in status-desktop lightMode locally after some more changes. New status-go commit is status-im/status-go@dcb26ff |
17% of end-end tests have passedFailed tests (39)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
Class TestCommunityMultipleDeviceMerged:
Class TestDeepLinksOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (8)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
|
6% of end-end tests have passedFailed tests (44)Click to expandClass TestDeepLinksOneDevice:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePR:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityOneDeviceMerged:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (3)Click to expandClass TestCommunityOneDeviceMerged:
|
vitvly
force-pushed
the
feat/filter-subscribes-order
branch
from
8ebd844
to
b4219dc
Compare
0% of end-end tests have passedFailed tests (47)Click to expandClass TestCommunityOneDeviceMerged:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestDeepLinksOneDevice:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
|
|
Recent e2e run shows that |
This same status-go code works without crashing on my status desktop with lightMode. |
@vitvly @chaitanyaprem the interesting fact is that in the previous build #5 (the one before recent rebasing) this issue has gone. But after rebase it appeared again. So maybe some problem occurred during recent rebase? |
vitvly
force-pushed
the
feat/filter-subscribes-order
branch
from
b4219dc
to
e3e49f0
Compare
|
@pavloburykh @chaitanyaprem i think there was an issue in status-go code, related to the way discv5/peerexchange settings were set. I was able to run a local ios build after the fix. Checking e2e now. |
2% of end-end tests have passedFailed tests (46)Click to expandClass TestDeepLinksOneDevice:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestGroupChatMultipleDeviceMergedNewUI:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (1)Click to expandClass TestDeepLinksOneDevice:
|
|
Hey @vitvly! According to the latest e2e run At the same time there are still bunch of failures related to data delivery issues. Below is an example of such failure: Contact request (CR) has not been delivered to the receiverSteps:
Actual result: User B has not received CR |
This is because PeerExchange could not get any peers. Another point to note is why doesn't mobile show that message send has failed after max retries? Probably that may help user know that peer connectivity is not there. |
12% of end-end tests have passedFailed tests (41)Click to expandClass TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityOneDeviceMerged:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePRTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestActivityMultipleDevicePR:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (6)Click to expandClass TestCommunityOneDeviceMerged:
Class TestDeepLinksOneDevice:
|
12% of end-end tests have passedFailed tests (41)Click to expandClass TestActivityMultipleDevicePRTwo:
Class TestActivityCenterContactRequestMultipleDevicePR:
Class TestGroupChatMultipleDeviceMergedNewUI:
Class TestCommunityMultipleDeviceMerged:
Class TestOneToOneChatMultipleSharedDevicesNewUi:
Class TestCommunityMultipleDeviceMergedTwo:
Class TestActivityMultipleDevicePR:
Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:
Class TestCommunityOneDeviceMerged:
Expected to fail tests (1)Click to expandClass TestCommunityOneDeviceMerged:
Passed tests (6)Click to expandClass TestDeepLinksOneDevice:
Class TestCommunityOneDeviceMerged:
|
|
looks stale, feel free to reopen if needed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Tests status-im/status-go#4665