Skip to content

Refactor FXIOS-12055 #26253 ⁃ Add logic to Danger to fail PR that add files without test #27258

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
Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Refactor FXIOS-12055 #26253 ⁃ Add logic to Danger to fail PR that add files without test #27258

wants to merge 19 commits into from
+37 −15

Conversation

yoanarios
Copy link
Contributor

@yoanarios yoanarios commented Jun 11, 2025 •
edited
Loading

📜 Tickets

Jira ticket
Github issue

💡 Description

Fix danger messaging

📝 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 needed, I updated documentation and added comments to complex code
  • If needed, I added a backport comment (example @Mergifyio backport release/v120)

@mobiletest-ci-bot
Copy link

mobiletest-ci-bot commented Jun 11, 2025 •
edited
Loading

Warnings
⚠️

New file DangerRuleCheck.swift is missing from coverage report. Please add unit tests. (cc: @cyndichin @yoanarios).
⚠️

New file SecondDangerCheck.swift has 0% test coverage. Please add unit tests. (cc: @cyndichin @yoanarios).
Messages
📖 Project coverage: 36.33%
📖 Edited 2 files
📖 Created 2 files

Client.app: Coverage: 35.51

File Coverage
SecondDangerCheck.swift 0.0% ⚠️

Generated by 🚫 Danger Swift against 69616c5

@yoanarios yoanarios force-pushed the yrd/FXIOS-12055-fix-danger-message branch from 9aeefab to 25b2963 Compare June 12, 2025 18:22
@yoanarios yoanarios closed this Jun 12, 2025
@yoanarios yoanarios reopened this Jun 12, 2025
@yoanarios yoanarios closed this Jun 12, 2025
@yoanarios yoanarios reopened this Jun 12, 2025
@yoanarios yoanarios force-pushed the yrd/FXIOS-12055-fix-danger-message branch from 25b2963 to 2f3ea2a Compare June 13, 2025 13:36
yoanarios added 19 commits June 16, 2025 09:47
@yoanarios
Fix messaging to receive warning
5f42e42
@yoanarios
Add empty file
e4ee1db
@yoanarios
More danger test
1a5544f
@yoanarios
fix variable
7a4a0da
@yoanarios
Rename class because Test is filter out + revert webengine test
403a71c
@yoanarios
Another test with new warning to help debug
ac891ec
@yoanarios
debug
f6d54f4
@yoanarios
Add 2 new files to check if warns works for mutliple files
b788924
@yoanarios
seems to be the matching logic that fails
2c17874
@yoanarios
Refactor matching logic to match filename only
d61b2e2
@yoanarios
Simplify logic and extra warning to check json is working
ea1d3ef
@yoanarios
Send a single warning with the number of files without coverage
55fffed
@yoanarios
Comment bitrise UItest step to focus on danger step
8911ff9
@yoanarios
Testing another version
028f521
@yoanarios
revert bitrise change
b5ca8f3
@yoanarios
Add contact and update to warning
2a8399e
@yoanarios
Clean up warnings
cd0478d
@yoanarios
test if file appears now in coverage.json
f8ba0ac
@yoanarios
Fix match check, coverage.json only has filename and extension
69616c5
@yoanarios yoanarios force-pushed the yrd/FXIOS-12055-fix-danger-message branch from 39e351b to 69616c5 Compare June 16, 2025 14:17
@yoanarios yoanarios marked this pull request as ready for review June 16, 2025 14:18
@yoanarios yoanarios requested a review from a team as a code owner June 16, 2025 14:18
@yoanarios yoanarios requested a review from thatswinnie June 16, 2025 14:18
@yoanarios
Copy link
Contributor Author

Managed to test locally and I've fixed the remaining issues where files with coverage were not found, it's still possible for files not to be included in XCTResult if they don't contain lines to be tested so we keeping the 2 warnings, 1 if is found with 0 coverage and another one if is not found meaning doesn't have test either

@yoanarios
Copy link
Contributor Author

Screenshot 2025-06-16 at 10 20 42 AM

@yoanarios yoanarios requested review from cyndichin and removed request for thatswinnie June 16, 2025 14:21
@yoanarios
Copy link
Contributor Author

@yoanarios test to get mention notification

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

@cyndichin cyndichin Awaiting requested review from cyndichin

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@yoanarios @mobiletest-ci-bot