Skip to content
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

Remove obsolete maestro test and add one for the dismissal of Dax fire dialog #5898

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

anikiki
Copy link
Contributor

@anikiki anikiki commented Apr 11, 2025

Removed fire_dialog_during_onboarding.yaml test as the skip button does not exist anymore.
Added dimiss_fire_during_onboarding.yaml to specifically test the dismissal of the Dax fire dialog.

Task/Issue URL: https://app.asana.com/1/137249556945/project/1200905986587319/task/1209951369112706?focus=true

Description

Steps to test this PR

maestro test dimiss_fire_during_onboarding.yaml

NO UI changes

Copy link
Contributor

@nalcalag nalcalag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought: could we have reused the existing test for dismissing the fire dialog specifically? Feel free to ignore if you've already covered it elsewhere

@anikiki
Copy link
Contributor Author

anikiki commented Apr 11, 2025

Just a thought: could we have reused the existing test for dismissing the fire dialog specifically? Feel free to ignore if you've already covered it elsewhere

Good catch. There are many tests for dismissing Dax dialogs in onboarding_dismiss_all_dialogs.yaml but the fire one is missing. Will add it.

@anikiki anikiki changed the title Remove obsolete maestro test Remove obsolete maestro test and add one for the dismissal of Dax fire dialog Apr 11, 2025
Copy link
Contributor

@nalcalag nalcalag left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Comment on lines 24 to 28
- assertVisible:
text: ".*Remember: every time you browse with me a creepy ad loses its wings.*"
id: daxDialogDismissButton
- tapOn:
id: "com.duckduckgo.mobile.android:id/fireIconImageView"
- tapOn: "Clear All Tabs And Data"
- assertNotVisible: "You've got this!.*" No newline at end of file
id: daxDialogDismissButton
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: i believe it’s redundant to assertVisible and then tapOn the same element since the tapOn will fail if it can’t be found anyway. so it’s slightly faster to omit the first check.

I know we’ve got this pairing throughout the tests but i’ve been removing the first step where i’ve been in making changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants