Skip to content

Conversation

@ih-codes
Copy link
Collaborator

@ih-codes ih-codes commented Dec 1, 2025

📜 Tickets

Jira ticket
Github issue

💡 Description

⚠️ Temporarily set the merge destination to my other PR just so I can check that Bitrise passes ⚠️

  • Fix setUp/tearDown warnings in the XCUITests suite by using the async throws version of the methods
  • Fix main actor isolation warnings for some local state with @MainActor

See the Swift 6 Migration channel (#tmp-swift-6-migration) for details on how to migrate the setUp and tearDown methods.

Batch 1: #31025
Batch 2: #31027

cc @Cramsden @lmarceau @dataports

📝 Checklist

  • I filled in the ticket numbers and a description of my work
  • I updated the PR name to follow our PR naming guidelines
  • I ensured unit tests pass and wrote tests for new code
  • If working on UI, I checked and implemented accessibility (Dynamic Text and VoiceOver)
  • If adding telemetry, I read the data stewardship requirements and will request a data review
  • If adding or modifying strings, I read the guidelines and will request a string review from l10n
  • If needed, I updated documentation and added comments to complex code

@ih-codes ih-codes requested a review from clarmso December 1, 2025 20:41
@ih-codes ih-codes marked this pull request as ready for review December 1, 2025 20:58
@ih-codes ih-codes requested a review from a team as a code owner December 1, 2025 20:58
Base automatically changed from ih/FXIOS-12796-XCUITests-batch-2 to main December 5, 2025 21:09
@mobiletest-ci-bot
Copy link

mobiletest-ci-bot commented Dec 5, 2025

Warnings
⚠️ Print() function seems to be used in file firefox-ios/firefox-ios-tests/Tests/XCUITests/DownloadsTests.swift at line 21. Please remove this usage from production code or use BrowserKit Logger. Since bypass label danger-bypass detected we are reporting as warning only for this PR.

💪 Quality guardian

9 tests files modified. You're a champion of test coverage! 🚀

🥇 Perfect PR size

Smaller PRs are easier to review. Thanks for making life easy for reviewers! ✨

🙌 Friday high-five

Thanks for pushing us across the finish line this week! 🙌

💬 Description craftsman

Great PR description! Reviewers salute you 🫡

✅ Per-file coverage

All changed files meet the threshold of 35.0%.

Generated by 🚫 Danger Swift against c1504f3

@ih-codes
Copy link
Collaborator Author

ih-codes commented Dec 5, 2025

Hey @lmarceau how can we get Danger not to fail the PR for this work?

Error 1

🚫 SwiftUI 'Text(""' in file firefox-ios/firefox-ios-tests/Tests/XCUITests/C_AddressesTests.swift at line 683 needs to be avoided, use Strings.swift localization instead.

For the second error it's a UI test and I also don't see any Text() objects on line 683 of the XCUITests/C_AddressesTests.swift file...
Screenshot 2025-12-05 at 3 56 45 PM

Error 2

🚫 Print() function seems to be used in file firefox-ios/firefox-ios-tests/Tests/XCUITests/DownloadsTests.swift at line 21. Please remove this usage from production code or use BrowserKit Logger.

For this one I think a print should be permissible in the UI tests?
Screenshot 2025-12-05 at 3 54 43 PM

@clarmso
Copy link
Collaborator

clarmso commented Dec 5, 2025

🚫 SwiftUI 'Text(""' in file firefox-ios/firefox-ios-tests/Tests/XCUITests/C_AddressesTests.swift at line 683 needs to be avoided, use Strings.swift localization instead.

Yes, I have encountered the same type of error from Danger, too. A Text("....") resolves the issue.

@ih-codes
Copy link
Collaborator Author

ih-codes commented Dec 5, 2025

🚫 SwiftUI 'Text(""' in file firefox-ios/firefox-ios-tests/Tests/XCUITests/C_AddressesTests.swift at line 683 needs to be avoided, use Strings.swift localization instead.

Yes, I have encountered the same type of error from Danger, too. A Text("....") resolves the issue.

Do you know @clarmso which SwiftUI Text(...) it is even complaining about in that file? I don't think I saw one... 😅

@lmarceau
Copy link
Contributor

lmarceau commented Dec 8, 2025

Opened a PR to fix the Danger detection in #31159

@mergify
Copy link
Contributor

mergify bot commented Dec 8, 2025

This pull request has conflicts when rebasing. Could you fix it @ih-codes? 🙏

@ih-codes ih-codes force-pushed the ih/FXIOS-12796-XCUITests-batch-3 branch from e94539e to 25cbbe3 Compare December 10, 2025 22:12
@ih-codes
Copy link
Collaborator Author

@lmarceau is there a way we can ignore danger for the print statement in the tests? I didn't really want to actually change any of the UI test code I was touching for this 👀

@lmarceau
Copy link
Contributor

@lmarceau is there a way we can ignore danger for the print statement in the tests? I didn't really want to actually change any of the UI test code I was touching for this 👀

Yeah let me add an option for that, Ill do that shortly to unblock this

@ih-codes
Copy link
Collaborator Author

@Mergifyio rebase

@mergify
Copy link
Contributor

mergify bot commented Dec 12, 2025

rebase

✅ Branch has been successfully rebased

@ih-codes ih-codes force-pushed the ih/FXIOS-12796-XCUITests-batch-3 branch from 25cbbe3 to c1504f3 Compare December 12, 2025 16:04
@ih-codes ih-codes merged commit bc96baf into main Dec 12, 2025
8 checks passed
@ih-codes ih-codes deleted the ih/FXIOS-12796-XCUITests-batch-3 branch December 12, 2025 17:07
@github-actions
Copy link
Contributor

🚀 PR merged to main, targeting version: 147.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants