Fix End to End tests

[alessandroboron]

Task/Issue URL: https://app.asana.com/0/1206329551987282/1208028109665424/f

Description:

I’ve opened a draft PR that implements some of the recommendations highlighted in this comment to have some feedback on the approach.

In particular this PR removes the onboarding_browsing.yaml file and the need to run this nested flow from every workflow to handle the dax dialogs.

This has been replaced by using launch arguments and environment variables in setup.yaml

// In setup.yaml
arguments:
        isUITesting: true
        isOnboardingCompleted: ${ONBOARDING_COMPLETED}

With the above we can pass values from Maestro to our App and set the App in a particular state:

• isUITesting is used in the App to check if UI tests are running.
• ONBOARDING_COMPLETED is defined as an environment variable that we can pass via command line when we run the tests. e.g.

maestro cloud --apiKey ${{ secrets.MAESTRO_CLOUD_API_KEY }} -e ONBOARDING_COMPLETED=true --fail-on-timeout=true --fail-on-cancellation=true --timeout=150 --ios-version=17 --include-tags=release DerivedData/Build/Products/Debug-iphonesimulator/DuckDuckGo.app .maestro/

In this way we can easily default the existing tests not to show the onboarding passing ONBOARDING_COMPLETED=true while maintaining the flexibility to pass false for tests related to the onboarding.

We could use this approach also for experiment variant e.g.

arguments:
        …
        onboardingVariant: ${ONBOARDING_VARIANT}   

And pass the variant value via the command line.

Next Steps / Future improvements:

• Show / hide onboarding intro in UITests based on ONBOARDING_COMPLETED and remove shared/onboarding.yaml
• Improve end-to-end.yaml workflow

Steps to test this PR:

1. The latest test run has all the tests passing apart from one that timed out (06_mjs_heuristic_ad_domain_provided_but_empty_dsl_not_needed (20m 56s). This though is because of Maestro not because of the approach. When I re-run manually the test in Maestro cloud the test passed

2. Run maestro locally

Definition of Done (Internal Only):

• Does this PR satisfy our Definition of Done?

Copy Testing:

• Use of correct apostrophes in new copy, ie ’ rather than '

Orientation Testing:

• Portrait
• Landscape

Device Testing:

• iPhone SE (1st Gen)
• iPhone 8
• iPhone X
• iPhone 14 Pro
• iPad

OS Testing:

• iOS 15
• iOS 16
• iOS 17

Theme Testing:

• Light theme
• Dark theme

---

Internal references:

Software Engineering Expectations
Technical Design Template

[alessandroboron]

As explained in the PR description I inject the ENV variable ONBOARDING_COMPLETED via command to execute the tests

[alessandroboron]

This test was failing even for control group. Anya helped fixing it.

[alessandroboron]

We can probably enhance the script to add optional env variables to pass when we run it

[alessandroboron]

Temporarily left this file in the AppDelegate.

Theoretically we could make an extension on ProcessInfo e.g.

extension ProcessInfo {
    var isUITesting: Bool {
        arguments.contains(Self.isUITesting)
    }
}

Because we need to read the value of the env variable from the user defaults we cannot mock the UserDefaults in extensions if we want to unit test this behaviour.

[alessandroboron]

Because autoclear uses its own setup instead of setup.yaml we need to copy the new arguments too

[loremattei]

@alessandroboron I like this. I agree we can reuse this approach also for other things. And thing change should make UI tests faster as they won't have to "solve" onboarding on every single test.

[brindy]

@loremattei @alessandroboron

My worry is that something we do affects onboarding or visa versa and we'd never spot that. Onboarding in particular is quite hard for us to see breaking changes just because it's quite rare for us to run the app from deleted state in production. However if you're OK with that risk I don't have objections.

[loremattei]

@brindy yep, that's a good point.

I think that using @alessandroboron's approach we can add tests specifically focused on onboarding. E.g. we could add a new test target and invoke it with a separate GHA command that passes ONBOARDING_COMPLETED=false and the variant we want to test (Alessandro mentioned in the task description that this is doable with this same approach). As a starting point, we could test the current onboarding only.

[github-actions]

	Warnings
⚠️	PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by 🚫 dangerJS against 8bc9a05

[alessandroboron]

@brindy yep, that's a good point.

I think that using @alessandroboron's approach we can add tests specifically focused on onboarding. E.g. we could add a new test target and invoke it with a separate GHA command that passes ONBOARDING_COMPLETED=false and the variant we want to test (Alessandro mentioned in the task description that this is doable with this same approach). As a starting point, we could test the current onboarding only.

@loremattei , @brindy correct the intention was to also write explicit tests for the onboarding. I’ve actually updated the PR and used the same approach to inject the variant in UI Tests.

You can test it by checking out the code and run:

1. ./setup_ui_tests.sh to setup the simulator for maestro.
2. ./run_ui_tests.sh onboarding_tests to run onboarding tests.

About onboarding_tests
01_control_group_onboarding.yaml -> Tests the control group linear flow.
02_control_group_hide_onboarding.yaml -> Tests the control group hide dax dialogs.
03_experiment_group_linear_onboarding.yaml -> Tests the experiment linear flow.

Disclaimer: The tests may not be perfect, e.g. find UIElements by strings rather than accessibility identifiers. I tried to re-use what was already there to show how we can do that.

Since I think this PR will fix the tests failing on main I will convert the draft to a PR.

[brindy]

LGTM - ran all the onboarding and release tests locally. Nice improvement with the Launch options code.