Fix part of #5343: Code Coverage script edge cases #5453

Rd4dev · 2024-06-28T16:24:04Z

Explanation

Fixes part of #5343

This is an extended part of Milestone 1 to address any edge cases.

Project

[Project 4.1]

Changes Made

Added arg names to the run_coverage scripts to cover edge cases on handling formats and reordering the arguments.

Updated script run command:

bazel run //scripts:run_coverage -- $(pwd) utility/src/main/java/org/oppia/android/util/parser/math/MathModel.kt format=HTML

Essential Checklist

The PR title and explanation each start with "Fix #bugnum: " (If this PR fixes part of an issue, prefix the title with "Fix part of #bugnum: ...".)
Any changes to scripts/assets files have their rationale included in the PR explanation.
The PR follows the style guide.
The PR does not contain any unnecessary code changes from Android Studio (reference).
The PR is made from a branch that's not called "develop" and is up-to-date with "develop".
The PR is assigned to the appropriate reviewers (reference).

For UI-specific PRs only

If your PR includes UI-related changes, then:

Add screenshots for portrait/landscape for both a tablet & phone of the before & after UI changes
For the screenshots above, include both English and pseudo-localized (RTL) screenshots (see RTL guide)
Add a video showing the full UX flow with a screen reader enabled (see accessibility guide)
For PRs introducing new UI elements or color changes, both light and dark mode screenshots must be included
Add a screenshot demonstrating that you ran affected Espresso tests locally & that they're passing

…args to and make reordering possible

…re descriptive

Rd4dev · 2024-06-28T16:56:07Z

Hi @adhiamboperes, addressed the argument name / ordering edge case as suggested. Can you PTAL!
Also changed exception statement to be more descriptive.

Rd4dev · 2024-06-28T19:50:28Z

@adhiamboperes added tests for case sensitivity and reordered arguments, would it require any other tests?

… a long time causing TIMEOUT failures

adhiamboperes

Thanks @Rd4dev. This LGTM.

Rd4dev · 2024-06-28T20:42:41Z

@adhiamboperes the tests fail due to TIMEOUT, I tried changing them in the test longCommandExecutor but that didn't seem to work: https://github.com/oppia/oppia-android/actions/runs/9717824867/job/26824483404?pr=5453#step:17:932

oppiabot · 2024-06-28T20:43:44Z

Unassigning @adhiamboperes since they have already approved the PR.

oppiabot · 2024-06-28T20:43:47Z

Assigning @BenHenning for code owner reviews. Thanks!

adhiamboperes · 2024-06-28T20:48:30Z

@seanlip, we discovered some edge cases from RD's last PR, and made a follow up PR instead of reverting the last one. cc @DubeySandeep

…cts the TIMEOUT

adhiamboperes · 2024-06-29T00:12:01Z

@Rd4dev PTAL for CI failures

…on the tests

…on the tests adding missed paranthesis

github-actions · 2024-06-29T03:27:47Z

APK & AAB differences analysis

Note that this is a summarized snapshot. See the CI artifacts for detailed differences.

Dev

Expand to see flavor specifics

Universal APK

APK file size: 16 MiB (old), 16 MiB (new), 8 bytes (Removed)

APK download size (estimated): 14 MiB (old), 14 MiB (new), 15 bytes (Added)

Method count: 227007 (old), 227007 (new), 0 (No change)

Features: 2 (old), 2 (new), 0 (No change)

Permissions: 6 (old), 6 (new), 0 (No change)

Resources: 6550 (old), 6550 (new), 0 (No change)

Anim: 49 (old), 49 (new), 0 (No change)
Animator: 20 (old), 20 (new), 0 (No change)
Array: 15 (old), 15 (new), 0 (No change)
Attr: 915 (old), 915 (new), 0 (No change)
Bool: 10 (old), 10 (new), 0 (No change)
Color: 911 (old), 911 (new), 0 (No change)
Dimen: 994 (old), 994 (new), 0 (No change)
Drawable: 373 (old), 373 (new), 0 (No change)
Id: 1206 (old), 1206 (new), 0 (No change)
Integer: 37 (old), 37 (new), 0 (No change)
Interpolator: 11 (old), 11 (new), 0 (No change)
Layout: 368 (old), 368 (new), 0 (No change)
Menu: 1 (old), 1 (new), 0 (No change)
Mipmap: 1 (old), 1 (new), 0 (No change)
Plurals: 10 (old), 10 (new), 0 (No change)
Raw: 2 (old), 2 (new), 0 (No change)
String: 805 (old), 805 (new), 0 (No change)
Style: 816 (old), 816 (new), 0 (No change)
Xml: 6 (old), 6 (new), 0 (No change)

Lesson assets: 111 (old), 111 (new), 0 (No change)

AAB differences

Expand to see AAB specifics

Supported configurations:

hdpi (same)
ldpi (same)
mdpi (same)
tvdpi (same)
xhdpi (same)
xxhdpi (same)
xxxhdpi (same)

Base APK

APK file size: 16 MiB (old), 16 MiB (new), 8 bytes (Removed)
APK download size (estimated): 14 MiB (old), 14 MiB (new), 20 bytes (Removed)

Configuration hdpi

APK file size: 59 KiB (old), 59 KiB (new), 0 bytes (No change)
APK download size (estimated): 23 KiB (old), 23 KiB (new), 0 bytes (No change)

Configuration ldpi

APK file size: 56 KiB (old), 56 KiB (new), 0 bytes (No change)
APK download size (estimated): 18 KiB (old), 18 KiB (new), 0 bytes (No change)

Configuration mdpi

APK file size: 53 KiB (old), 53 KiB (new), 0 bytes (No change)
APK download size (estimated): 18 KiB (old), 18 KiB (new), 0 bytes (No change)

Configuration tvdpi

APK file size: 102 KiB (old), 102 KiB (new), 0 bytes (No change)
APK download size (estimated): 38 KiB (old), 38 KiB (new), 0 bytes (No change)

Configuration xhdpi

APK file size: 67 KiB (old), 67 KiB (new), 0 bytes (No change)
APK download size (estimated): 28 KiB (old), 28 KiB (new), 0 bytes (No change)

Configuration xxhdpi

APK file size: 76 KiB (old), 76 KiB (new), 0 bytes (No change)
APK download size (estimated): 38 KiB (old), 38 KiB (new), 0 bytes (No change)

Configuration xxxhdpi

APK file size: 79 KiB (old), 79 KiB (new), 0 bytes (No change)
APK download size (estimated): 39 KiB (old), 39 KiB (new), 0 bytes (No change)

Alpha

Expand to see flavor specifics

Universal APK

APK file size: 10 MiB (old), 10 MiB (new), 4 bytes (Removed)

APK download size (estimated): 9184 KiB (old), 9184 KiB (new), 5 bytes (Removed)

Method count: 101341 (old), 101341 (new), 0 (No change)

Features: 2 (old), 2 (new), 0 (No change)

Permissions: 6 (old), 6 (new), 0 (No change)

Resources: 5504 (old), 5504 (new), 0 (No change)

Anim: 39 (old), 39 (new), 0 (No change)
Animator: 18 (old), 18 (new), 0 (No change)
Array: 14 (old), 14 (new), 0 (No change)
Attr: 879 (old), 879 (new), 0 (No change)
Bool: 8 (old), 8 (new), 0 (No change)
Color: 767 (old), 767 (new), 0 (No change)
Dimen: 722 (old), 722 (new), 0 (No change)
Drawable: 333 (old), 333 (new), 0 (No change)
Id: 1148 (old), 1148 (new), 0 (No change)
Integer: 32 (old), 32 (new), 0 (No change)
Interpolator: 11 (old), 11 (new), 0 (No change)
Layout: 327 (old), 327 (new), 0 (No change)
Menu: 1 (old), 1 (new), 0 (No change)
Mipmap: 1 (old), 1 (new), 0 (No change)
Plurals: 10 (old), 10 (new), 0 (No change)
String: 736 (old), 736 (new), 0 (No change)
Style: 457 (old), 457 (new), 0 (No change)
Xml: 1 (old), 1 (new), 0 (No change)

Lesson assets: 111 (old), 111 (new), 0 (No change)

AAB differences

Expand to see AAB specifics

Supported configurations:

hdpi (same)
ldpi (same)
mdpi (same)
tvdpi (same)
xhdpi (same)
xxhdpi (same)
xxxhdpi (same)

Base APK

APK file size: 9 MiB (old), 9 MiB (new), 8 bytes (Removed)
APK download size (estimated): 9064 KiB (old), 9064 KiB (new), 33 bytes (Removed)

Configuration hdpi

APK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change)
APK download size (estimated): 22 KiB (old), 22 KiB (new), 0 bytes (No change)

Configuration ldpi

APK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change)
APK download size (estimated): 17 KiB (old), 17 KiB (new), 0 bytes (No change)

Configuration mdpi

APK file size: 46 KiB (old), 46 KiB (new), 0 bytes (No change)
APK download size (estimated): 17 KiB (old), 17 KiB (new), 0 bytes (No change)

Configuration tvdpi

APK file size: 90 KiB (old), 90 KiB (new), 0 bytes (No change)
APK download size (estimated): 37 KiB (old), 37 KiB (new), 0 bytes (No change)

Configuration xhdpi

APK file size: 60 KiB (old), 60 KiB (new), 0 bytes (No change)
APK download size (estimated): 27 KiB (old), 27 KiB (new), 0 bytes (No change)

Configuration xxhdpi

APK file size: 69 KiB (old), 69 KiB (new), 0 bytes (No change)
APK download size (estimated): 37 KiB (old), 37 KiB (new), 0 bytes (No change)

Configuration xxxhdpi

APK file size: 71 KiB (old), 71 KiB (new), 0 bytes (No change)
APK download size (estimated): 38 KiB (old), 38 KiB (new), 0 bytes (No change)

Beta

Expand to see flavor specifics

Universal APK

APK file size: 10 MiB (old), 10 MiB (new), 0 bytes (No change)

APK download size (estimated): 9169 KiB (old), 9169 KiB (new), 21 bytes (Added)

Method count: 101341 (old), 101341 (new), 0 (No change)

Features: 2 (old), 2 (new), 0 (No change)

Permissions: 6 (old), 6 (new), 0 (No change)

Resources: 5504 (old), 5504 (new), 0 (No change)

Anim: 39 (old), 39 (new), 0 (No change)
Animator: 18 (old), 18 (new), 0 (No change)
Array: 14 (old), 14 (new), 0 (No change)
Attr: 879 (old), 879 (new), 0 (No change)
Bool: 8 (old), 8 (new), 0 (No change)
Color: 767 (old), 767 (new), 0 (No change)
Dimen: 722 (old), 722 (new), 0 (No change)
Drawable: 333 (old), 333 (new), 0 (No change)
Id: 1148 (old), 1148 (new), 0 (No change)
Integer: 32 (old), 32 (new), 0 (No change)
Interpolator: 11 (old), 11 (new), 0 (No change)
Layout: 327 (old), 327 (new), 0 (No change)
Menu: 1 (old), 1 (new), 0 (No change)
Mipmap: 1 (old), 1 (new), 0 (No change)
Plurals: 10 (old), 10 (new), 0 (No change)
String: 736 (old), 736 (new), 0 (No change)
Style: 457 (old), 457 (new), 0 (No change)
Xml: 1 (old), 1 (new), 0 (No change)

Lesson assets: 111 (old), 111 (new), 0 (No change)

AAB differences

Expand to see AAB specifics

Supported configurations:

hdpi (same)
ldpi (same)
mdpi (same)
tvdpi (same)
xhdpi (same)
xxhdpi (same)
xxxhdpi (same)

Base APK

APK file size: 9 MiB (old), 9 MiB (new), 0 bytes (No change)
APK download size (estimated): 9053 KiB (old), 9053 KiB (new), 15 bytes (Removed)

Configuration hdpi

APK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change)
APK download size (estimated): 22 KiB (old), 22 KiB (new), 0 bytes (No change)

Configuration ldpi

APK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change)
APK download size (estimated): 17 KiB (old), 17 KiB (new), 0 bytes (No change)

Configuration mdpi

APK file size: 46 KiB (old), 46 KiB (new), 0 bytes (No change)
APK download size (estimated): 17 KiB (old), 17 KiB (new), 0 bytes (No change)

Configuration tvdpi

APK file size: 90 KiB (old), 90 KiB (new), 0 bytes (No change)
APK download size (estimated): 37 KiB (old), 37 KiB (new), 0 bytes (No change)

Configuration xhdpi

APK file size: 60 KiB (old), 60 KiB (new), 0 bytes (No change)
APK download size (estimated): 27 KiB (old), 27 KiB (new), 0 bytes (No change)

Configuration xxhdpi

APK file size: 69 KiB (old), 69 KiB (new), 0 bytes (No change)
APK download size (estimated): 37 KiB (old), 37 KiB (new), 0 bytes (No change)

Configuration xxxhdpi

APK file size: 71 KiB (old), 71 KiB (new), 0 bytes (No change)
APK download size (estimated): 38 KiB (old), 38 KiB (new), 0 bytes (No change)

Ga

Expand to see flavor specifics

Universal APK

APK file size: 10 MiB (old), 10 MiB (new), 0 bytes (No change)

APK download size (estimated): 9169 KiB (old), 9169 KiB (new), 6 bytes (Removed)

Method count: 101341 (old), 101341 (new), 0 (No change)

Features: 2 (old), 2 (new), 0 (No change)

Permissions: 6 (old), 6 (new), 0 (No change)

Resources: 5504 (old), 5504 (new), 0 (No change)

Anim: 39 (old), 39 (new), 0 (No change)
Animator: 18 (old), 18 (new), 0 (No change)
Array: 14 (old), 14 (new), 0 (No change)
Attr: 879 (old), 879 (new), 0 (No change)
Bool: 8 (old), 8 (new), 0 (No change)
Color: 767 (old), 767 (new), 0 (No change)
Dimen: 722 (old), 722 (new), 0 (No change)
Drawable: 333 (old), 333 (new), 0 (No change)
Id: 1148 (old), 1148 (new), 0 (No change)
Integer: 32 (old), 32 (new), 0 (No change)
Interpolator: 11 (old), 11 (new), 0 (No change)
Layout: 327 (old), 327 (new), 0 (No change)
Menu: 1 (old), 1 (new), 0 (No change)
Mipmap: 1 (old), 1 (new), 0 (No change)
Plurals: 10 (old), 10 (new), 0 (No change)
String: 736 (old), 736 (new), 0 (No change)
Style: 457 (old), 457 (new), 0 (No change)
Xml: 1 (old), 1 (new), 0 (No change)

Lesson assets: 111 (old), 111 (new), 0 (No change)

AAB differences

Expand to see AAB specifics

Supported configurations:

hdpi (same)
ldpi (same)
mdpi (same)
tvdpi (same)
xhdpi (same)
xxhdpi (same)
xxxhdpi (same)

Base APK

APK file size: 9 MiB (old), 9 MiB (new), 0 bytes (No change)
APK download size (estimated): 9053 KiB (old), 9053 KiB (new), 12 bytes (Removed)

Configuration hdpi

APK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change)
APK download size (estimated): 22 KiB (old), 22 KiB (new), 0 bytes (No change)

Configuration ldpi

APK file size: 52 KiB (old), 52 KiB (new), 0 bytes (No change)
APK download size (estimated): 17 KiB (old), 17 KiB (new), 0 bytes (No change)

Configuration mdpi

APK file size: 46 KiB (old), 46 KiB (new), 0 bytes (No change)
APK download size (estimated): 17 KiB (old), 17 KiB (new), 0 bytes (No change)

Configuration tvdpi

APK file size: 90 KiB (old), 90 KiB (new), 0 bytes (No change)
APK download size (estimated): 37 KiB (old), 37 KiB (new), 0 bytes (No change)

Configuration xhdpi

APK file size: 60 KiB (old), 60 KiB (new), 0 bytes (No change)
APK download size (estimated): 27 KiB (old), 27 KiB (new), 0 bytes (No change)

Configuration xxhdpi

APK file size: 69 KiB (old), 69 KiB (new), 0 bytes (No change)
APK download size (estimated): 37 KiB (old), 37 KiB (new), 0 bytes (No change)

Configuration xxxhdpi

APK file size: 71 KiB (old), 71 KiB (new), 0 bytes (No change)
APK download size (estimated): 38 KiB (old), 38 KiB (new), 0 bytes (No change)

Rd4dev · 2024-06-29T03:56:23Z

Hey @adhiamboperes, I just tried reverting all the changes to see if those are causing any issues and the tests though timed out at Attempt 1 seems to pass from Attempt 2: stack trace from Attempt 2
(All checks passed)

So, there is something from the newly introduced changes (which is only the arg parsing) that was messing up the tests.
I will try to re-introduce them and see what's actually happening.

Edit: There is nothing failing with the logic (all checks passed reverting the test cases), its just the addition of tests making it wait too long causing TIMEOUT!

Passing locally:

Edit:
Tried changing the processTimeout in computeAffectedTestsTest and they properly reflect the timeout setting: 100d8f1

Edit:
The processTimeout works if I set it to a lower value but doesn't reflect if I choose a higher value, guess that's a limitation to timeout at max 300s.

This is the stack trace of : e82f2fd where I tried to set processTimeout to 1L MILLISECOND and that is reflected.

I am not sure where the actual max limitation is set (if that's something that is restricting the processTimeout to go beyond 300s which is actually needed to run coverages).

Edit: I probably doubt if this could be due to varied processTimeout references at various places.
If that's TODO: Test with a common commandExecutor value for RunCoverage

Edit: Tried increasing the processTimeout in computeAffectedTestsTest to 10 minutes while adding a sleep of 350 seconds to the test and that works [stack trace], so there is no max limit its just the processTimeout somewhere messing in the RunCoverageTest.

Edit: I doubt the timeout is not actually related to RunCoverage script settings solely but was failing because of the kotlin coroutine timeouts

Edit: Tried testing with a more time intensive test case locally - assertEquals(computeFibonacci(45), 1134903170L) and that failed with kotlin coroutine timed out error and processTimeout set via RunCoverage was ignored. Its ultimately the runBlocking timing out (earlier the computeAffectedTestsTest processed with a higher time out value since it didn't use any coroutines)

Edit: Tried adding a delay to the MavenDependeciesCheckList, in ci it throws the same TIMED OUT error on Coroutines but here the timeout occured after 800+ seconds which was 300s for RunCoverage [stack trace]

Fix

Edit: Ok, I think I understood where the actual problem is, so it was basically due to the test exceeding bazel's --test_timeout value, it was set to bazel runner doc bazel test's moderate value causing a timeout at 300s. And increasing the timeout value using --test_timeout flag fixes that. [checks passing in ci]

… then convert them to uppercase

…case removal

Rd4dev · 2024-07-11T14:43:56Z

@BenHenning, @adhiamboperes, I think I finally figured it out (shouldn't have taken this long).

The test runtimes consistently exceed 300 seconds, so the previous implementation introduced --test_timeout to the Bazel test runs. However, there were concerns that this would affect the entire run rather than specific test targets.

It turned out to be a straightforward fix 75e28e2 to add size = "large" to the test rule [bazel implied timeout labels], similar to what was done for other test targets (MavenDependenciesListCheckTest, ComputeAffectedTestsTest). This change should set the timeout for the test target to (long) 900 seconds, allowing it to complete its execution.

Most recent ci run passing in ~480 seconds [stack trace]

Uhm.. I'd like to know if this is the right approach and hope this resolves everything related to --test_timeout issues!

Edit: Also, I wonder if this should be subdivided to help parallelize test execution using shard_count, as done with other large test targets. If yes, the count needs to be determined. Looking to discuss this and proceed.

scripts/src/java/org/oppia/android/scripts/testing/TestBazelWorkspace.kt

adhiamboperes · 2024-07-11T20:56:30Z

@Rd4dev, good job on identifying the problem!

Re:

I wonder if this should be subdivided to help parallelize test execution using shard_count, as done with other large test targets.

That could be discussed later, I will not block merging this PR on that. Can you add it to your list of tasks for CI validations later in M2?

github-actions · 2024-07-12T04:10:52Z

APK & AAB differences analysis

Note that this is a summarized snapshot. See the CI artifacts for detailed differences.

Dev

Expand to see flavor specifics

Universal APK

APK file size: 19 MiB (old), 19 MiB (new), 4 bytes (Removed)

APK download size (estimated): 17 MiB (old), 17 MiB (new), 5 bytes (Added)

Method count: 258445 (old), 258445 (new), 0 (No change)

Features: 2 (old), 2 (new), 0 (No change)

Permissions: 6 (old), 6 (new), 0 (No change)

Resources: 6775 (old), 6775 (new), 0 (No change)

Anim: 43 (old), 43 (new), 0 (No change)
Animator: 26 (old), 26 (new), 0 (No change)
Array: 15 (old), 15 (new), 0 (No change)
Attr: 922 (old), 922 (new), 0 (No change)
Bool: 9 (old), 9 (new), 0 (No change)
Color: 954 (old), 954 (new), 0 (No change)
Dimen: 1030 (old), 1030 (new), 0 (No change)
Drawable: 378 (old), 378 (new), 0 (No change)
Id: 1271 (old), 1271 (new), 0 (No change)
Integer: 37 (old), 37 (new), 0 (No change)
Interpolator: 11 (old), 11 (new), 0 (No change)
Layout: 378 (old), 378 (new), 0 (No change)
Menu: 3 (old), 3 (new), 0 (No change)
Mipmap: 1 (old), 1 (new), 0 (No change)
Plurals: 10 (old), 10 (new), 0 (No change)
Raw: 2 (old), 2 (new), 0 (No change)
String: 848 (old), 848 (new), 0 (No change)
Style: 831 (old), 831 (new), 0 (No change)
Xml: 6 (old), 6 (new), 0 (No change)

Lesson assets: 111 (old), 111 (new), 0 (No change)

AAB differences

Expand to see AAB specifics

Supported configurations:

hdpi (same)
ldpi (same)
mdpi (same)
tvdpi (same)
xhdpi (same)
xxhdpi (same)
xxxhdpi (same)

Base APK

APK file size: 18 MiB (old), 18 MiB (new), 4 bytes (Removed)
APK download size (estimated): 17 MiB (old), 17 MiB (new), 4 bytes (Added)

Configuration hdpi

APK file size: 49 KiB (old), 49 KiB (new), 0 bytes (No change)
APK download size (estimated): 18 KiB (old), 18 KiB (new), 0 bytes (No change)

Configuration ldpi

APK file size: 48 KiB (old), 48 KiB (new), 0 bytes (No change)
APK download size (estimated): 14 KiB (old), 14 KiB (new), 0 bytes (No change)

Configuration mdpi

APK file size: 45 KiB (old), 45 KiB (new), 0 bytes (No change)
APK download size (estimated): 14 KiB (old), 14 KiB (new), 0 bytes (No change)

Configuration tvdpi

APK file size: 85 KiB (old), 85 KiB (new), 0 bytes (No change)
APK download size (estimated): 29 KiB (old), 29 KiB (new), 0 bytes (No change)

Configuration xhdpi

APK file size: 56 KiB (old), 56 KiB (new), 0 bytes (No change)
APK download size (estimated): 21 KiB (old), 21 KiB (new), 0 bytes (No change)

Configuration xxhdpi

APK file size: 62 KiB (old), 62 KiB (new), 0 bytes (No change)
APK download size (estimated): 29 KiB (old), 29 KiB (new), 0 bytes (No change)

Configuration xxxhdpi

APK file size: 63 KiB (old), 63 KiB (new), 0 bytes (No change)
APK download size (estimated): 28 KiB (old), 28 KiB (new), 0 bytes (No change)

Alpha

Expand to see flavor specifics

Universal APK

APK file size: 11 MiB (old), 11 MiB (new), 8 bytes (Removed)

APK download size (estimated): 10 MiB (old), 10 MiB (new), 6 bytes (Removed)

Method count: 114844 (old), 114844 (new), 0 (No change)

Features: 2 (old), 2 (new), 0 (No change)

Permissions: 6 (old), 6 (new), 0 (No change)

Resources: 5745 (old), 5745 (new), 0 (No change)

Anim: 33 (old), 33 (new), 0 (No change)
Animator: 24 (old), 24 (new), 0 (No change)
Array: 14 (old), 14 (new), 0 (No change)
Attr: 888 (old), 888 (new), 0 (No change)
Bool: 8 (old), 8 (new), 0 (No change)
Color: 806 (old), 806 (new), 0 (No change)
Dimen: 765 (old), 765 (new), 0 (No change)
Drawable: 340 (old), 340 (new), 0 (No change)
Id: 1217 (old), 1217 (new), 0 (No change)
Integer: 32 (old), 32 (new), 0 (No change)
Interpolator: 11 (old), 11 (new), 0 (No change)
Layout: 341 (old), 341 (new), 0 (No change)
Menu: 1 (old), 1 (new), 0 (No change)
Mipmap: 1 (old), 1 (new), 0 (No change)
Plurals: 10 (old), 10 (new), 0 (No change)
String: 781 (old), 781 (new), 0 (No change)
Style: 472 (old), 472 (new), 0 (No change)
Xml: 1 (old), 1 (new), 0 (No change)

Lesson assets: 111 (old), 111 (new), 0 (No change)

AAB differences

Expand to see AAB specifics

Supported configurations:

hdpi (same)
ldpi (same)
mdpi (same)
tvdpi (same)
xhdpi (same)
xxhdpi (same)
xxxhdpi (same)

Base APK

APK file size: 10 MiB (old), 10 MiB (new), 8 bytes (Removed)
APK download size (estimated): 9 MiB (old), 9 MiB (new), 4 bytes (Removed)

Configuration hdpi

APK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change)
APK download size (estimated): 17 KiB (old), 17 KiB (new), 0 bytes (No change)

Configuration ldpi

APK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change)
APK download size (estimated): 13 KiB (old), 13 KiB (new), 0 bytes (No change)

Configuration mdpi

APK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change)
APK download size (estimated): 13 KiB (old), 13 KiB (new), 0 bytes (No change)

Configuration tvdpi

APK file size: 72 KiB (old), 72 KiB (new), 0 bytes (No change)
APK download size (estimated): 27 KiB (old), 27 KiB (new), 0 bytes (No change)

Configuration xhdpi

APK file size: 49 KiB (old), 49 KiB (new), 0 bytes (No change)
APK download size (estimated): 20 KiB (old), 20 KiB (new), 0 bytes (No change)

Configuration xxhdpi

APK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change)
APK download size (estimated): 28 KiB (old), 28 KiB (new), 0 bytes (No change)

Configuration xxxhdpi

APK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change)
APK download size (estimated): 27 KiB (old), 27 KiB (new), 0 bytes (No change)

Beta

Expand to see flavor specifics

Universal APK

APK file size: 11 MiB (old), 11 MiB (new), 4 bytes (Added)

APK download size (estimated): 10 MiB (old), 10 MiB (new), 7 bytes (Added)

Method count: 114850 (old), 114850 (new), 0 (No change)

Features: 2 (old), 2 (new), 0 (No change)

Permissions: 6 (old), 6 (new), 0 (No change)

Resources: 5745 (old), 5745 (new), 0 (No change)

Anim: 33 (old), 33 (new), 0 (No change)
Animator: 24 (old), 24 (new), 0 (No change)
Array: 14 (old), 14 (new), 0 (No change)
Attr: 888 (old), 888 (new), 0 (No change)
Bool: 8 (old), 8 (new), 0 (No change)
Color: 806 (old), 806 (new), 0 (No change)
Dimen: 765 (old), 765 (new), 0 (No change)
Drawable: 340 (old), 340 (new), 0 (No change)
Id: 1217 (old), 1217 (new), 0 (No change)
Integer: 32 (old), 32 (new), 0 (No change)
Interpolator: 11 (old), 11 (new), 0 (No change)
Layout: 341 (old), 341 (new), 0 (No change)
Menu: 1 (old), 1 (new), 0 (No change)
Mipmap: 1 (old), 1 (new), 0 (No change)
Plurals: 10 (old), 10 (new), 0 (No change)
String: 781 (old), 781 (new), 0 (No change)
Style: 472 (old), 472 (new), 0 (No change)
Xml: 1 (old), 1 (new), 0 (No change)

Lesson assets: 111 (old), 111 (new), 0 (No change)

AAB differences

Expand to see AAB specifics

Supported configurations:

hdpi (same)
ldpi (same)
mdpi (same)
tvdpi (same)
xhdpi (same)
xxhdpi (same)
xxxhdpi (same)

Base APK

APK file size: 10 MiB (old), 10 MiB (new), 4 bytes (Added)
APK download size (estimated): 9 MiB (old), 9 MiB (new), 36 bytes (Added)

Configuration hdpi

APK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change)
APK download size (estimated): 17 KiB (old), 17 KiB (new), 0 bytes (No change)

Configuration ldpi

APK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change)
APK download size (estimated): 13 KiB (old), 13 KiB (new), 0 bytes (No change)

Configuration mdpi

APK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change)
APK download size (estimated): 13 KiB (old), 13 KiB (new), 0 bytes (No change)

Configuration tvdpi

APK file size: 72 KiB (old), 72 KiB (new), 0 bytes (No change)
APK download size (estimated): 27 KiB (old), 27 KiB (new), 0 bytes (No change)

Configuration xhdpi

APK file size: 49 KiB (old), 49 KiB (new), 0 bytes (No change)
APK download size (estimated): 20 KiB (old), 20 KiB (new), 0 bytes (No change)

Configuration xxhdpi

APK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change)
APK download size (estimated): 28 KiB (old), 28 KiB (new), 0 bytes (No change)

Configuration xxxhdpi

APK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change)
APK download size (estimated): 27 KiB (old), 27 KiB (new), 0 bytes (No change)

Ga

Expand to see flavor specifics

Universal APK

APK file size: 11 MiB (old), 11 MiB (new), 0 bytes (No change)

APK download size (estimated): 10 MiB (old), 10 MiB (new), 5 bytes (Removed)

Method count: 114850 (old), 114850 (new), 0 (No change)

Features: 2 (old), 2 (new), 0 (No change)

Permissions: 6 (old), 6 (new), 0 (No change)

Resources: 5745 (old), 5745 (new), 0 (No change)

Anim: 33 (old), 33 (new), 0 (No change)
Animator: 24 (old), 24 (new), 0 (No change)
Array: 14 (old), 14 (new), 0 (No change)
Attr: 888 (old), 888 (new), 0 (No change)
Bool: 8 (old), 8 (new), 0 (No change)
Color: 806 (old), 806 (new), 0 (No change)
Dimen: 765 (old), 765 (new), 0 (No change)
Drawable: 340 (old), 340 (new), 0 (No change)
Id: 1217 (old), 1217 (new), 0 (No change)
Integer: 32 (old), 32 (new), 0 (No change)
Interpolator: 11 (old), 11 (new), 0 (No change)
Layout: 341 (old), 341 (new), 0 (No change)
Menu: 1 (old), 1 (new), 0 (No change)
Mipmap: 1 (old), 1 (new), 0 (No change)
Plurals: 10 (old), 10 (new), 0 (No change)
String: 781 (old), 781 (new), 0 (No change)
Style: 472 (old), 472 (new), 0 (No change)
Xml: 1 (old), 1 (new), 0 (No change)

Lesson assets: 111 (old), 111 (new), 0 (No change)

AAB differences

Expand to see AAB specifics

Supported configurations:

hdpi (same)
ldpi (same)
mdpi (same)
tvdpi (same)
xhdpi (same)
xxhdpi (same)
xxxhdpi (same)

Base APK

APK file size: 10 MiB (old), 10 MiB (new), 0 bytes (No change)
APK download size (estimated): 9 MiB (old), 9 MiB (new), 10 bytes (Added)

Configuration hdpi

APK file size: 43 KiB (old), 43 KiB (new), 0 bytes (No change)
APK download size (estimated): 17 KiB (old), 17 KiB (new), 0 bytes (No change)

Configuration ldpi

APK file size: 44 KiB (old), 44 KiB (new), 0 bytes (No change)
APK download size (estimated): 13 KiB (old), 13 KiB (new), 0 bytes (No change)

Configuration mdpi

APK file size: 38 KiB (old), 38 KiB (new), 0 bytes (No change)
APK download size (estimated): 13 KiB (old), 13 KiB (new), 0 bytes (No change)

Configuration tvdpi

APK file size: 72 KiB (old), 72 KiB (new), 0 bytes (No change)
APK download size (estimated): 27 KiB (old), 27 KiB (new), 0 bytes (No change)

Configuration xhdpi

APK file size: 49 KiB (old), 49 KiB (new), 0 bytes (No change)
APK download size (estimated): 20 KiB (old), 20 KiB (new), 0 bytes (No change)

Configuration xxhdpi

APK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change)
APK download size (estimated): 28 KiB (old), 28 KiB (new), 0 bytes (No change)

Configuration xxxhdpi

APK file size: 55 KiB (old), 55 KiB (new), 0 bytes (No change)
APK download size (estimated): 27 KiB (old), 27 KiB (new), 0 bytes (No change)

…ent specific to the test case

Rd4dev · 2024-07-12T08:49:48Z

@Rd4dev, good job on identifying the problem!

Re:

I wonder if this should be subdivided to help parallelize test execution using shard_count, as done with other large test targets.

That could be discussed later, I will not block merging this PR on that. Can you add it to your list of tasks for CI validations later in M2?

Thanks!

Sure, I'll include it as a sub-task for the M2 list.

… its not a flake

…emoving it to see if that is actually causing the issues (may be with idle times)

… most importantly helps with max idle run time issues

Rd4dev · 2024-07-12T18:45:54Z

Hey @adhiamboperes,

The tests failed with the following error:

1) testRunCoverage_scriptTestsMarkdownFormat_returnsCoverageData(org.oppia.android.scripts.coverage.RunCoverageTest)
java.lang.IllegalStateException: Expected non-zero exit code (not 1) for command: [bazel, query, --noshow_progress, --keep_going, set(scripts/javatests/com/example/TwoSumTest.kt)].
Standard output:
Error output:
$TEST_TMPDIR defined: output root default is '/home/runner/.cache/bazel/_bazel_runner/ef69b31fa14ea8341d6589b0a8832bdd/sandbox/linux-sandbox/652/execroot/__main__/_tmp/fdac3d66717137a77a91b3f32165a80f' and max_idle_secs def
================================================================================
INFO: Elapsed time: 333.443s, Critical Path: 333.28s

They passed locally, so I tried adding shards_count and set it to 4, which is the minimum for other large tests. This significantly reduced the run time (from ~350 seconds to ~240 seconds) as they now run in parallel. It also seems to help with timeouts and the max_idle_secs issue, as introducing it solved the problem. I verified it by removing and re-adding it; the issue reappears when it's removed [failure stack trace] and is resolved when re-added [success stack trace]. I guess the issue should be with tests taking too long to complete and hitting idle times and causing this issue.

And this was something mentioned to be considered for M2 but given the priority to merge this PR ASAP and the small fix it represents; I'm including it here. The exact number of shards can be updated in M2 if needed. Also, I believe this will be very beneficial for upcoming PRs, as they are likely to introduce more test cases with multiple test suites. Can you PTAL.

Thanks!

oppiabot · 2024-07-12T18:48:58Z

Unassigning @Rd4dev since a re-review was requested. @Rd4dev, please make sure you have addressed all review comments. Thanks!

adhiamboperes

Thanks @Rd4dev! Latest changes LGTM.

oppiabot · 2024-07-12T22:07:41Z

Unassigning @adhiamboperes since they have already approved the PR.

oppiabot · 2024-07-12T22:07:44Z

Assigning @BenHenning for code owner reviews. Thanks!

Code Coverage M1 edge case, added arg names to specifically pick the …

d1c07cd

…args to and make reordering possible

Rd4dev requested a review from a team as a code owner June 28, 2024 16:24

Rd4dev requested a review from BenHenning June 28, 2024 16:24

Modified script example doc and updated the exception string to be mo…

b26ac35

…re descriptive

oppiabot bot assigned adhiamboperes Jun 28, 2024

Rd4dev self-assigned this Jun 28, 2024

Added tests for edge case checks

b89d9a8

Increasing the processTimeout to 10 minutes as coverage analysis take…

23cba86

… a long time causing TIMEOUT failures

adhiamboperes approved these changes Jun 28, 2024

View reviewed changes

oppiabot bot unassigned adhiamboperes Jun 28, 2024

oppiabot bot assigned BenHenning Jun 28, 2024

adhiamboperes assigned seanlip and unassigned BenHenning Jun 28, 2024

Rd4dev added 2 commits June 29, 2024 02:22

Setting processTimeout in the run coverage script to see if this affe…

e24578e

…cts the TIMEOUT

Set processTimeout for added test cases

96c2b74

adhiamboperes unassigned seanlip Jun 29, 2024

Rd4dev added 3 commits June 29, 2024 08:24

Triggering Build to check TIMEOUT

349be2e

Reverting the changes just to check if these changes have any effect …

300fb90

…on the tests

Reverting the changes just to check if these changes have any effect …

181e8b2

…on the tests adding missed paranthesis

Rd4dev added 2 commits June 29, 2024 09:29

Re-introducing the changes while removing the ignore case check as we…

87926ad

… then convert them to uppercase

Re-introducing the test changes with the passed ci check with ignore …

718b065

…case removal

Add size large to RunCoverageTest to set timeout to 900 seconds

75e28e2

Rd4dev requested a review from a team as a code owner July 11, 2024 14:11

Rd4dev requested a review from BenHenning July 11, 2024 14:11

Rd4dev assigned BenHenning and adhiamboperes and unassigned Rd4dev Jul 11, 2024

adhiamboperes reviewed Jul 11, 2024

View reviewed changes

scripts/src/java/org/oppia/android/scripts/testing/TestBazelWorkspace.kt Show resolved Hide resolved

adhiamboperes assigned Rd4dev and unassigned BenHenning and adhiamboperes Jul 11, 2024

Updated processTimeout to set as standard 5 minutes and moved outCont…

d3eb372

…ent specific to the test case

Rd4dev added 4 commits July 12, 2024 20:20

Retriggering since localTestHtmlFormat pass locally and to confirm if…

6710f66

… its not a flake

Adding shard_count to see if that is making any difference

756bed8

Addition of dividing tests with shards did help with ci checks, now r…

ab60123

…emoving it to see if that is actually causing the issues (may be with idle times)

Adding back shards since that significantly lessens the test run time…

d25ea9c

… most importantly helps with max idle run time issues

oppiabot bot assigned adhiamboperes and unassigned Rd4dev Jul 12, 2024

adhiamboperes approved these changes Jul 12, 2024

View reviewed changes

oppiabot bot unassigned adhiamboperes Jul 12, 2024

oppiabot bot assigned BenHenning Jul 12, 2024

adhiamboperes added the PR: Review post-merge The PR contains changes that may need to be verified post-merge label Jul 12, 2024

adhiamboperes merged commit b33ca1c into oppia:develop Jul 12, 2024
25 checks passed

Fix part of #5343: Code Coverage script edge cases #5453

Fix part of #5343: Code Coverage script edge cases #5453

Conversation

Rd4dev commented Jun 28, 2024 • edited Loading

Explanation

Project

Changes Made

Essential Checklist

For UI-specific PRs only

Rd4dev commented Jun 28, 2024

Rd4dev commented Jun 28, 2024

adhiamboperes left a comment • edited Loading

Choose a reason for hiding this comment

Rd4dev commented Jun 28, 2024

oppiabot bot commented Jun 28, 2024

oppiabot bot commented Jun 28, 2024

adhiamboperes commented Jun 28, 2024

adhiamboperes commented Jun 29, 2024

github-actions bot commented Jun 29, 2024

APK & AAB differences analysis

Dev

Universal APK

AAB differences

Base APK

Configuration hdpi

Configuration ldpi

Configuration mdpi

Configuration tvdpi

Configuration xhdpi

Configuration xxhdpi

Configuration xxxhdpi

Alpha

Universal APK

AAB differences

Base APK

Configuration hdpi

Configuration ldpi

Configuration mdpi

Configuration tvdpi

Configuration xhdpi

Configuration xxhdpi

Configuration xxxhdpi

Beta

Universal APK

AAB differences

Base APK

Configuration hdpi

Configuration ldpi

Configuration mdpi

Configuration tvdpi

Configuration xhdpi

Configuration xxhdpi

Configuration xxxhdpi

Ga

Universal APK

AAB differences

Base APK

Configuration hdpi

Configuration ldpi

Configuration mdpi

Configuration tvdpi

Configuration xhdpi

Configuration xxhdpi

Configuration xxxhdpi

Rd4dev commented Jun 29, 2024 • edited Loading

Passing locally:

Fix

Rd4dev commented Jul 11, 2024 • edited Loading

adhiamboperes commented Jul 11, 2024

github-actions bot commented Jul 12, 2024

APK & AAB differences analysis

Dev

Universal APK

AAB differences

Base APK

Configuration hdpi

Configuration ldpi

Configuration mdpi

Configuration tvdpi

Configuration xhdpi

Rd4dev commented Jun 28, 2024 •

edited

Loading

adhiamboperes left a comment •

edited

Loading

Rd4dev commented Jun 29, 2024 •

edited

Loading

Rd4dev commented Jul 11, 2024 •

edited

Loading