status-mobile icon indicating copy to clipboard operation
status-mobile copied to clipboard

Published 20 hours ago • verified publisher icon status-im

chore: qualify screen keywords

Open J-Son89 opened this issue 1 year ago • 11 comments

This pr is more of a proposal at this point, so please leave your thoughts whether you are happy with this change or not and if it's an approach we wish to take.

I only made these changes for the wallet as it is safer to make this refactor in multiple stages and I wanted to be sure the team is happy with making this adjustment.

The change here is simple, to qualify screen keywords with :screen/ and then further qualify the ns with ., e,g :screen/wallet.some-screen or :screen/chat.some-other-screen

Let me know your thoughts 🧠 ✏️

J-Son89 avatar Feb 09 '24 15:02 J-Son89

Jenkins Builds

Click to see older builds (48)
:grey_question: Commit :hash: Finished (UTC) Duration Platform Result
:heavy_check_mark: 88aaff79 #1 2024-02-09 15:42:42 ~6 min ios :iphone:ipa :calling:
:heavy_check_mark: 88aaff79 #1 2024-02-09 15:42:49 ~6 min tests :page_facing_up:log
:heavy_check_mark: 41bb2975 #2 2024-02-09 15:51:23 ~5 min ios :iphone:ipa :calling:
:heavy_check_mark: 41bb2975 #2 2024-02-09 15:51:36 ~5 min tests :page_facing_up:log
:heavy_check_mark: 41bb2975 #2 2024-02-09 15:53:57 ~8 min android-e2e :robot:apk :calling:
:heavy_check_mark: 41bb2975 #2 2024-02-09 15:54:22 ~8 min android :robot:apk :calling:
:x: adf573a8 #3 2024-02-09 16:26:53 ~1 min tests :page_facing_up:log
:heavy_check_mark: adf573a8 #3 2024-02-09 16:32:31 ~7 min android-e2e :robot:apk :calling:
:heavy_check_mark: adf573a8 #3 2024-02-09 16:32:33 ~7 min android :robot:apk :calling:
:heavy_check_mark: adf573a8 #3 2024-02-09 16:33:37 ~8 min ios :iphone:ipa :calling:
:x: ca89d1a5 #5 2024-02-09 18:04:44 ~1 min tests :page_facing_up:log
:heavy_check_mark: ca89d1a5 #5 2024-02-09 18:08:48 ~5 min ios :iphone:ipa :calling:
:heavy_check_mark: ca89d1a5 #5 2024-02-09 18:09:27 ~6 min android :robot:apk :calling:
:heavy_check_mark: ca89d1a5 #5 2024-02-09 18:09:46 ~6 min android-e2e :robot:apk :calling:
:heavy_check_mark: 4a12e4fa #6 2024-02-14 14:19:32 ~5 min ios :iphone:ipa :calling:
:heavy_check_mark: 4a12e4fa #6 2024-02-14 14:19:33 ~5 min tests :page_facing_up:log
:heavy_check_mark: 4a12e4fa #6 2024-02-14 14:22:18 ~8 min android-e2e :robot:apk :calling:
:heavy_check_mark: 4a12e4fa #6 2024-02-14 14:22:25 ~8 min android :robot:apk :calling:
:heavy_check_mark: eb176e80 #7 2024-02-16 14:39:01 ~5 min tests :page_facing_up:log
:heavy_check_mark: eb176e80 #7 2024-02-16 14:39:27 ~5 min ios :iphone:ipa :calling:
:heavy_check_mark: eb176e80 #7 2024-02-16 14:41:24 ~7 min android :robot:apk :calling:
:heavy_check_mark: eb176e80 #7 2024-02-16 14:41:31 ~7 min android-e2e :robot:apk :calling:
:x: 7a8f23c4 #8 2024-02-26 22:39:11 ~1 min tests :page_facing_up:log
:heavy_check_mark: 7a8f23c4 #8 2024-02-26 22:44:50 ~7 min android-e2e :robot:apk :calling:
:heavy_check_mark: 7a8f23c4 #8 2024-02-26 22:44:56 ~7 min android :robot:apk :calling:
:heavy_check_mark: 7a8f23c4 #8 2024-02-26 22:46:05 ~8 min ios :iphone:ipa :calling:
:heavy_check_mark: 3976e189 #9 2024-02-27 15:48:10 ~33 min android-e2e :robot:apk :calling:
:heavy_check_mark: 3976e189 #9 2024-02-27 15:48:16 ~33 min android :robot:apk :calling:
:heavy_check_mark: 3976e189 #9 2024-02-27 15:49:50 ~34 min tests :page_facing_up:log
:heavy_check_mark: 3976e189 #10 2024-02-27 17:17:32 ~1 hr 22 min ios :iphone:ipa :calling:
:heavy_check_mark: 6cdb0125 #10 2024-02-29 18:31:28 ~7 min tests :page_facing_up:log
:heavy_check_mark: 6cdb0125 #10 2024-02-29 18:32:38 ~8 min android-e2e :robot:apk :calling:
:heavy_check_mark: 6cdb0125 #10 2024-02-29 18:32:42 ~8 min android :robot:apk :calling:
:heavy_check_mark: 6cdb0125 #11 2024-02-29 18:34:03 ~9 min ios :iphone:ipa :calling:
:heavy_check_mark: ec168017 #11 2024-03-05 10:35:08 ~6 min tests :page_facing_up:log
:heavy_check_mark: ec168017 #11 2024-03-05 10:35:47 ~7 min android-e2e :robot:apk :calling:
:heavy_check_mark: ec168017 #12 2024-03-05 10:36:07 ~7 min ios :iphone:ipa :calling:
:heavy_check_mark: ec168017 #11 2024-03-05 10:36:19 ~8 min android :robot:apk :calling:
:heavy_check_mark: ce2308fe #12 2024-03-07 11:14:36 ~9 min android-e2e :robot:apk :calling:
:heavy_check_mark: ce2308fe #12 2024-03-07 11:16:05 ~10 min tests :page_facing_up:log
:heavy_check_mark: ce2308fe #12 2024-03-07 11:17:23 ~12 min android :robot:apk :calling:
:heavy_check_mark: ce2308fe #13 2024-03-07 11:17:38 ~12 min ios :iphone:ipa :calling:
:heavy_check_mark: 9cdbc044 #13 2024-03-07 22:07:47 ~7 min tests :page_facing_up:log
:heavy_check_mark: 9cdbc044 #13 2024-03-07 22:08:04 ~7 min android-e2e :robot:apk :calling:
:heavy_check_mark: 3e269f24 #14 2024-03-07 22:14:33 ~5 min tests :page_facing_up:log
:heavy_check_mark: 3e269f24 #14 2024-03-07 22:15:23 ~6 min android :robot:apk :calling:
:heavy_check_mark: 3e269f24 #14 2024-03-07 22:16:18 ~7 min android-e2e :robot:apk :calling:
:heavy_check_mark: 3e269f24 #15 2024-03-07 22:16:26 ~7 min ios :iphone:ipa :calling:
:grey_question: Commit :hash: Finished (UTC) Duration Platform Result
:heavy_check_mark: 0a81e449 #15 2024-03-12 08:30:26 ~5 min tests :page_facing_up:log
:heavy_check_mark: 0a81e449 #15 2024-03-12 08:32:03 ~7 min android-e2e :robot:apk :calling:
:heavy_check_mark: 0a81e449 #15 2024-03-12 08:32:06 ~7 min android :robot:apk :calling:
:heavy_check_mark: 0a81e449 #16 2024-03-12 08:33:19 ~8 min ios :iphone:ipa :calling:
:heavy_check_mark: d952ed5f #16 2024-03-12 10:52:42 ~6 min tests :page_facing_up:log
:heavy_check_mark: d952ed5f #16 2024-03-12 10:53:28 ~7 min android-e2e :robot:apk :calling:
:heavy_check_mark: d952ed5f #16 2024-03-12 10:54:08 ~7 min android :robot:apk :calling:
:heavy_check_mark: d952ed5f #17 2024-03-12 10:54:08 ~7 min ios :iphone:ipa :calling:

status-im-auto avatar Feb 09 '24 15:02 status-im-auto

:heavy_check_mark: status-mobile/prs/android/PR-18778#1 :small_blue_diamond: ~9 min 1 sec :small_blue_diamond: 88aaff79 :small_blue_diamond: :package: android package

status-im-auto avatar Feb 09 '24 15:02 status-im-auto

Appreciate you raising this point and for your work in improving the flow.

While I see the intention behind adding the :screen keyword for clarity, I'm wondering if it might be a bit redundant, considering the context of navigation. Typically, when we navigate, it's already implied that we're moving to a screen.

However, I'm open to exploring scenarios where this extra qualification could be helpful or prevent any confusion

mohsen-ghafouri avatar Feb 09 '24 16:02 mohsen-ghafouri

Appreciate you raising this point and for your work in improving the flow.

While I see the intention behind adding the :screen keyword for clarity, I'm wondering if it might be a bit redundant, considering the context of navigation. Typically, when we navigate, it's already implied that we're moving to a screen.

However, I'm open to exploring scenarios where this extra qualification could be helpful or prevent any confusion

yeah when using a keyword we are pretty much always aware of the use case and so that is not what I am trying to improve here. (well in my experience so far) The most valuable reason I see for this is that when doing later refactors of screen keywords (or any other keywords) that we can safely update the keywords without affecting other unintended domains/ aspects of the codebase. Without qualification we have to look quite precisely at what each keywords purpose is and that is both tedious and error prone.

There might be further use cases (or drawbacks) @ulisesmac, @ilmotta or our other more experienced Clojurists might be aware of.

J-Son89 avatar Feb 09 '24 16:02 J-Son89

The most valuable reason I see for this is that when doing later refactors of screen keywords (or any other keywords) that we can safely update the keywords without affecting other unintended domains/ aspects of the codebase. Without qualification we have to look quite precisely at what each keywords purpose is and that is both tedious and error prone.

I don't understand this. If we want a refactor related to screens, we can easily look up for contexts, or do a project with search for navigate.

Not able to understand what value this brings (maybe I'm wrong re: refactoring? please correct me if that's the case).

I do see a downside. Having arbitrary keywords will pollute the codebase. I know with :screen, your intent is very clear. But there could be a case where navigation opens a bottom-sheet. It will be very enticing to have a :bottom-sheet keyword in that case. There is no end to this.

shivekkhurana avatar Feb 12 '24 15:02 shivekkhurana

Appreciate you raising this point and for your work in improving the flow. While I see the intention behind adding the :screen keyword for clarity, I'm wondering if it might be a bit redundant, considering the context of navigation. Typically, when we navigate, it's already implied that we're moving to a screen. However, I'm open to exploring scenarios where this extra qualification could be helpful or prevent any confusion

yeah when using a keyword we are pretty much always aware of the use case and so that is not what I am trying to improve here. (well in my experience so far) The most valuable reason I see for this is that when doing later refactors of screen keywords (or any other keywords) that we can safely update the keywords without affecting other unintended domains/ aspects of the codebase. Without qualification we have to look quite precisely at what each keywords purpose is and that is both tedious and error prone.

There might be further use cases (or drawbacks) @ulisesmac, @ilmotta or our other more experienced Clojurists might be aware of.

@mohsen-ghafouri Some other aspects improved are the code organization and the dev experience while writing keywords. It's less error-prone to write because the editor suggest the keys:

Currently with icons: image

Currently with translations: image

ulisesmac avatar Feb 12 '24 18:02 ulisesmac

Appreciate you raising this point and for your work in improving the flow. While I see the intention behind adding the :screen keyword for clarity, I'm wondering if it might be a bit redundant, considering the context of navigation. Typically, when we navigate, it's already implied that we're moving to a screen. However, I'm open to exploring scenarios where this extra qualification could be helpful or prevent any confusion

yeah when using a keyword we are pretty much always aware of the use case and so that is not what I am trying to improve here. (well in my experience so far) The most valuable reason I see for this is that when doing later refactors of screen keywords (or any other keywords) that we can safely update the keywords without affecting other unintended domains/ aspects of the codebase. Without qualification we have to look quite precisely at what each keywords purpose is and that is both tedious and error prone. There might be further use cases (or drawbacks) @ulisesmac, @ilmotta or our other more experienced Clojurists might be aware of.

@mohsen-ghafouri Some other aspects improved are the code organization and the dev experience while writing keywords. It's less error-prone to write because the editor suggest the keys:

Currently with icons: image

Currently with translations: image

If we use :i/ for icons and :t/ for translations, maybe use :s/ instead of :screen/?

briansztamfater avatar Feb 14 '24 17:02 briansztamfater

Appreciate you raising this point and for your work in improving the flow. While I see the intention behind adding the :screen keyword for clarity, I'm wondering if it might be a bit redundant, considering the context of navigation. Typically, when we navigate, it's already implied that we're moving to a screen. However, I'm open to exploring scenarios where this extra qualification could be helpful or prevent any confusion

yeah when using a keyword we are pretty much always aware of the use case and so that is not what I am trying to improve here. (well in my experience so far) The most valuable reason I see for this is that when doing later refactors of screen keywords (or any other keywords) that we can safely update the keywords without affecting other unintended domains/ aspects of the codebase. Without qualification we have to look quite precisely at what each keywords purpose is and that is both tedious and error prone. There might be further use cases (or drawbacks) @ulisesmac, @ilmotta or our other more experienced Clojurists might be aware of.

@mohsen-ghafouri Some other aspects improved are the code organization and the dev experience while writing keywords. It's less error-prone to write because the editor suggest the keys: Currently with icons: image Currently with translations: image

If we use :i/ for icons and :t/ for translations, maybe use :s/ instead of :screen/?

We do have some other keywords that are more verbose too. for example :social/, :token/. I think the decision we were basing it off for when we want to use the abbreviated form vs more verbose is how frequently we use these. right @ilmotta? 🤔 and I guess something else to consider is there any other qualified keyword that would benefit more from having :s/ instead? 🤔 - although once it place it would be a very quick refactor to adjust this most likely

J-Son89 avatar Feb 15 '24 12:02 J-Son89

I don't understand this. If we want a refactor related to screens, we can easily look up for contexts, or do a project with search for navigate.

Not able to understand what value this brings (maybe I'm wrong re: refactoring? please correct me if that's the case).

I do see a downside. Having arbitrary keywords will pollute the codebase. I know with :screen, your intent is very clear. But there could be a case where navigation opens a bottom-sheet. It will be very enticing to have a :bottom-sheet keyword in that case. There is no end to this.

We have many qualified keywords like this used for different cases, e.g icons are using :/i, translations are using :/t. it's helps categorise certain aspects of the codebase.

In the case of translation keywords the intent is obvious because it's basically always using an i18n/label. however, consider for example if we are refactoring and we want to change all uses of the :password translation, it is clearly much easier and safer to find the relevant keywords using :t/password then a generic search for :password and take the context from it's usage - this in turn simplifies the pr review process too.

We have for example some generic screen names here, such as :shell - "Shell" is a concept we use all throughout the designs to suggest the overlay pages that are dark theme, regardless of the users theme. Some other very generic screen names :settings, :new-contact, :airdrop-addresses, :shell-stack - there are many more (and I'm sure more will be added), all which can be used in several other contexts within our project.

Of course, this can go further as shown in your bottom-sheet example, but I think in this case we need specificity of what :screen/ qualification is used for. In this case, I think it can be simply any screen we have registered in the screens.cljs file. Nothing outside of that. If something else should be used as :screen/ and is outside the screens.cljs file then it should be moved there imo 👍 Whether a qualified keyword is then needed for bottom-sheets is another question and so I guess with qualified keywords we can take them on a case by case basis as we are already doing this a lot in the codebase 👌

That's just my view on this anyway, @ulisesmac also pointed out some good points too 👍

J-Son89 avatar Feb 16 '24 11:02 J-Son89

@J-Son89, I didn't read all the comments in this PR, but I am absolutely in favor of qualifying certain keywords to make them unique in the system. I lost count of the amount of times I had to find the view definition of a screen, but the keyword had too many hits while grepping (e.g. :photo-selector, settings, intro, and many others).

ilmotta avatar Feb 16 '24 17:02 ilmotta

@J-Son89, I didn't read all the comments in this PR, but I am absolutely in favor of qualifying certain keywords to make them unique in the system. I lost count of the amount of times I had to find the view definition of a screen, but the keyword had too many hits while grepping (e.g. :photo-selector, settings, intro, and many others).

@ilmotta - I believe it is currently just @shivekkhurana with some concerns here. https://github.com/status-im/status-mobile/pull/18778#issuecomment-1938880350

I have responded - https://github.com/status-im/status-mobile/pull/18778#issuecomment-1948207052

and @ulisesmac contributed some insights on why he is in preference of this approach - https://github.com/status-im/status-mobile/pull/18778#issuecomment-1939277284

@shivekkhurana have your concerns been somewhat satisfied?

J-Son89 avatar Feb 19 '24 10:02 J-Son89

I think the decision we were basing it off for when we want to use the abbreviated form vs more verbose is how frequently we use these. right @ilmotta? 🤔

Correct @J-Son89, we decided on the short form of :i for :icon based on frequency, and also because [quo/icon :icon/foo] felt too redundant.

I'm not a fan of abbreviating :screen to :s, but that would work fore sure.

But here's my take on this subject.

These discussions about non-namespaced keyword names are more difficult as well because we are creating categories (qualifications) for things that already have a proper place. With namespaced keywords, we still get keyword auto-complete, jump to source (depending on your editor and settings), and we can alias the namespace as short as we desire (such as s). Ah, we also get compile-time protection against typos in the namespace part of a keyword like ::screens/settings. Namespaced keywords shouldn't always be used though. But if the problem is to uniquely refer to some keyword and differentiate them in the whole system, it's really handy.

It feels a bit like we're skipping this very core feature of Clojure.

One of the reasons I so wanted to upgrade shadow-cljs in PR https://github.com/status-im/status-mobile/pull/15417 was to get the :as-alias feature from Clojure 1.11+, which we can use now. as-alias eliminates the problem of circular dependencies to refer to keywords.

The code below does not even compile pre Clojure 1.11 if you use :as (circular dependency error), but I checked this in our repo with :as-alias and it works beautifully.

;; Anywhere we want to refer to a screen name:
(require '[status-im.navigation.screens :as-alias screens])

;; Then on that same namespace (note the double colon):
(rf/dispatch [:open-modal ::screens/settings])

;; And in the screens namespace `status-im.navigation.screens` (note the double colon)
{:name      ::settings
 :options   options/transparent-screen-options
 :component settings/view}

Clojure gives us a way to differentiate values based on their context. Currently, the keyword :settings can be a screen, but also a list type in the quo component settings.category. We can refer to different keywords (semantically speaking) by using their namespaces.

Edit: I approved the PR. Above, I just shared my personal best choice and tried to explain why.

ilmotta avatar Feb 19 '24 22:02 ilmotta

92% of end-end tests have passed

Total executed tests: 48
Failed tests: 3
Expected to fail tests: 1
Passed tests: 44

IDs of failed tests: 702809,703194,703629 

IDs of expected to fail tests: 703503

Failed tests (3)

Click to expand
  • Rerun failed tests

    • Class TestCommunityMultipleDeviceMerged:

    1. test_community_several_images_send_reply, id: 703194

    Device 2: Tap on found: SendMessageButton
    Device 1: Looking for a message by text: reply to gallery 
    critical/chats/test_public_chat_browsing.py:428: in test_community_several_images_send_reply
    self.errors.verify_no_errors()
base_test_case.py:191: in verify_no_errors
    pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
 Reply message was not received by the sender



    Device sessions

    Class TestCommunityMultipleDeviceMergedTwo:

    1. test_community_markdown_support, id: 702809

    Device 1: Looking for a message by text: quote reply (one row)
    Device 2: Looking for a message by text: quote reply (one row) 
    critical/chats/test_public_chat_browsing.py:980: in test_community_markdown_support
    self.errors.verify_no_errors()
base_test_case.py:191: in verify_no_errors
    pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
 quote reply (one row) is not displayed with markdown in 1-1 chat for the recipient (device 2)



    Device sessions

    2. test_community_join_when_node_owner_offline, id: 703629

    Device 2: Looking for community: 'open community'
    Device 2: Click until Text by accessibility id: community-description-text will be presented 
    critical/chats/test_public_chat_browsing.py:1178: in test_community_join_when_node_owner_offline
    self.errors.verify_no_errors()
base_test_case.py:191: in verify_no_errors
    pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
 Text "You joined “closed community”" in shown toast element doesn't match expected "You joined “open community”"



    Device sessions

    Expected to fail tests (1)

    Click to expand

    Class TestCommunityOneDeviceMerged:

    1. test_community_discovery, id: 703503 
    Test is not run, e2e blocker

    [[reason: [NOTRUN] Curated communities not loading, https://github.com/status-im/status-mobile/issues/17852]]

    Passed tests (44)

    Click to expand

    Class TestActivityMultipleDevicePR:

    1. test_navigation_jump_to, id: 702936
    Device sessions

    2. test_activity_center_reply_read_unread_delete_filter_swipe, id: 702947
    Device sessions

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_one_image_send_reply, id: 702859
    Device sessions

    2. test_community_emoji_send_copy_paste_reply, id: 702840
    Device sessions

    3. test_community_mark_all_messages_as_read, id: 703086
    Device sessions

    4. test_community_contact_block_unblock_offline, id: 702894
    Device sessions

    5. test_community_edit_delete_message_when_offline, id: 704615
    Device sessions

    6. test_community_message_delete, id: 702839
    Device sessions

    7. test_community_message_send_check_timestamps_sender_username, id: 702838
    Device sessions

    8. test_community_links_with_previews_github_youtube_twitter_gif_send_enable, id: 702844
    Device sessions

    9. test_community_message_edit, id: 702843
    Device sessions

    10. test_community_unread_messages_badge, id: 702841
    Device sessions

    Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:

    1. test_1_1_chat_delete_via_long_press_relogin, id: 702784
    Device sessions

    2. test_1_1_chat_is_shown_message_sent_delivered_from_offline, id: 702783
    Device sessions

    3. test_1_1_chat_mute_chat, id: 703496
    Device sessions

    Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. test_1_1_chat_emoji_send_reply_and_open_link, id: 702782
    Device sessions

    2. test_1_1_chat_text_message_delete_push_disappear, id: 702733
    Device sessions

    3. test_1_1_chat_push_emoji, id: 702813
    Device sessions

    4. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
    Device sessions

    5. test_1_1_chat_edit_message, id: 702855
    Device sessions

    6. test_1_1_chat_send_image_save_and_share, id: 703391
    Device sessions

    7. test_1_1_chat_pin_messages, id: 702731
    Device sessions

    8. test_1_1_chat_message_reaction, id: 702730
    Device sessions

    Class TestActivityMultipleDevicePRTwo:

    1. test_activity_center_mentions, id: 702957
    Device sessions

    2. test_activity_center_admin_notification_accept_swipe, id: 702958
    Device sessions

    Class TestCommunityMultipleDeviceMergedTwo:

    1. test_community_hashtag_links_to_community_channels, id: 702948
    Device sessions

    2. test_community_mentions_push_notification, id: 702786
    Device sessions

    3. test_community_leave, id: 702845
    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133
    Device sessions

    2. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    3. test_community_undo_delete_message, id: 702869
    Device sessions

    4. test_community_navigate_to_channel_when_relaunch, id: 702846
    Device sessions

    5. test_community_mute_community_and_channel, id: 703382
    Device sessions

    Class TestDeepLinksOneDevice:

    1. test_links_open_universal_links_from_chat, id: 704613
    Device sessions

    2. test_links_deep_links, id: 702775
    Device sessions

    Class TestActivityCenterContactRequestMultipleDevicePR:

    1. test_add_contact_field_validation, id: 702777
    Device sessions

    2. test_activity_center_contact_request_accept_swipe_mark_all_as_read, id: 702851
    Device sessions

    3. test_activity_center_contact_request_decline, id: 702850
    Device sessions

    Class TestGroupChatMultipleDeviceMergedNewUI:

    1. test_group_chat_pin_messages, id: 702732
    Device sessions

    2. test_group_chat_mute_chat, id: 703495
    Device sessions

    3. test_group_chat_send_image_save_and_share, id: 703297
    Device sessions

    4. test_group_chat_reactions, id: 703202
    Device sessions

    5. test_group_chat_join_send_text_messages_push, id: 702807
    Device sessions

    6. test_group_chat_offline_pn, id: 702808
    Device sessions

    status-im-auto avatar Feb 29 '24 21:02 status-im-auto

    88% of end-end tests have passed

    Total executed tests: 48
Failed tests: 5
Expected to fail tests: 1
Passed tests: 42

    IDs of failed tests: 702948,702859,702869,702777,704615 

    IDs of expected to fail tests: 703503

    Failed tests (5)

    Click to expand
  • Rerun failed tests

    • Class TestCommunityMultipleDeviceMergedTwo:

    1. test_community_hashtag_links_to_community_channels, id: 702948

    Device 2: Click until `ChatMessageInput` by `accessibility id`: `chat-message-input` will be presented
    Device 2: Looking for a message by text: #cats 
    critical/chats/test_public_chat_browsing.py:1063: in test_community_hashtag_links_to_community_channels
    self.channel_2.chat_element_by_text(message_with_hashtag).click_on_link_inside_message_body()
../views/chat_view.py:193: in click_on_link_inside_message_body
    self.message_body.wait_for_visibility_of_element(30)
../views/base_element.py:147: in wait_for_visibility_of_element
    raise TimeoutException(
 Device 2: Text by xpath:`//android.view.ViewGroup[@content-desc='chat-item']//android.widget.TextView[contains(@text,'#cats')]` is not found on the screen after wait_for_visibility_of_element



    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_community_undo_delete_message, id: 702869

    Device 1: Tap on found: Button
    Device 1: Find Button by xpath: //*[@text="Undo"] 
    critical/chats/test_public_chat_browsing.py:112: in test_community_undo_delete_message
    self.channel.element_by_text("Undo").click()
../views/base_element.py:90: in click
    element = self.find_element()
../views/base_element.py:79: in find_element
    raise NoSuchElementException(
 Device 1: Button by xpath: `//*[@text="Undo"]` is not found on the screen; For documentation on this error, please visit: https://www.selenium.dev/documentation/webdriver/troubleshooting/errors#no-such-element-exception



    Device sessions

    Class TestActivityCenterContactRequestMultipleDevicePR:

    1. test_add_contact_field_validation, id: 702777

    Device 2: Tap on found: LogInButton
    ## Signed in successfully!
    activity_center/test_activity_center.py:169: in test_add_contact_field_validation
    self.loop.run_until_complete(run_in_parallel(((_device_1_creates_user, {}),
/usr/lib/python3.10/asyncio/base_events.py:649: in run_until_complete
    return future.result()
__init__.py:44: in run_in_parallel
    returns.append(await k)
/usr/lib/python3.10/concurrent/futures/thread.py:58: in run
    result = self.fn(*self.args, **self.kwargs)
activity_center/test_activity_center.py:159: in _device_1_creates_user
    self.profile_1.driver.reset()
/usr/local/lib/python3.10/dist-packages/appium/webdriver/extensions/applications.py:299: in reset
    self.execute(Command.RESET)
/home/jenkins/.local/lib/python3.10/site-packages/selenium/webdriver/remote/webdriver.py:345: in execute
    self.error_handler.check_response(response)
/usr/local/lib/python3.10/dist-packages/appium/webdriver/errorhandler.py:122: in check_response
    raise exception_class(msg=message, stacktrace=format_stacktrace(stacktrace))
 An unknown server-side error occurred while processing the command. Original error: Cannot start the 'im.status.ethereum.pr' application. Consider checking the driver's troubleshooting documentation. Original error: Error executing adbExec. Original error: 'Command '/home/chef/android-sdk-linux/platform-tools/adb -P 5037 -s emulator-5554 shell am start -W -n im.status.ethereum.pr/im.status.ethereum.MainActivity -S' timed out after 20000ms'. Try to increase the 20000ms adb execution timeout represented by 'adbExecTimeout' capability
E   Stacktrace:
E   UnknownError: An unknown server-side error occurred while processing the command. Original error: Cannot start the 'im.status.ethereum.pr' application. Consider checking the driver's troubleshooting documentation. Original error: Error executing adbExec. Original error: 'Command '/home/chef/android-sdk-linux/platform-tools/adb -P 5037 -s emulator-5554 shell am start -W -n im.status.ethereum.pr/im.status.ethereum.MainActivity -S' timed out after 20000ms'. Try to increase the 20000ms adb execution timeout represented by 'adbExecTimeout' capability
E       at getResponseForW3CError (/mnt/sauce/appium/appium-v2.0.0/packages/base-driver/lib/protocol/errors.js:1073:9)
E       at asyncHandler (/mnt/sauce/appium/appium-v2.0.0/packages/base-driver/lib/protocol/protocol.js:491:57)



    Device sessions

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_one_image_send_reply, id: 702859

    Device 2: Find EmojisNumber by xpath: //*[starts-with(@text,'reply to image')]/ancestor::android.view.ViewGroup[@content-desc='chat-item']/../..//*[@content-desc='emoji-reaction-1']/android.widget.TextView[2]
    Device 2: Element EmojisNumber text is equal to 1 
    critical/chats/test_public_chat_browsing.py:501: in test_community_one_image_send_reply
    self.errors.verify_no_errors()
base_test_case.py:191: in verify_no_errors
    pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
 Message about saving a photo is not shown.



    Device sessions

    2. test_community_edit_delete_message_when_offline, id: 704615

    Device 1: Looking for a message by text: text after edit
    Device 1: Looking for a message by text: message to delete 
    critical/chats/test_public_chat_browsing.py:797: in test_community_edit_delete_message_when_offline
    self.errors.verify_no_errors()
base_test_case.py:191: in verify_no_errors
    pytest.fail('\n '.join([self.errors.pop(0) for _ in range(len(self.errors))]))
 Updated message 'text after edit' is not delivered to the receiver



    Device sessions

    Expected to fail tests (1)

    Click to expand

    Class TestCommunityOneDeviceMerged:

    1. test_community_discovery, id: 703503 
    Test is not run, e2e blocker

    [[reason: [NOTRUN] Curated communities not loading, https://github.com/status-im/status-mobile/issues/17852]]

    Passed tests (42)

    Click to expand

    Class TestActivityMultipleDevicePRTwo:

    1. test_activity_center_admin_notification_accept_swipe, id: 702958
    Device sessions

    2. test_activity_center_mentions, id: 702957
    Device sessions

    Class TestCommunityMultipleDeviceMergedTwo:

    1. test_community_mentions_push_notification, id: 702786
    Device sessions

    2. test_community_join_when_node_owner_offline, id: 703629
    Device sessions

    3. test_community_markdown_support, id: 702809
    Device sessions

    4. test_community_leave, id: 702845
    Device sessions

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_mark_all_messages_as_read, id: 703086
    Device sessions

    2. test_community_message_send_check_timestamps_sender_username, id: 702838
    Device sessions

    3. test_community_message_edit, id: 702843
    Device sessions

    4. test_community_several_images_send_reply, id: 703194
    Device sessions

    5. test_community_links_with_previews_github_youtube_twitter_gif_send_enable, id: 702844
    Device sessions

    6. test_community_message_delete, id: 702839
    Device sessions

    7. test_community_contact_block_unblock_offline, id: 702894
    Device sessions

    8. test_community_unread_messages_badge, id: 702841
    Device sessions

    9. test_community_emoji_send_copy_paste_reply, id: 702840
    Device sessions

    Class TestOneToOneChatMultipleSharedDevicesNewUi:

    1. test_1_1_chat_emoji_send_reply_and_open_link, id: 702782
    Device sessions

    2. test_1_1_chat_non_latin_messages_stack_update_profile_photo, id: 702745
    Device sessions

    3. test_1_1_chat_push_emoji, id: 702813
    Device sessions

    4. test_1_1_chat_message_reaction, id: 702730
    Device sessions

    5. test_1_1_chat_text_message_delete_push_disappear, id: 702733
    Device sessions

    6. test_1_1_chat_edit_message, id: 702855
    Device sessions

    7. test_1_1_chat_pin_messages, id: 702731
    Device sessions

    8. test_1_1_chat_send_image_save_and_share, id: 703391
    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_community_copy_and_paste_message_in_chat_input, id: 702742
    Device sessions

    2. test_community_mute_community_and_channel, id: 703382
    Device sessions

    3. test_community_navigate_to_channel_when_relaunch, id: 702846
    Device sessions

    4. test_restore_multiaccount_with_waku_backup_remove_switch, id: 703133
    Device sessions

    Class TestOneToOneChatMultipleSharedDevicesNewUiTwo:

    1. test_1_1_chat_mute_chat, id: 703496
    Device sessions

    2. test_1_1_chat_is_shown_message_sent_delivered_from_offline, id: 702783
    Device sessions

    3. test_1_1_chat_delete_via_long_press_relogin, id: 702784
    Device sessions

    Class TestActivityMultipleDevicePR:

    1. test_navigation_jump_to, id: 702936
    Device sessions

    2. test_activity_center_reply_read_unread_delete_filter_swipe, id: 702947
    Device sessions

    Class TestGroupChatMultipleDeviceMergedNewUI:

    1. test_group_chat_join_send_text_messages_push, id: 702807
    Device sessions

    2. test_group_chat_mute_chat, id: 703495
    Device sessions

    3. test_group_chat_pin_messages, id: 702732
    Device sessions

    4. test_group_chat_reactions, id: 703202
    Device sessions

    5. test_group_chat_offline_pn, id: 702808
    Device sessions

    6. test_group_chat_send_image_save_and_share, id: 703297
    Device sessions

    Class TestDeepLinksOneDevice:

    1. test_links_deep_links, id: 702775
    Device sessions

    2. test_links_open_universal_links_from_chat, id: 704613
    Device sessions

    Class TestActivityCenterContactRequestMultipleDevicePR:

    1. test_activity_center_contact_request_decline, id: 702850
    Device sessions

    2. test_activity_center_contact_request_accept_swipe_mark_all_as_read, id: 702851
    Device sessions

    status-im-auto avatar Mar 07 '24 13:03 status-im-auto

    100% of end-end tests have passed

    Total executed tests: 5
Failed tests: 0
Expected to fail tests: 0
Passed tests: 5

    Passed tests (5)

    Click to expand

    Class TestActivityCenterContactRequestMultipleDevicePR:

    1. test_add_contact_field_validation, id: 702777
    Device sessions

    Class TestCommunityOneDeviceMerged:

    1. test_community_undo_delete_message, id: 702869
    Device sessions

    Class TestCommunityMultipleDeviceMergedTwo:

    1. test_community_hashtag_links_to_community_channels, id: 702948
    Device sessions

    Class TestCommunityMultipleDeviceMerged:

    1. test_community_edit_delete_message_when_offline, id: 704615
    Device sessions

    2. test_community_one_image_send_reply, id: 702859
    Device sessions

    status-im-auto avatar Mar 07 '24 13:03 status-im-auto

    @J-Son89 thanks for the PR! Take a look at the issues please:

    ISSUE 1: Cannot set property 'reagent-compiler2' of null when opening a collectible

    STR:

    1. Open Collectibles
    2. Press any collectible

    https://github.com/status-im/status-mobile/assets/67952253/effba0b7-b558-4152-ae13-3aed39b082e4

    qoqobolo avatar Mar 07 '24 18:03 qoqobolo

    ISSUE 2: Enter amount page issues

    There are two issues on this page:

    1. Routs/fees are not shown on the screen. But it seems this is something different from https://github.com/status-im/status-mobile/issues/19099, in the video below you can see that this is just a blank page under the amount entry field
    2. You cannot go back from this page. When you press the back button, the token icon disappears, as well as the Max amount below

    PR:

    https://github.com/status-im/status-mobile/assets/67952253/6c9ce84b-e6e7-4948-8894-46711a2b6df6

    Nightly:

    https://github.com/status-im/status-mobile/assets/67952253/2b5d8247-f499-481a-a5e0-0011659e2760

    STR:

    1. Go to the Send flow
    2. Enter a valid amount, wait for routs and fees
    3. Tap the back button

    qoqobolo avatar Mar 07 '24 18:03 qoqobolo

    @qoqobolo - Apologies about that, I think it was a rebasing issue that they snook in. I addressed all those issues now as they were straightforward problems to fix with the info you gave me 🙏

    J-Son89 avatar Mar 07 '24 22:03 J-Son89

    @qoqobolo - Apologies about that, I think it was a rebasing issue that they snook in. I addressed all those issues now as they were straightforward problems to fix with the info you gave me 🙏

    No worries, the main thing is that we didn’t let them sneak into develop! 🙂 And thanks for the fix! Could you rebase the branch and resolve the conflict, please? I'll re-check it

    qoqobolo avatar Mar 11 '24 09:03 qoqobolo

    Thanks @J-Son89! LGTM PR can be merged.

    qoqobolo avatar Mar 12 '24 10:03 qoqobolo