-
Notifications
You must be signed in to change notification settings - Fork 31
[Autofill] Remove encrypted messaging from tests #2114
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
base: main
Are you sure you want to change the base?
Conversation
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.
Pull Request Overview
This PR removes encrypted messaging functionality from the AutofillEmailUserScript tests and updates them to use the newer reply handler pattern instead of the deprecated encrypted messaging approach.
Key changes:
- Removed encrypted messaging test utilities and related test methods
- Updated all test methods to use the modern
userContentController(_:didReceive:replyHandler:)
API - Added proper reply handler testing with expectation fulfillment
- Added
@available(iOS 14, macOS 11, *)
annotations to all test methods
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
AutofillEmailUserScriptTests.swift | Removed encrypted messaging tests and updated all test methods to use reply handlers with proper async testing patterns |
AutofillUserScript.swift | Changed unsupported message handling from assertion failure to proper error response via reply handler |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
||
func testWhenReceivesRefreshAliasMessageThenCallsDelegateMethod() { | ||
@available(iOS 14, macOS 11, *) | ||
func testWhenReceivesRefreshAliasMessageThenCallsDelegateMethod() async { |
Copilot
AI
Oct 6, 2025
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.
Mixing async/await with expectation-based testing is not recommended. Either use async/await throughout the test or stick with expectations. The current implementation uses both await
and waitForExpectations
which is inconsistent.
Copilot uses AI. Check for mistakes.
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.
Good point ;)
let message = MockWKScriptMessage(name: "emailHandlerRefreshAlias", body: mockBody) | ||
|
||
waitForExpectations(timeout: 1.0, handler: nil) | ||
await userScript.userContentController(userContentController, didReceive: message) { reply, error in |
Copilot
AI
Oct 6, 2025
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.
The userContentController(_:didReceive:replyHandler:)
method is not async and cannot be awaited. Remove the await
keyword from this method call.
await userScript.userContentController(userContentController, didReceive: message) { reply, error in | |
userScript.userContentController(userContentController, didReceive: message) { reply, error in |
Copilot uses AI. Check for mistakes.
…om:duckduckgo/apple-browsers into dbajpeyi/autofill-test-encrypted-messaging
…om:duckduckgo/apple-browsers into dbajpeyi/autofill-test-encrypted-messaging
Task/Issue URL: https://app.asana.com/1/137249556945/project/1205996472158114/task/1210844778150737?focus=true
Tech Design URL:
CC: @amddg44
Description
Getting rid of tests using encrypted messaging, and using
replyHandler
instead.Testing Steps
Impact and Risks
What could go wrong?
Quality Considerations
Notes to Reviewer
Internal references:
Definition of Done | Engineering Expectations | Tech Design Template