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

fix ui tests broken by new onboarding and use shared setup flow #3081

Merged
merged 8 commits into from
Jul 18, 2024

Conversation

brindy
Copy link
Contributor

@brindy brindy commented Jul 15, 2024

Task/Issue URL: https://app.asana.com/0/392891325557410/1207808692245660/f
Tech Design URL:
CC:

Description:

Also: https://app.asana.com/0/392891325557410/1207809241818381/f

Fixes UI test breakage by skipping assertion for title of onboarding screen.

Steps to test this PR:

  1. Confirm E2E tests pass: https://github.com/duckduckgo/iOS/actions/runs/9974333207
  2. Confirm Sync tests pass: https://github.com/duckduckgo/iOS/actions/runs/9974343415
  3. Run the .maestro/setup_ui_tests.sh script.
  4. Run some of the UI tests using the .maestro/run_ui_tests.sh script. Should consistently pass.

@github-actions github-actions bot added the bot: not in app board Added by automation for pull requests with tasks not added to iOS App Board Asana project label Jul 15, 2024
@brindy brindy removed the bot: not in app board Added by automation for pull requests with tasks not added to iOS App Board Asana project label Jul 15, 2024
@brindy brindy marked this pull request as ready for review July 15, 2024 13:48
@@ -91,7 +91,7 @@ public protocol VariantRNG {
public class DefaultVariantManager: VariantManager {

public var currentVariant: Variant? {
let variantName = ProcessInfo.processInfo.environment["VARIANT", default: storage.variant ?? "" ]
Copy link
Contributor

Choose a reason for hiding this comment

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

Out of curiosity, how come we changed from environment variable to launch argument?
Maestro lets set you the env variable as per Maestro doc

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah, I just didn't know. This is even better. Will revert to the env version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, had a look at this.

1/ Env doesn't appear to actually work. I added some extra UI in the debug screen to check it (haven't committed that).

2/ We'd have to put it in every file since env is specific to the flow and I'm trying to have a standard 'set up' flow.

Interestingly the VARIANT argument gets changed to -VARIANT for some reason. Tested this with autoclear and that argument doesn't, but maybe because it's a bool?

Anyway, specifying an override prevents dax dialogs from showing and I think we want that because we want these tests to run against a something as close to production as possible. So I am going to change the tests to just get past the onboarding and do everything else as expected.

Once we look to merge the new dax dialogs we'll have to update the tests again.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah bummer, I saw that it was available so I suggested it. Thanks for the explanation, makes sense.

@alessandroboron
Copy link
Contributor

alessandroboron commented Jul 16, 2024

Hey Brindy:

When I tested manually I got the expected behaviour. When I run the test in some instance I got into this scenario. The launch argument variant is “ru” but the onboarding shown is the new one.

Screenshot 2024-07-16 at 1 07 52 PM

Copy link
Contributor

@alessandroboron alessandroboron left a comment

Choose a reason for hiding this comment

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

Works as expected!

@brindy brindy merged commit 4b865ea into main Jul 18, 2024
14 checks passed
@brindy brindy deleted the brindy/fix-e2e-tests branch July 18, 2024 11:44
samsymons added a commit that referenced this pull request Jul 18, 2024
# By Mariusz Śpiewak (2) and others
# Via GitHub
* main:
  Remove print (#3101)
  Update Package.resolved file (#3102)
  New Tab Page favorites section (#3083)
  Properly compare actual value of the entitlement check (#3100)
  fix ui tests broken by new onboarding and use shared setup flow (#3081)

# Conflicts:
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
samsymons added a commit that referenced this pull request Jul 19, 2024
* main:
  Bump submodules/privacy-reference-tests from `a242bf0` to `afb4f61` (#3096)
  Updates BSK to 171.2.3
  Remove print (#3101)
  Update Package.resolved file (#3102)
  New Tab Page favorites section (#3083)
  Properly compare actual value of the entitlement check (#3100)
  fix ui tests broken by new onboarding and use shared setup flow (#3081)
samsymons added a commit that referenced this pull request Jul 19, 2024
# By Christopher Brind (9) and others
# Via Chris Brind (1) and GitHub (1)
* main: (48 commits)
  Reduce VPN manager instances (#3097)
  Update BSK for Mac RMF changes (#3107)
  [DuckPlayer] 7- Open Settings (#3110)
  [DuckPlayer] 6 - Init updates and Watch on YouTube (#3066)
  New Tab Page Shortcuts section (#3104)
  Fix VPN configuration removal to stop the tunnel (#3099)
  avoid resizing webview when keyboard shows/hides (#3094)
  Add support for skipping sending usage pixels for remote messages (#3106)
  Bump submodules/privacy-reference-tests from `a242bf0` to `afb4f61` (#3096)
  Updates BSK to 171.2.3
  Remove print (#3101)
  Update Package.resolved file (#3102)
  New Tab Page favorites section (#3083)
  Properly compare actual value of the entitlement check (#3100)
  fix ui tests broken by new onboarding and use shared setup flow (#3081)
  AdHoc lane: Make proper assignment to variable (#3095)
  Expand AdHoc build workflow, add debug bookmarks screen (#3086)
  Update BSK to latest - for macOS fix to AdAttribution (#3084)
  Revert "Fix tests"
  Fix tests
  ...

# Conflicts:
#	DuckDuckGo.xcodeproj/project.pbxproj
#	DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
#	DuckDuckGo/NetworkProtectionStatusViewModel.swift
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants