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

Add e2e test for iOS/Android #1516

Merged
merged 16 commits into from
Jul 18, 2023
Merged

Add e2e test for iOS/Android #1516

merged 16 commits into from
Jul 18, 2023

Conversation

denrase
Copy link
Collaborator

@denrase denrase commented Jun 13, 2023

#skip-changelog

📜 Description

  • Adds an e2e test to iOS & Android integration tests
  • Opted to extend the example app after all, but with a section that is only visible when the integration test is run.
  • That way we don't need to keep two apps up to date and we use the same options

💡 Motivation and Context

Relates to #514

💚 How did you test it?

Test

📝 Checklist

  • I reviewed submitted code
  • I added tests to verify changes
  • No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
  • I updated the docs if needed
  • All tests passing
  • No breaking changes

@codecov
Copy link

codecov bot commented Jun 13, 2023

Codecov Report

Patch coverage has no change and project coverage change: +1.07 🎉

Comparison is base (fdf9b62) 90.29% compared to head (6276ead) 91.36%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1516      +/-   ##
==========================================
+ Coverage   90.29%   91.36%   +1.07%     
==========================================
  Files         181      160      -21     
  Lines        5830     5175     -655     
==========================================
- Hits         5264     4728     -536     
+ Misses        566      447     -119     

see 21 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@marandaneto
Copy link
Contributor

if we go down this way, let's migrate https://github.com/getsentry/sentry-dart/blob/main/flutter/example/integration_test/integration_test.dart to this example so we have a single place to test stuff.

@github-actions
Copy link
Contributor

github-actions bot commented Jun 19, 2023

Android Performance metrics 🚀

  Plain With Sentry Diff
Startup time 299.35 ms 358.06 ms 58.71 ms
Size 6.16 MiB 7.14 MiB 1004.21 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
abcdba3 354.68 ms 399.04 ms 44.36 ms
ebfead1 298.44 ms 374.28 ms 75.84 ms
8d64376 302.88 ms 356.84 ms 53.96 ms
e5b744f 302.70 ms 342.17 ms 39.47 ms
43760f9 321.78 ms 366.77 ms 44.99 ms
5aba417 355.78 ms 450.39 ms 94.61 ms
521cfbf 332.78 ms 376.04 ms 43.26 ms
aa950e9 337.42 ms 400.72 ms 63.30 ms
56810ff 309.72 ms 352.26 ms 42.54 ms
379d7a8 327.10 ms 355.39 ms 28.29 ms

App size

Revision Plain With Sentry Diff
abcdba3 5.94 MiB 6.95 MiB 1.01 MiB
ebfead1 6.06 MiB 7.03 MiB 989.24 KiB
8d64376 5.94 MiB 6.96 MiB 1.02 MiB
e5b744f 6.06 MiB 7.09 MiB 1.03 MiB
43760f9 6.15 MiB 7.13 MiB 1000.49 KiB
5aba417 5.94 MiB 6.96 MiB 1.02 MiB
521cfbf 5.94 MiB 6.97 MiB 1.03 MiB
aa950e9 5.94 MiB 6.96 MiB 1.02 MiB
56810ff 5.94 MiB 6.92 MiB 1001.71 KiB
379d7a8 5.94 MiB 6.95 MiB 1.01 MiB

Previous results on branch: enha/ui-integration-tests

Startup times

Revision Plain With Sentry Diff
ea2826f 381.60 ms 457.26 ms 75.66 ms
d143570 301.63 ms 380.92 ms 79.29 ms
984c0a5 302.19 ms 367.80 ms 65.60 ms
b576967 301.73 ms 380.96 ms 79.23 ms
0342b49 337.98 ms 390.56 ms 52.58 ms
30fde7b 299.89 ms 378.68 ms 78.79 ms
896fae5 298.88 ms 354.80 ms 55.92 ms

App size

Revision Plain With Sentry Diff
ea2826f 6.16 MiB 7.14 MiB 1003.77 KiB
d143570 6.16 MiB 7.14 MiB 1003.98 KiB
984c0a5 6.16 MiB 7.14 MiB 1003.98 KiB
b576967 6.16 MiB 7.14 MiB 1004.21 KiB
0342b49 6.16 MiB 7.14 MiB 1003.99 KiB
30fde7b 6.16 MiB 7.14 MiB 1003.77 KiB
896fae5 6.16 MiB 7.14 MiB 1003.75 KiB

@github-actions
Copy link
Contributor

github-actions bot commented Jun 19, 2023

iOS Performance metrics 🚀

  Plain With Sentry Diff
Startup time 1219.20 ms 1223.33 ms 4.13 ms
Size 8.29 MiB 9.38 MiB 1.09 MiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
a817b8f 1261.90 ms 1264.62 ms 2.73 ms
6aab859 1245.14 ms 1247.59 ms 2.45 ms
e66e71e 1273.55 ms 1291.63 ms 18.08 ms
8cb6557 1265.14 ms 1266.08 ms 0.94 ms
3a69405 1292.84 ms 1303.96 ms 11.12 ms
f96ca24 1264.39 ms 1287.98 ms 23.59 ms
521cfbf 1258.94 ms 1275.18 ms 16.25 ms
aed5947 1268.67 ms 1281.10 ms 12.43 ms
870f5eb 1267.78 ms 1286.86 ms 19.08 ms
90db9ff 1277.18 ms 1283.69 ms 6.51 ms

App size

Revision Plain With Sentry Diff
a817b8f 8.29 MiB 9.37 MiB 1.07 MiB
6aab859 8.29 MiB 9.36 MiB 1.07 MiB
e66e71e 8.10 MiB 9.08 MiB 1004.36 KiB
8cb6557 8.10 MiB 9.18 MiB 1.08 MiB
3a69405 8.15 MiB 9.15 MiB 1018.56 KiB
f96ca24 8.10 MiB 9.08 MiB 1004.36 KiB
521cfbf 8.16 MiB 9.17 MiB 1.01 MiB
aed5947 8.16 MiB 9.17 MiB 1.01 MiB
870f5eb 8.15 MiB 9.13 MiB 1000.08 KiB
90db9ff 8.10 MiB 9.08 MiB 1004.27 KiB

Previous results on branch: enha/ui-integration-tests

Startup times

Revision Plain With Sentry Diff
896fae5 1232.69 ms 1247.69 ms 15.00 ms
ea2826f 1237.39 ms 1250.65 ms 13.27 ms
984c0a5 1244.41 ms 1250.61 ms 6.20 ms
b576967 1232.06 ms 1244.83 ms 12.77 ms
30fde7b 1243.33 ms 1268.18 ms 24.86 ms
0342b49 1248.96 ms 1263.51 ms 14.55 ms
d143570 1233.00 ms 1253.06 ms 20.06 ms

App size

Revision Plain With Sentry Diff
896fae5 8.29 MiB 9.37 MiB 1.08 MiB
ea2826f 8.29 MiB 9.37 MiB 1.08 MiB
984c0a5 8.29 MiB 9.37 MiB 1.08 MiB
b576967 8.29 MiB 9.37 MiB 1.08 MiB
30fde7b 8.29 MiB 9.37 MiB 1.08 MiB
0342b49 8.29 MiB 9.37 MiB 1.08 MiB
d143570 8.29 MiB 9.37 MiB 1.08 MiB

@denrase denrase changed the title UI Integration & E2E Tests Add e2e test Jun 19, 2023
@getsentry getsentry deleted a comment from github-actions bot Jun 19, 2023
@getsentry getsentry deleted a comment from github-actions bot Jun 19, 2023
@getsentry getsentry deleted a comment from github-actions bot Jun 19, 2023
@krystofwoldrich
Copy link
Member

Would there be any benefit of running E2E tests with multiple Flutter versions like we have it in RN?

@denrase denrase changed the title Add e2e test Add e2e test for iOS/Android Jun 27, 2023
@marandaneto
Copy link
Contributor

Would there be any benefit of running E2E tests with multiple Flutter versions like we have it in RN?

Yes, but the overhead is quite big (CI time/maintenance), we run in the minimal supported version, the stable and beta version, which is IMO enough.

@denrase denrase marked this pull request as ready for review July 4, 2023 12:31
return null;
}

bool validate(Map<String, dynamic> event) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we actually validate more properties in the event?
I know this is a copy of https://github.com/getsentry/sentry-dart/blob/main/e2e_test/bin/e2e_test.dart
Maybe we should improve that test as well.
The goal is that we assert every property that is static and not dependent on the host platform, so it wont fail in different devices, etc.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I check fro the same properties than getsentry/sentry-kotlin-multiplatform#78 now. Do we need more? If not, i will update the dart only test as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's start with the current approach and expand as we go.
Ideally, we'd check every property that is static, does not change depending on the device, etc.

@krystofwoldrich
Copy link
Member

minimal supported version, the stable and beta version

Yes, I agree, this is enough for sure, I've missed that, I thought the e2e run only on stable.

@marandaneto marandaneto enabled auto-merge (squash) July 17, 2023 13:58
@marandaneto marandaneto merged commit 0aaa46e into main Jul 18, 2023
37 of 38 checks passed
@marandaneto marandaneto deleted the enha/ui-integration-tests branch July 18, 2023 11:26
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.

3 participants