Skip to content

Conversation

dbajpeyi
Copy link
Collaborator

@dbajpeyi dbajpeyi commented Oct 6, 2025

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

@dbajpeyi dbajpeyi requested a review from Copilot October 6, 2025 15:26
Copy link
Contributor

@Copilot Copilot AI left a 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 {
Copy link

Copilot AI Oct 6, 2025

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.

Copy link
Collaborator Author

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
Copy link

Copilot AI Oct 6, 2025

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.

Suggested change
await userScript.userContentController(userContentController, didReceive: message) { reply, error in
userScript.userContentController(userContentController, didReceive: message) { reply, error in

Copilot uses AI. Check for mistakes.

@dbajpeyi dbajpeyi changed the title Dbajpeyi/autofill test encrypted messaging [Autofill] Remove encrypted messaging from tests Oct 7, 2025
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