Skip to content

Conversation

@jonahwilliams
Copy link
Contributor

Description

These devicelab tests are incredibly flaky. Currently they will build & install the same application 15 times. This causes the temp storage to fill up on android, and has a good chance of flaking on iOS due to install issues.

The change from 3 to 15 increased total test time for 2 -> 12 minutes on iOS, or roughly 5 more test equivalents for every single iOS test. Reduce the iteration count back to 5

Uninstall the app after each run so temp storage does not fill up.

@flutter-dashboard flutter-dashboard bot added the c: contributor-productivity Team-specific productivity, code health, technical debt. label Oct 19, 2020
@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie on the #hackers channel in Chat.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.


const int iterations = 15;
final Device device = await devices.workingDevice;
const int iterations = 5;
Copy link
Member

Choose a reason for hiding this comment

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

\cc @liyuqian re: #67147
Upping from 3 and then 15 significantly increased the amount of time it took this test to run.

We also suspect it's the cause for the temp storage filling up on Android (hence the uninstall at the end of every loop).

Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

LGTM
@liyuqian can comment on the concrete impact of the increased data average noice.
Even if it NEEDS to be left at 15 for some reason, the rest seems like a good improvement.

@jonahwilliams
Copy link
Contributor Author

if it needs to be 15, then it should be restricted to one important test case. Right now this effects 11 tests, so its a large resource increase to the devicelab

@liyuqian
Copy link
Contributor

liyuqian commented Oct 19, 2020

All start_up tests seem to be quite noisy to me, and they're of similar importance. I think the best way forward is to reduce the iteration time so we can do a startup iteration in ~10 second so maybe we can do 50 iterations to further reduce the noise without depleting the device lab.

@jonahwilliams
Copy link
Contributor Author

We're never going to be able to do an iteration in 10 seconds

@jmagman
Copy link
Member

jmagman commented Oct 19, 2020

For fun I ran this change on an iPhone 6 with 15 iterations and it reduced complex_layout_ios__start_up from 6:47 to 43 seconds.

@jonahwilliams
Copy link
Contributor Author

Additionally, even if our launch was 99% reliable, running it 50 times would mean that at least one run would fail about 40% of the time.

@jonahwilliams jonahwilliams merged commit a4c2075 into flutter:master Oct 19, 2020
@jonahwilliams jonahwilliams deleted the reduce_iterations branch October 19, 2020 21:16
@liyuqian
Copy link
Contributor

liyuqian commented Oct 19, 2020

For fun I ran this change on an iPhone 6 with 15 iterations and it reduced complex_layout_ios__start_up from 6:47 to 43 seconds.

This looks promising. If I understand it correctly, this is 2.87 seconds/iteration which is much faster than my 10 seconds/iteration goal?

Additionally, even if our launch was 99% reliable, running it 50 times would mean that at least one run would fail about 40% of the time.

We could write the benchmark in a way to ignore a certain proportion of failures and only use the successful ones to compute the average. But as a developer, I'd really love to increase the reliability as much as possible (e.g., flutter run should retry certain fragile steps to make the whole command fail less often). The 1% failure may still cost a developer headaches and loss of time.

jonahwilliams pushed a commit that referenced this pull request Oct 19, 2020
…plication-binary in all startup tests (#68516)"

This reverts commit a4c2075.
jonahwilliams pushed a commit that referenced this pull request Oct 19, 2020
…plication-binary in all startup tests (#68516)" (#68531)

This reverts commit a4c2075.
@jonahwilliams jonahwilliams restored the reduce_iterations branch October 19, 2020 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants