Skip to content
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

test: Add TestSample and Maestro Mobile #4421

Merged
merged 22 commits into from
Oct 15, 2024
Merged

test: Add TestSample and Maestro Mobile #4421

merged 22 commits into from
Oct 15, 2024

Conversation

krystofwoldrich
Copy link
Member

@krystofwoldrich krystofwoldrich commented Oct 9, 2024

📜 Description

This PR adds new test app with minimal Sentry config for capturing errors and minimal implementation to UI test crash recovery and corrupted envelope recovery.

This PR also add a new CI job which runs the meantioned tests using Maestro Mobile.

💡 Motivation and Context

💚 How did you test it?

CI, https://github.com/getsentry/sentry-cocoa/actions/runs/11274454945/job/31370707526?pr=4421

📝 Checklist

You have to check all boxes before merging:

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

#skip-changelog

Copy link

codecov bot commented Oct 9, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.331%. Comparing base (5a6e387) to head (40c3f12).
Report is 6 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@              Coverage Diff              @@
##              main     #4421       +/-   ##
=============================================
+ Coverage   91.319%   91.331%   +0.011%     
=============================================
  Files          610       610               
  Lines        49755     49926      +171     
  Branches     17928     18040      +112     
=============================================
+ Hits         45436     45598      +162     
- Misses        4227      4236        +9     
  Partials        92        92               

see 18 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5a6e387...40c3f12. Read the comment docs.

Copy link

github-actions bot commented Oct 9, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1230.55 ms 1247.50 ms 16.95 ms
Size 21.90 KiB 707.35 KiB 685.45 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
4bad5f9 1237.20 ms 1250.51 ms 13.31 ms
728804f 1235.26 ms 1254.20 ms 18.94 ms
861d361 1227.90 ms 1231.45 ms 3.55 ms
add8416 1270.72 ms 1300.38 ms 29.66 ms
af1f4dd 1225.39 ms 1245.48 ms 20.09 ms
ed49f0c 1245.67 ms 1261.15 ms 15.48 ms
7bc3c0d 1259.74 ms 1268.45 ms 8.71 ms
02a972c 1207.08 ms 1221.78 ms 14.70 ms
c59914b 1342.08 ms 1375.73 ms 33.65 ms
6943de0 1235.98 ms 1246.88 ms 10.90 ms

App size

Revision Plain With Sentry Diff
4bad5f9 21.58 KiB 678.20 KiB 656.61 KiB
728804f 22.85 KiB 411.76 KiB 388.91 KiB
861d361 20.76 KiB 435.65 KiB 414.89 KiB
add8416 21.58 KiB 541.91 KiB 520.33 KiB
af1f4dd 22.85 KiB 414.71 KiB 391.86 KiB
ed49f0c 21.58 KiB 632.13 KiB 610.55 KiB
7bc3c0d 20.76 KiB 427.35 KiB 406.59 KiB
02a972c 22.85 KiB 413.42 KiB 390.57 KiB
c59914b 21.58 KiB 671.90 KiB 650.32 KiB
6943de0 20.76 KiB 393.33 KiB 372.57 KiB

Previous results on branch: kw-maestro

Startup times

Revision Plain With Sentry Diff
79295e9 1218.27 ms 1236.49 ms 18.22 ms
edd1109 1243.51 ms 1263.44 ms 19.93 ms
32f7936 1225.55 ms 1232.20 ms 6.65 ms
2be8264 1217.89 ms 1253.24 ms 35.35 ms
700ce73 1218.73 ms 1239.21 ms 20.47 ms
951bf1c 1229.24 ms 1249.71 ms 20.46 ms

App size

Revision Plain With Sentry Diff
79295e9 21.90 KiB 705.30 KiB 683.40 KiB
edd1109 21.90 KiB 705.31 KiB 683.41 KiB
32f7936 21.90 KiB 705.30 KiB 683.40 KiB
2be8264 21.90 KiB 706.05 KiB 684.16 KiB
700ce73 21.90 KiB 705.31 KiB 683.41 KiB
951bf1c 21.90 KiB 705.31 KiB 683.41 KiB

@krystofwoldrich
Copy link
Member Author

Although I changed the Xcode project version to Xcode 15.3, the schema is only visible when selecting Xcode 16 in CI.

Let me know if that's ok.

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

Looking great. I added a few comments.

.github/workflows/eval-ui-tests-p0.yml Outdated Show resolved Hide resolved
.github/workflows/eval-ui-tests-p0.yml Outdated Show resolved Hide resolved
.github/workflows/eval-ui-tests-p0.yml Outdated Show resolved Hide resolved
.github/workflows/eval-ui-tests-p0.yml Outdated Show resolved Hide resolved
.github/workflows/eval-ui-tests-p0.yml Outdated Show resolved Hide resolved
@krystofwoldrich
Copy link
Member Author

Note: I've bumped the iOS 17 runned to macos-14-xlarge as with macos-14 it was consistently failing messages regarding running the iPhone simulator. The rest, iOS 15, 16 and 18 are running correctly on the default macos-12/13/15 runners.

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

Almost LGTM.

.github/workflows/ui-tests-critical.yml Outdated Show resolved Hide resolved
Makefile Show resolved Hide resolved
TestSamples/.swiftlint.yml Outdated Show resolved Hide resolved
TestSamples/SwiftUITestSample/Maestro/corruptEnvelope.yaml Outdated Show resolved Hide resolved
Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

LGTM, nothing blocking. Thanks for doing this.

I guess as a follow-up PR, we could remove this test so we don't have it duplicated: https://github.com/getsentry/sentry-cocoa/blob/5a6e387e0b47f0b4fe046839feb957f3fd76c0fc/Samples/iOS-Swift/iOS-Swift-UITests/LaunchUITests.swift#L5C10-L20.

Copy link
Member

@philipphofmann philipphofmann left a comment

Choose a reason for hiding this comment

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

Again, LGTM.

@philipphofmann philipphofmann changed the title Add TestSample and Maestro Mobile test: Add TestSample and Maestro Mobile Oct 15, 2024
@krystofwoldrich krystofwoldrich merged commit 79eb67d into main Oct 15, 2024
46 of 50 checks passed
@krystofwoldrich krystofwoldrich deleted the kw-maestro branch October 15, 2024 12:18
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