-
Notifications
You must be signed in to change notification settings - Fork 45
test: add allure test steps and removed global clientUserManager singleton pattern(WPB-22314) #4519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Ups 🫰🟨This PR is too big. Please try to break it up into smaller PRs. |
yamilmedina
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
left some comments, looking good
| @Suppress("CyclomaticComplexMethod", "LongMethod") | ||
| @TestCaseId("TC-8603") | ||
| @Category("criticalFlow") | ||
| @Category("criticalFlow", "testTest") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this "testTest" needed ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for spotting this, I only used the "testTest" for testing
| @Suppress("CyclomaticComplexMethod", "LongMethod") | ||
| @TestCaseId("TC-8605") | ||
| @Category("criticalFlow") | ||
| @Category("criticalFlow", "testTest") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here, "testTest" does not seem descriptive. maybe leaked into the final PR ?
| // fun waitUntilLoginFlowIsCompleted(): RegistrationPage { | ||
| // val device = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()) | ||
| // waitUntilElementGone(device, loginButtonGoneSelector, timeoutMillis = 12_000) | ||
| // waitUntilElementGone(device, settingUpWireGoneSelector, timeoutMillis = 35_000) | ||
| // return this | ||
| // } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // fun waitUntilLoginFlowIsCompleted(): RegistrationPage { | |
| // val device = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()) | |
| // waitUntilElementGone(device, loginButtonGoneSelector, timeoutMillis = 12_000) | |
| // waitUntilElementGone(device, settingUpWireGoneSelector, timeoutMillis = 35_000) | |
| // return this | |
| // } | |
| // fun waitUntilLoginFlowIsCompleted(): RegistrationPage { | |
| // val device = UiDevice.getInstance(InstrumentationRegistry.getInstrumentation()) | |
| // waitUntilElementGone(device, loginButtonGoneSelector, timeoutMillis = 12_000) | |
| // waitUntilElementGone(device, settingUpWireGoneSelector, timeoutMillis = 35_000) | |
| // return this | |
| // } | |
| import uiautomatorutils.UiWaitUtils.WaitUtils.waitFor | ||
|
|
||
| @RunWith(AndroidJUnit4::class) | ||
| class FileSharing : BaseUiTest() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggestion/question: I'm wondering if this test class should be renamed to something like: FileSharingBetweenTeams. Since it looks to me it's what we are actually doing in here ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
CinnamonNinja
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! Left some minor suggestions.
| } | ||
| } | ||
|
|
||
| step("Fetch email verification link from Inbucket") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it good to mention inbucket in open source code? Can we maybe just reduce it to "Fetch email verification link"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have removed the inbucket
| iMinimiseOngoingCall() | ||
| } | ||
| } | ||
| step("Send chat message 'Hello Friends' from user2") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this name need to contain the message and user? Or can it be more generic?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
| false | ||
| ) | ||
| } | ||
| step("Share location from user3 and verify call continues") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also here, do we need to mention the user specifically? Or can it be more generic?
| assertTermsOfUseModalVisible() // Asserts all elements | ||
| clickContinueButton() | ||
| // These values are pulled from BuildConfig injected from secrets.json) | ||
| step("Fetch OTP from Inbucket to complete 2FA verification and complete registration") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also here, can we remove Inbucket and instead just use "Fetch OTP to complete 2FA verification and complete registration"?
|



PR Submission Checklist for internal contributors
The PR Title
SQPIT-764The PR Description
What's new in this PR?
lateinitwhere they are always initialized insetUp()!!null assertionsprivateto prevent accidental external accessIssues
Briefly describe the issue you have solved or implemented with this pull request. If the PR contains multiple issues, use a bullet list.
lateinitwhere they are always initialized insetUp()!!null assertionsprivateto prevent accidental external accessCauses (Optional)
Briefly describe the causes behind the issues. This could be helpful to understand the adopted solutions behind some nasty bugs or complex issues.
Solutions
Briefly describe the solutions you have implemented for the issues explained above.
Dependencies (Optional)
If there are some other pull requests related to this one (e.g. new releases of frameworks), specify them here.
Needs releases with:
Testing
Test Coverage (Optional)
How to Test
Briefly describe how this change was tested and if applicable the exact steps taken to verify that it works as expected.
Notes (Optional)
Specify here any other facts that you think are important for this issue.
Attachments (Optional)
Attachments like images, videos, etc. (drag and drop in the text box)
PR Post Submission Checklist for internal contributors (Optional)
PR Post Merge Checklist for internal contributors
References
feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.