Introduce tests sharding #21101

wzieba · 2024-07-31T11:04:36Z

Description

This PR implements instrumentation tests sharding using Fladle Gradle Plugin. Comparing to the WooCommerce Android configuration (woocommerce/woocommerce-android#12029) it has some twists:

support for two different flavors: wordpress and jetpack
working Buildkite annotations (see: https://buildkite.com/automattic/wordpress-android/builds/19466#01910d9b-4833-4b77-9a90-92315ee8da3e )

Impact

Duration of execution of instrumented tests

	Jetpack	WordPress
Before (`b5a72e3`)	8m 39s	6m 17s
After	2m 2s	2m 9s
Diff	-76.5%	-66%

Testing

I've rerun the whole job several times to make sure that tests aren't flaky. We shouldn't expect any difference in tests execution though, as we only distribute tests across few devices, instead of running all of them on a single device.

Demo

Remove unused Ruby code

wpmobilebot · 2024-07-31T11:23:15Z

📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.

	App Name	Jetpack
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr21101-40986a8
	Commit	`40986a8`
	Direct Download	`jetpack-prototype-build-pr21101-40986a8.apk`

Note: Google Login is not supported on these builds.

wpmobilebot · 2024-07-31T11:23:40Z

📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.

	App Name	WordPress
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr21101-9c05ddd
	Commit	`9c05ddd`
	Direct Download	`wordpress-prototype-build-pr21101-9c05ddd.apk`

Note: Google Login is not supported on these builds.

Setting `variant` under `configs` doesn't seem to work: runningcode/fladle#60

Copy recursively, as now there'll be multiple xml file reports

As tests between `wordpress` and `jetpack` can differ. A good example is `StatsTests#e2eAllDayStatsLoad` which runs for Jetpack app only.

Instead of relying on env variable used by previously used Fastlane Firebase action, we'll parse report generated by fladle/flank

to test Buildkite annotations

dangermattic · 2024-08-01T08:42:31Z

fastlane/lanes/test.rb

      sh("buildkite-agent annotation remove --context '#{annotation_ctx}' || true") if is_ci?
-    else
-      details_url = lane_context[SharedValues::FIREBASE_TEST_MORE_DETAILS_URL]
+    rescue


🚫 Style/RescueStandardError: Avoid rescuing without specifying an error class.

@iangmaia could you please confirm if it's okay to ignore? I want to rescue from all possible causes of gradle or sh crash.

I think those would still be caught if using rescue StandardError?

Suggested change

rescue

rescue StandardError

By the way, mentioning just in case (not sure if it would make more sense here, as you also have the gradle call): for sh, you can also use a error_callback parameter if you want to do a custom handling in case the command returns an error.

True, rescue StandardError worked fine, thanks! Addressed in 997e808

About error_callback - I don't see it being mentioned in gradle action documentation, and Gradle part is the most crucial part of this handling so I think we could keep this try/catch approach

"every action and every plugin's code runs in the root of the project, while all user code from the Fastfile runs inside the ./fastlane directory." https://docs.fastlane.tools/advanced/fastlane/\#directory-behavior

This should fix annotating test failures.

This reverts commit 3d7c853.

These tests are ignored anyway during a runtime by `assume` method. This doesn't work well with tests sharding though. In case if one of `e2eAllDayStatsLoad` tests is added on its own to a separate shard, Firebase Test Lab will mark the test suite as failed with message: "Some test executions didn't run any test cases. This is usually caused by aggressive sharding (e.g. more shards than test cases), manual sharding errors, or skipping tests. All executions have overhead time, so these may be billable or may count against your quota. A failure outcome was generated to bring your attention to this."

So the config in `wordpress` can indeed append new exclusions

fastlane/lanes/test.rb

codecov · 2024-08-02T11:24:37Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 40.39%. Comparing base (2069f57) to head (e7d901b).
Report is 38 commits behind head on trunk.

Additional details and impacted files

@@            Coverage Diff             @@
##            trunk   #21101      +/-   ##
==========================================
- Coverage   40.71%   40.39%   -0.33%     
==========================================
  Files        1530     1515      -15     
  Lines       70256    69722     -534     
  Branches    11612    11562      -50     
==========================================
- Hits        28606    28165     -441     
+ Misses      39065    38990      -75     
+ Partials     2585     2567      -18

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jostnes · 2024-08-05T06:05:12Z

WordPress/build.gradle

+                    "notClass org.wordpress.android.e2e.StatsTests",
+                    "notClass org.wordpress.android.e2e.StatsGranularTabsTest",


for test targets unavailable on the variant, do we need to define them here to be excluded?

asking because when looking at the tests, it looks like there's a check to see if it's jetpack app before continuing with test setup:

WordPress-Android/WordPress/src/androidTest/java/org/wordpress/android/e2e/StatsGranularTabsTest.kt

Line 24 in a93b999

assumeTrue(BuildConfig.IS_JETPACK_APP)

Unfortunately, they have to. If they're not, fladle/flank might create a shard with only tests from StatsTests or StatsGranularTabsTest. As they're ignored, the execution will be very fast (few seconds) and Firebase Test Lab will mark the test run as failed. It will do this to signal a problem, as too aggressive sharding might bring more unwanted costs. Some more details are available in here the comment description: cc4cc3b . Failed run like this can be found here https://buildkite.com/automattic/wordpress-android/builds/19467#01910e2f-5783-4f74-8897-2725f66ce343

I see, thanks for the detailed explanation and linking that commit's comment description! What do you think about adding a comment on the test too? So we would know for a future test that's only available on one variant we should add it to the exclusion list too

Sure thing, added a comment! WDYT?

9c05ddd

ParaskP7

👋 @wzieba !

I have reviewed this PR and everything LGTM, once again, a really awesome job done here, kudos! 🌟 x 🌟 ^ 🌟

Thanks for making fladle work with a project structure on multiple apps, I am sure it mustn't have been easy! 💯
Thanks for keeping the Firebase annotation! 🥇

I have left one questions (❓), one suggestions (💡) and one minor (🔍) comment for you to consider. I am going to approve this PR anyway, since none is blocking. I am NOT going to merge this PR yet to give you some time to apply any of my suggestions. However, feel free to ignore them and merge the PR yourself.

fastlane/lanes/test.rb

WordPress/build.gradle

To increase readability

… using Fladle/Flank

sonarqubecloud · 2024-08-09T16:55:17Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud

wzieba added 3 commits July 31, 2024 12:47

Add and configure fladle Gradle plugin

168a994

Run instrumented tests via fladle

ceb5d82

Update firebase.secrets.json path

e96ee1a

Remove unused Ruby code

wzieba added 4 commits July 31, 2024 13:26

Fix path to secrets

99d5c67

Fix setting up paths for debug and instrumentation apks

9e6afa0

Setting `variant` under `configs` doesn't seem to work: runningcode/fladle#60

Fix copying test logs for test collector

3e93f61

Copy recursively, as now there'll be multiple xml file reports

Set different paths for smart flank report file

14ba431

As tests between `wordpress` and `jetpack` can differ. A good example is `StatsTests#e2eAllDayStatsLoad` which runs for Jetpack app only.

wzieba force-pushed the introduce_tests_sharding branch from 094a51e to 6240d49 Compare August 1, 2024 08:12

Make Buildkite annotation work

d36b4a9

Instead of relying on env variable used by previously used Fastlane Firebase action, we'll parse report generated by fladle/flank

wzieba force-pushed the introduce_tests_sharding branch from 6240d49 to d00c6b6 Compare August 1, 2024 08:13

wzieba added 2 commits August 1, 2024 10:38

temp: break instrumentation tests

3d7c853

to test Buildkite annotations

Fix Ruby formatting in test.rb

cd01ef0

wzieba force-pushed the introduce_tests_sharding branch from d00c6b6 to cd01ef0 Compare August 1, 2024 08:40

dangermattic reviewed Aug 1, 2024

View reviewed changes

wzieba added 5 commits August 1, 2024 11:04

Use double asterix wildcard to locate matrix ids

a462b4d

Use relative path to locate matrix ids

4882026

Fix invalid matrix_ids relative path

7c69f45

Set correct relative path

9616d52

"every action and every plugin's code runs in the root of the project, while all user code from the Fastfile runs inside the ./fastlane directory." https://docs.fastlane.tools/advanced/fastlane/\#directory-behavior

Fix path for instrumented-tests result file

cc83564

This should fix annotating test failures.

wzieba force-pushed the introduce_tests_sharding branch from 13176ae to cc83564 Compare August 1, 2024 11:02

wzieba added 3 commits August 1, 2024 15:44

Revert "temp: break instrumentation tests"

c1ef744

This reverts commit 3d7c853.

Update order of testTargets

10c9a74

So the config in `wordpress` can indeed append new exclusions

wzieba force-pushed the introduce_tests_sharding branch from 7f9f125 to 10c9a74 Compare August 1, 2024 15:59

wzieba added [Type] Enhancement UI Tests Anything related to automated UI Tests. labels Aug 2, 2024

wzieba added this to the 25.5 milestone Aug 2, 2024

wzieba marked this pull request as ready for review August 2, 2024 09:14

wzieba requested review from ParaskP7 and jostnes August 2, 2024 09:14

iangmaia reviewed Aug 2, 2024

View reviewed changes

fastlane/lanes/test.rb Outdated Show resolved Hide resolved

wzieba added 2 commits August 2, 2024 13:00

rescue on StandardError when instrumentation tests fail

997e808

Break down jq command and its arguments

e7d901b

jostnes reviewed Aug 5, 2024

View reviewed changes

wzieba added the Do Not Merge In PRs with this label, our automation will fail a require check, preventing accidental merging label Aug 5, 2024

ParaskP7 approved these changes Aug 5, 2024

View reviewed changes

fastlane/lanes/test.rb Show resolved Hide resolved

WordPress/build.gradle Outdated Show resolved Hide resolved

WordPress/build.gradle Show resolved Hide resolved

WordPress/build.gradle Outdated Show resolved Hide resolved

wzieba added 2 commits August 5, 2024 11:24

Split pathForVariant into two methods

4d7c0b7

To increase readability

Add a comment about ignoring specific tests on WordPress variant when…

9c05ddd

… using Fladle/Flank

wzieba modified the milestones: 25.5, Future Aug 5, 2024

wzieba removed the Do Not Merge In PRs with this label, our automation will fail a require check, preventing accidental merging label Aug 9, 2024

Merge branch 'trunk' into introduce_tests_sharding

40986a8

wzieba enabled auto-merge August 9, 2024 16:52

wzieba merged commit 206f726 into trunk Aug 9, 2024
18 of 20 checks passed

wzieba deleted the introduce_tests_sharding branch August 9, 2024 17:13

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Introduce tests sharding #21101

Introduce tests sharding #21101

wzieba commented Jul 31, 2024 •

edited

Loading

wpmobilebot commented Jul 31, 2024 •

edited

Loading

wpmobilebot commented Jul 31, 2024 •

edited

Loading

dangermattic Aug 1, 2024 •

edited

Loading

wzieba Aug 2, 2024

iangmaia Aug 2, 2024

wzieba Aug 2, 2024

codecov bot commented Aug 2, 2024 •

edited

Loading

jostnes Aug 5, 2024

wzieba Aug 5, 2024

jostnes Aug 5, 2024

wzieba Aug 5, 2024 •

edited

Loading

ParaskP7 left a comment

sonarqubecloud bot commented Aug 9, 2024

		"notClass org.wordpress.android.e2e.StatsTests",
		"notClass org.wordpress.android.e2e.StatsGranularTabsTest",

Introduce tests sharding #21101

Introduce tests sharding #21101

Conversation

wzieba commented Jul 31, 2024 • edited Loading

Description

Impact

Testing

Demo

wpmobilebot commented Jul 31, 2024 • edited Loading

wpmobilebot commented Jul 31, 2024 • edited Loading

dangermattic Aug 1, 2024 • edited Loading

Choose a reason for hiding this comment

wzieba Aug 2, 2024

Choose a reason for hiding this comment

iangmaia Aug 2, 2024

Choose a reason for hiding this comment

wzieba Aug 2, 2024

Choose a reason for hiding this comment

codecov bot commented Aug 2, 2024 • edited Loading

Codecov Report

jostnes Aug 5, 2024

Choose a reason for hiding this comment

wzieba Aug 5, 2024

Choose a reason for hiding this comment

jostnes Aug 5, 2024

Choose a reason for hiding this comment

wzieba Aug 5, 2024 • edited Loading

Choose a reason for hiding this comment

ParaskP7 left a comment

Choose a reason for hiding this comment

sonarqubecloud bot commented Aug 9, 2024

Quality Gate passed

wzieba commented Jul 31, 2024 •

edited

Loading

wpmobilebot commented Jul 31, 2024 •

edited

Loading

wpmobilebot commented Jul 31, 2024 •

edited

Loading

dangermattic Aug 1, 2024 •

edited

Loading

codecov bot commented Aug 2, 2024 •

edited

Loading

wzieba Aug 5, 2024 •

edited

Loading