PIR E2E tests GitHub actions #3528

THISISDINOSAUR · 2024-11-07T15:56:06Z

Task/Issue URL: https://app.asana.com/0/72649045549333/1208698939169508/f
Tech Design URL:
CC:

Description:
Adds the yml file to run the new PIR e2e tests. It should trigger on:

PRs against release and hotfix branches
Nightly
If the new PR checkbox is ticked (See below)

Also slips in a minor change to the already merged tests to prevent a potential crash which offered less useful error messaging than if the expectation wasn't fulfilled

Optional E2E tests:

Run PIR E2E tests
Check this to run the Personal Information Removal end to end tests. If updating CCF, or any PIR related code, tick this.

Steps to test this PR:

Check the yml that everything looks sensible
Check that the tests pass against this PR [Note: The staging backend is currently down, which the tests are reliant on, so at the moment they fail]
I've tested with a variety of setups with branches to test that trigger, but if keen you could also try making a release of hotfix branch to test against.

Definition of Done:

Does this PR satisfy our Definition of Done?

Internal references:

Pull Request Review Checklist
Software Engineering Expectations
Technical Design Template
Pull Request Documentation

.github/workflows/pir_end_to_end_tests.yml

DBPE2ETests/DBPEndToEndTests.swift

...n/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionSubscriptionManaging.swift

loremattei · 2024-11-08T14:24:52Z

.github/workflows/pir_end_to_end_tests.yml

+      with:
+        repository: DuckDuckGo/pir-fake-broker
+        ssh-key: ${{ secrets.SSH_PRIVATE_KEY_PIR_FAKE_BROKER }}
+        ref: loremattei/ci-integration


😅 This was a mostly a test... and I see it's now an outdated branch, so we should probably merge or rewrite and point to main.
I'll follow up in the fake broker repo to get this sorted out.

Ah thanks for flagging, I completely forgot about this

This reverts commit e5cbc75.

This reverts commit 6b369d1.

THISISDINOSAUR · 2024-11-08T18:20:52Z

Staging is now back up and I've managed to sort the new token, so the tests pass again 🎉

loremattei · 2024-11-15T17:13:35Z

.github/workflows/pir_end_to_end_tests.yml

+jobs:
+  pir-e2e-tests:
+    name: PIR e2e tests
+    runs-on: ${{ matrix.runner }}


Are you doing this because we plan to run on multiple versions later?

Yeah I left it in since I figured we might, do you think we should?

It may make sense to test across different platforms, but it's probably a good idea to make sure tests run reliably on the current version before adding more.

loremattei · 2024-11-15T17:14:29Z

.github/workflows/pir_end_to_end_tests.yml

+      matrix:
+        runner: [macos-14-xlarge]
+        include:
+          - xcode-version: "15.4"


I think we are changing this to 16, so we should check what we have in main when we merge.

loremattei · 2024-11-15T17:21:23Z

@THISISDINOSAUR I left a couple of comments, but this looks good to me.

One thing that's unclear to me, though: the tests are shown green as if they passed successfully, but if I look at the test report it should one error (one test failed).

Am I missing/misunderstanding the report, or there's some issues with the reporting?

THISISDINOSAUR · 2024-11-15T18:03:15Z

Am I missing/misunderstanding the report, or there's some issues with the reporting?

Hmm, definitely something is off. It's currently set to retry tests on failure, and looks like it reports overall as green if the retry works, but then still reports the initial failure.
In terms of the GitHub action, that seems like okay behavior to me?

The fact that it's failing in the first place is more troubling.
It's possible that it's simply exceeding the timeout since macOS seems to have a lackadaisical attitude towards the NSBackgroundActivityScheduler values we set (in which case we should increase the timeout since we already set very low values there for tests ).

However, the fact that the behavior seems consistent that it only works the second time makes me think it's something to do with how we tell what values to set there. We had to use user defaults to let the login item know it's currently running integration tests, so I'm wondering if it's something there. I'll experiment and report back (assuming staging is back up again...)

ayoy · 2024-11-15T19:48:47Z

.github/workflows/pir_end_to_end_tests.yml

+      if: always() 
+      with:
+        check_name: "Test Report ${{ matrix.runner }}"
+        report_paths: pir-e2e-tests.xml


Tests report publisher action will show failures if you have tests that only passed on retry. The initial failure is reported in the JUnit XML file and recognized as failure by this reporter action.

To handle this, add an extra parameter below this line:

check_retries: true

Thanks! Although I think the current behavior (besides the fact that it fails in the first place) might be more desirable so we know about those initial failures

Thanks! This makes sense to me. I didn't notice the retry.
👍

THISISDINOSAUR · 2024-11-18T15:25:24Z

Made a follow up task to see what's going on with the initial failure: https://app.asana.com/0/1203581873609357/1208788768876699/f

* main: Bump version to 1.115.0 (310) Update skip-release check Update fastlane plugin to 0.11.6 Bump ddg-apple-automation version for bugfix (#3565) PIR E2E tests GitHub actions (#3528) Update autoconsent to v11.5.0 (#3561) Bump version to 1.115.0 (309) Set marketing version to 1.115.0 Update embedded files Bump plugin-ddg_apple_automation (#3564) Migrate release flow components to FastLane (#3563) VPN auth token logic update (#3562)

…g-error # By Anka (5) and others # Via GitHub (5) and Dominik Kapusta (1) * main: (28 commits) Add attemptCount and maxAttempts to broker config (#3533) Hide continue setup cards after 1 week (#3471) Add expectation when checking email text field value (#3572) [macos] adding support for message bridge (#3558) Update PIR test runner (#3570) Add support for controlling sections visibility on HTML New Tab Page (#3551) Bump version to 1.115.0 (311) macOS 13/14 UI test compilation fix (#3569) macOS 13/14 UI test compilation fix (#3569) Fix bug where bookmarks bar prompt does not hide bar (#3553) Add OS version to download fail pixel (#3568) Remove get_tasks_in_last_internal_release (#3566) Bump version to 1.115.0 (310) Update skip-release check Update fastlane plugin to 0.11.6 Bump ddg-apple-automation version for bugfix (#3565) PIR E2E tests GitHub actions (#3528) Update autoconsent to v11.5.0 (#3561) Bump version to 1.115.0 (309) Set marketing version to 1.115.0 ... # Conflicts: # DuckDuckGo.xcodeproj/project.pbxproj # DuckDuckGo.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved

THISISDINOSAUR added 3 commits November 7, 2024 15:55

Add PIR E2E tests yml file

d93b089

Update PR template to have pir tests checkbox

4cc4386

Remove possibility of array index out of bounds exception

625853f

THISISDINOSAUR closed this Nov 7, 2024

THISISDINOSAUR reopened this Nov 7, 2024

THISISDINOSAUR added 3 commits November 7, 2024 17:57

Lower timeout

2fd5048

Lower timeout for testing

d577c0f

Reenable continue after failure

2788927

THISISDINOSAUR requested review from ayoy and loremattei November 7, 2024 19:44

THISISDINOSAUR commented Nov 7, 2024

View reviewed changes

.github/workflows/pir_end_to_end_tests.yml Outdated Show resolved Hide resolved

Add token for debugging

e5cbc75

THISISDINOSAUR commented Nov 8, 2024

View reviewed changes

DBPE2ETests/DBPEndToEndTests.swift Outdated Show resolved Hide resolved

THISISDINOSAUR commented Nov 8, 2024

View reviewed changes

...n/Sources/DataBrokerProtection/Authentication/DataBrokerProtectionSubscriptionManaging.swift Outdated Show resolved Hide resolved

loremattei reviewed Nov 8, 2024

View reviewed changes

THISISDINOSAUR added 8 commits November 8, 2024 14:41

Revert "Add token for debugging"

4e4dc6e

This reverts commit e5cbc75.

Return token regardless to help with debugging

6b369d1

Reduce timeout to make debugging easier

4efbecb

Try adding launchctl setenv

73ef08f

Restore longer timeout

0aeacdb

Revert "Return token regardless to help with debugging"

5961498

This reverts commit 6b369d1.

Change continue after failure back to false

76f2459

Change timeout back to 40 mins

dfbb67d

Change fake broker to point at main

98967bd

loremattei reviewed Nov 15, 2024

View reviewed changes

ayoy reviewed Nov 15, 2024

View reviewed changes

loremattei self-requested a review November 18, 2024 15:14

loremattei approved these changes Nov 18, 2024

View reviewed changes

THISISDINOSAUR merged commit cbd1a1e into main Nov 18, 2024
24 checks passed

THISISDINOSAUR deleted the elle/pir-e2e-tests-github-actions branch November 18, 2024 15:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

PIR E2E tests GitHub actions #3528

PIR E2E tests GitHub actions #3528

THISISDINOSAUR commented Nov 7, 2024 •

edited

Loading

loremattei Nov 8, 2024

THISISDINOSAUR Nov 8, 2024

THISISDINOSAUR commented Nov 8, 2024

loremattei Nov 15, 2024

THISISDINOSAUR Nov 15, 2024

loremattei Nov 18, 2024

loremattei Nov 15, 2024

loremattei commented Nov 15, 2024

THISISDINOSAUR commented Nov 15, 2024 •

edited

Loading

ayoy Nov 15, 2024

THISISDINOSAUR Nov 15, 2024

loremattei Nov 18, 2024

THISISDINOSAUR commented Nov 18, 2024

PIR E2E tests GitHub actions #3528

PIR E2E tests GitHub actions #3528

Conversation

THISISDINOSAUR commented Nov 7, 2024 • edited Loading

Internal references:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

THISISDINOSAUR commented Nov 8, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loremattei commented Nov 15, 2024

THISISDINOSAUR commented Nov 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

THISISDINOSAUR commented Nov 18, 2024

THISISDINOSAUR commented Nov 7, 2024 •

edited

Loading

THISISDINOSAUR commented Nov 15, 2024 •

edited

Loading