Skip to content

Refactor FXIOS-13748 [Telemetry] Migrate FxAWebViewTelemetry to use mock telemetry #30245

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 4 commits into from
Nov 6, 2025
Merged

Refactor FXIOS-13748 [Telemetry] Migrate FxAWebViewTelemetry to use mock telemetry #30245

merged 4 commits into from
Nov 6, 2025

Conversation

@vee-kamunya
Copy link
Contributor

@vee-kamunya vee-kamunya commented Oct 29, 2025 •
edited by lmarceau
Loading

📜 Tickets

Jira ticket
Github issue

💡 Description

  • Add dependency injection for TelemetryWrapperProtocol in FxAWebViewTelemetry
  • Mock TelemetryWrapper in SyncTelemetryTests to avoid calling Glean during tests
  • Add 7 new test methods to verify recordTelemetry() behavior

📝 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

@vee-kamunya
Bugfix FXIOS-13685 [Bookmarks] Fix navigation bar partial display in …
b20d6b4 
…New Folder screen on landscape
@vee-kamunya
Refactor FXIOS-13748 [Telemetry] Use TelemetryWrapperProtocol in
093b7f7 
  FxAWebViewTelemetry

  - Add dependency injection for TelemetryWrapperProtocol in FxAWebViewTelemetry
  - Mock TelemetryWrapper in SyncTelemetryTests to avoid calling Glean during tests
  - Add 7 new test methods to verify recordTelemetry() behavior
@vee-kamunya vee-kamunya requested a review from a team as a code owner October 29, 2025 17:22
lmarceau
lmarceau reviewed Oct 29, 2025
View reviewed changes
Copy link
Contributor

@lmarceau lmarceau left a comment

Choose a reason for hiding this comment

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

Some nits, but looks good to me 😄 great work

@vee-kamunya
Copy link
Contributor Author

vee-kamunya commented Oct 30, 2025

removed the spaces as well from @lmarceau 😃 @mattreaganmozilla

@lmarceau
Copy link
Contributor

lmarceau commented Oct 30, 2025

@vee-kamunya FxALoginRegistrationTelemetryTests is failing no the CI pipeline, can you investigate?

@vee-kamunya
Copy link
Contributor Author

vee-kamunya commented Oct 31, 2025

I modified the Xcode project file (firefox-ios/Client.xcodeproj/project.pbxproj) to include the existing MockTelemetryWrapper.swift file in the FxA telemetry test target's build sources. It should fix it @lmarceau

@vee-kamunya
Copy link
Contributor Author

vee-kamunya commented Nov 5, 2025

Hello @mattreaganmozilla, could you please review this? Thanks!!

@mobiletest-ci-bot
Copy link

mobiletest-ci-bot commented Nov 6, 2025

Messages
📖 Project coverage: 37.72%

💪 Quality guardian

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

🧹 Tidy commit

Just 3 file(s) touched. Thanks for keeping it clean and review-friendly!

✅ Per-file coverage

All changed files meet the threshold of 35.0%.

Client.app: Coverage: 37.08

File Coverage
FxAWebViewTelemetry.swift 100.0% ✅

Generated by 🚫 Danger Swift against 47404d0

@lmarceau
Copy link
Contributor

lmarceau commented Nov 6, 2025

Sorry @vee-kamunya, I should have ran the build and dropped the ball here. All is good ✅ I am merging your work, thank you very much!

@lmarceau lmarceau merged commit 320596d into mozilla-mobile:main Nov 6, 2025
9 checks passed
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2025

🚀 PR merged to main, targeting version: 145.1

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

Reviewers

@lmarceau lmarceau lmarceau approved these changes

@mattreaganmozilla mattreaganmozilla Awaiting requested review from mattreaganmozilla mattreaganmozilla is a code owner automatically assigned from mozilla-mobile/fxios-eng

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@vee-kamunya @lmarceau @mobiletest-ci-bot