Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Parallelize Android unit tests - #30728

Merged
fluttergithubbot merged 8 commits into
flutter-team-archive:mainfrom
blasten:robo_parallel
Feb 5, 2022
Merged

fluttergithubbot merged 8 commits into
flutter-team-archive:mainfrom
blasten:robo_parallel

Conversation

@blasten

@blasten blasten commented Jan 7, 2022

Copy link
Copy Markdown

Helps flutter/flutter#96124

Parallelizes Android unit tests.

Summary:

  • Creates process forks up to the max number of cores.
  • Removes FlutterTestSuite.java. Using a single suite limits the number of concurrent jobs to 1.
  • Uses the Android Junit runner.


@Config(manifest = Config.NONE)
@RunWith(RobolectricTestRunner.class)
@RunWith(AndroidJUnit4.class)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

FlutterViewTest uses Shadow APIs of Robolectric, it's better to keep it using RobolectricTestRunner.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

These specific unit tests are passing in CI. what issue did you find? Robolectric's readme defaults to using AndroidJUnit4 https://github.com/robolectric/robolectric#usage

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This example doesn't use any shadow API of Robolectric. The tests use AndroidJUnit4 also can run on real Emulator/device, and shared with sharedTest pattern. If those tests will not run on real Emulator, it's fine to use AndroidJUnit4. If yes, we should avoid use shadow APIs for tests use AndroidJUnit4, or keep it to use RobolectricTestRunner for shadow APIs usage. I know there are some tests still use ShadowDisplay to control display rotation for tests.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh, it's my fault. It uses Robolectric API. It's better to update it with ActivityScenario to lunch Activity, instead of current Robolectric.setupActivity in example. I will send a PR for it at Robolectric to discuss it.

@chinmaygarde

Copy link
Copy Markdown
Contributor

cc @jason-simmons

@chinmaygarde

Copy link
Copy Markdown
Contributor

@blasten Can you fix the conflict and land this please?

@chinmaygarde

chinmaygarde commented Feb 3, 2022 •

Copy link
Copy Markdown
Contributor

Any updates on fixing the conflict?

@blasten

blasten commented Feb 3, 2022

Copy link
Copy Markdown
Author

It might worth to introduce a label or something to denote that a PR has been deprioritized due to other higher priority work, so it can be skipped easily in PR triages? @chinmaygarde

@blasten blasten added the Work in progress (WIP) Not ready (yet) for review! label Feb 3, 2022
@chinmaygarde

Copy link
Copy Markdown
Contributor

The WIP tag you added is the way to go. OTOH. If no work is likely, you could also just close it and reopen later if it ever gets prioritized.

@blasten blasten added waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land. and removed Work in progress (WIP) Not ready (yet) for review! labels Feb 4, 2022
@fluttergithubbot
fluttergithubbot merged commit c058dd1 into flutter-team-archive:main Feb 5, 2022
@blasten
blasten deleted the robo_parallel branch February 5, 2022 01:45
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

platform-android waiting for tree to go green This PR is approved and tested, but waiting for the tree to be green to land.

Development

Successfully merging this pull request may close these issues.

4 participants