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

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

Merged
merged 80 commits into from
Jul 12, 2024

Conversation

Rd4dev
Copy link
Collaborator

@Rd4dev Rd4dev commented Jun 28, 2024

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

@Rd4dev Rd4dev requested a review from a team as a code owner June 28, 2024 16:24
@Rd4dev Rd4dev requested a review from BenHenning June 28, 2024 16:24
@Rd4dev
Copy link
Collaborator Author

Rd4dev commented Jun 28, 2024

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

@Rd4dev Rd4dev self-assigned this Jun 28, 2024
@Rd4dev
Copy link
Collaborator Author

Rd4dev commented Jun 28, 2024

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

Copy link
Collaborator

@adhiamboperes adhiamboperes left a comment

Choose a reason for hiding this comment

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

Thanks @Rd4dev. This LGTM.

@Rd4dev
Copy link
Collaborator Author

Rd4dev commented Jun 28, 2024

@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

Copy link

oppiabot bot commented Jun 28, 2024

Unassigning @adhiamboperes since they have already approved the PR.

Copy link

oppiabot bot commented Jun 28, 2024

Assigning @BenHenning for code owner reviews. Thanks!

@adhiamboperes
Copy link
Collaborator

@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

@adhiamboperes
Copy link
Collaborator

@Rd4dev PTAL for CI failures

Copy link

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
Copy link
Collaborator Author

Rd4dev commented Jun 29, 2024

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:

image

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.
image

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]

@Rd4dev Rd4dev requested a review from a team as a code owner July 11, 2024 14:11
@Rd4dev Rd4dev requested a review from BenHenning July 11, 2024 14:11
@Rd4dev
Copy link
Collaborator Author

Rd4dev commented Jul 11, 2024

@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.

@Rd4dev Rd4dev assigned BenHenning and adhiamboperes and unassigned Rd4dev Jul 11, 2024
@adhiamboperes
Copy link
Collaborator

@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?

Copy link

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)

@Rd4dev
Copy link
Collaborator Author

Rd4dev commented Jul 12, 2024

@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.

@Rd4dev
Copy link
Collaborator Author

Rd4dev commented Jul 12, 2024

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 oppiabot bot assigned adhiamboperes and unassigned Rd4dev Jul 12, 2024
Copy link

oppiabot bot commented Jul 12, 2024

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

Copy link
Collaborator

@adhiamboperes adhiamboperes left a comment

Choose a reason for hiding this comment

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

Thanks @Rd4dev! Latest changes LGTM.

Copy link

oppiabot bot commented Jul 12, 2024

Unassigning @adhiamboperes since they have already approved the PR.

Copy link

oppiabot bot commented Jul 12, 2024

Assigning @BenHenning for code owner reviews. Thanks!

@adhiamboperes adhiamboperes added the PR: Review post-merge The PR contains changes that may need to be verified post-merge label Jul 12, 2024
@adhiamboperes adhiamboperes merged commit b33ca1c into oppia:develop Jul 12, 2024
25 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
PR: Review post-merge The PR contains changes that may need to be verified post-merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants