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

[web] increase number of shards. sync engine web tests same as flutter repo #20164

Merged
merged 3 commits into from
Aug 1, 2020

Conversation

nturgut
Copy link
Contributor

@nturgut nturgut commented Jul 30, 2020

increase number of shards. sync engine web tests same as flutter repo tests. Similar framework PR: flutter/flutter#62623

Related Issues

flutter/flutter#62510

Tests

Tests run on cirrus ci.

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

  • I read the [contributor guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [C++, Objective-C, Java style guides] for the engine.
  • I read the [tree hygiene] wiki page, which explains my responsibilities.
  • I updated/added relevant documentation.
  • All existing and new tests are passing.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

No.

.cirrus.yml Outdated
CPU: 4
MEMORY: 16G
WEB_SHARD_COUNT: 4
CHROME_NO_SANDBOX: true
GOLD_SERVICE_ACCOUNT: ENCRYPTED[3afeea5ac7201151c3d0dc9648862f0462b5e4f55dc600ca8b692319622f7c3eda3d577b1b16cc2ef0311b7314c1c095]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Piinks Do you know what GOLD_SERVICE_ACCOUNT is used for, this was added to web shards for flutter/flutter but was missing on the engine side, so I'm also adding this value here.

Copy link
Contributor

Choose a reason for hiding this comment

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

If the tests are passing without it, it's probably not needed here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, seems like they failed. so I'll try another run without it.

Copy link
Contributor

Choose a reason for hiding this comment

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

What are we missing to migrate this tests to LUCI?

Copy link
Contributor

@mdebbar mdebbar left a comment

Choose a reason for hiding this comment

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

LGTM

@nturgut
Copy link
Contributor Author

nturgut commented Jul 30, 2020

/cc @christopherfujino We have an interesting example of bandwidth consumption here :) I'm changing a configuration on .cirrus.yml file but all LUCI tests are still triggered. Where should I change, for these type of changes to not trigger LUCI?

@christopherfujino
Copy link
Member

/cc @christopherfujino We have an interesting example of bandwidth consumption here :) I'm changing a configuration on .cirrus.yml file but all LUCI tests are still triggered. Where should I change, for these type of changes to not trigger LUCI?

Scheduling of LUCI try jobs happens here https://github.com/flutter/cocoon/blob/master/app_dart/lib/src/request_handlers/github_webhook.dart#L162. We currently don't look at all at which files are affected. You could double check, but at first glance it doesn't look like the PR webhook provides any information on the affected files https://docs.github.com/en/developers/webhooks-and-events/webhook-events-and-payloads#pull_request or https://docs.github.com/en/rest/reference/pulls. It looks like to conditionally trigger try jobs, we'd need to do an additional request (or use the graphql api) and pull in all the commits of the PR, and then all the files.

@godofredoc
Copy link
Contributor

It looks like to conditionally trigger try jobs,
That's something that I forgot to add to the migration design doc, the initial pull request contains the list of files affected in the PR but we still need to expand the json/yaml file to include the filters.

@nturgut
Copy link
Contributor Author

nturgut commented Aug 1, 2020

I'm merging this PR since tests run without issues. Flutter framework tree is closed now, therefore I'll merge the framework PR later.

I'll keep an eye on this Cirrus tests to see if they reduced the flakiness.

@nturgut nturgut merged commit cb4bb93 into flutter:master Aug 1, 2020
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Aug 3, 2020
zanderso pushed a commit to flutter/flutter that referenced this pull request Aug 3, 2020
* 7f5d044 Wait before switching surfaces (flutter/engine#20100)

* 5513273 Reland: Avoid a copy in EncodeImage (flutter/engine#20003)

* 1efdd95 Roll Dart SDK from bd528bfbd69d to ea6bde577d1c (19 revisions) (flutter/engine#20172)

* 3b0e697 Roll Skia from 8cc118dce813 to c3794dd52778 (27 revisions) (flutter/engine#20173)

* cb1a374 Roll Fuchsia Mac SDK from T2xc0OuiK... to i0zTcQ8Qb... (flutter/engine#20175)

* ed36b1a Roll Skia from c3794dd52778 to 2d01ed94605a (10 revisions) (flutter/engine#20179)

* fcc1eaf Fix iOS Keyboard stuck as UIKeyboardTypeNamePhonePad (flutter/engine#20181)

* 9c6837c Roll Skia from 2d01ed94605a to 7225788b9070 (6 revisions) (flutter/engine#20183)

* 13e993e Fix Typos (flutter/engine#19691)

* 98cfd1d Move platform specific information to `PlatformConfiguration` class (flutter/engine#19652)

* 22fb58b update nullability of drawAtlas methods and flesh out docs (flutter/engine#20176)

* bcc43df Roll Dart SDK from ea6bde577d1c to 033a81d924b9 (23 revisions) (flutter/engine#20186)

* cb4bb93 [web] increase number of shards. sync engine web tests same as flutter repo (flutter/engine#20164)

* d986b8d Enable linting in several files (flutter/engine#20134)

* 7dd092d Enable more linting (flutter/engine#20187)

* 3cc86ac Roll Dart SDK from 033a81d924b9 to ad5bcf16f1c8 (9 revisions) (flutter/engine#20191)

* 5ca8a2a Roll Dart SDK from ad5bcf16f1c8 to d169af6f7d8f (1 revision) (flutter/engine#20192)

* 4de0c04 Roll Dart SDK from d169af6f7d8f to 7e6c55e3aaf5 (1 revision) (flutter/engine#20196)

* 908fe01 Fix navigation message relay. (flutter/engine#20193)

* f1b3b69 Roll Dart SDK from 7e6c55e3aaf5 to 365525432a70 (2 revisions) (flutter/engine#20197)

* 8fbdd3f Fix parameter names

* 083282e Fix Implments typo
@liyuqian
Copy link
Contributor

liyuqian commented Aug 4, 2020

@nturgut : engine Cirrus tests run in our own GKE cluster and there's a different resource limit than the framework Cirrus tests. Adding ~10 shards per commit will exhaust our resources and make engine commit presubmit tests to have a long pending time. Are we seeing similar flaky results (flutter/flutter#62510) in engine Cirrus tests before this PR, or is there another critical reason to add those ~10 shards? ? CC @yjbanov who originally helped bring the shard number down.

@nturgut
Copy link
Contributor Author

nturgut commented Aug 4, 2020

@nturgut : engine Cirrus tests run in our own GKE cluster and there's a different resource limit than the framework Cirrus tests. Adding ~10 shards per commit will exhaust our resources and make engine commit presubmit tests to have a long pending time. Are we seeing similar flaky results (flutter/flutter#62510) in engine Cirrus tests before this PR, or is there another critical reason to add those ~10 shards? ? CC @yjbanov who originally helped bring the shard number down.

@tvolkert Increasing the shard number helped me to avoid the issue flutter/flutter#62510 However it looks like it exhaust the Cirrus limits. What should I do? Since #62510 is a P2, I don't have any other solutions at hand.

@nturgut
Copy link
Contributor Author

nturgut commented Aug 4, 2020

@nturgut : engine Cirrus tests run in our own GKE cluster and there's a different resource limit than the framework Cirrus tests. Adding ~10 shards per commit will exhaust our resources and make engine commit presubmit tests to have a long pending time. Are we seeing similar flaky results (flutter/flutter#62510) in engine Cirrus tests before this PR, or is there another critical reason to add those ~10 shards? ? CC @yjbanov who originally helped bring the shard number down.

@yjbanov suggested Dart team to increase the number of shards for the same issue: flutter/flutter#61462

@liyuqian
Copy link
Contributor

liyuqian commented Aug 4, 2020

Discussed with @nturgut offline. We're only exhausting the engine Cirrus limits here so having a high shard number in the framework Cirrus tests to solve P2 flutter/flutter#62510 is still fine. We're going to bring down the engine shard number to 8 to see if we can strike a balance between flakiness and resource limits. After all, it seems that the engine tests aren't that flaky even if the shard number was 4.

Pragya007 pushed a commit to Pragya007/flutter that referenced this pull request Aug 11, 2020
* 7f5d044 Wait before switching surfaces (flutter/engine#20100)

* 5513273 Reland: Avoid a copy in EncodeImage (flutter/engine#20003)

* 1efdd95 Roll Dart SDK from bd528bfbd69d to ea6bde577d1c (19 revisions) (flutter/engine#20172)

* 3b0e697 Roll Skia from 8cc118dce813 to c3794dd52778 (27 revisions) (flutter/engine#20173)

* cb1a374 Roll Fuchsia Mac SDK from T2xc0OuiK... to i0zTcQ8Qb... (flutter/engine#20175)

* ed36b1a Roll Skia from c3794dd52778 to 2d01ed94605a (10 revisions) (flutter/engine#20179)

* fcc1eaf Fix iOS Keyboard stuck as UIKeyboardTypeNamePhonePad (flutter/engine#20181)

* 9c6837c Roll Skia from 2d01ed94605a to 7225788b9070 (6 revisions) (flutter/engine#20183)

* 13e993e Fix Typos (flutter/engine#19691)

* 98cfd1d Move platform specific information to `PlatformConfiguration` class (flutter/engine#19652)

* 22fb58b update nullability of drawAtlas methods and flesh out docs (flutter/engine#20176)

* bcc43df Roll Dart SDK from ea6bde577d1c to 033a81d924b9 (23 revisions) (flutter/engine#20186)

* cb4bb93 [web] increase number of shards. sync engine web tests same as flutter repo (flutter/engine#20164)

* d986b8d Enable linting in several files (flutter/engine#20134)

* 7dd092d Enable more linting (flutter/engine#20187)

* 3cc86ac Roll Dart SDK from 033a81d924b9 to ad5bcf16f1c8 (9 revisions) (flutter/engine#20191)

* 5ca8a2a Roll Dart SDK from ad5bcf16f1c8 to d169af6f7d8f (1 revision) (flutter/engine#20192)

* 4de0c04 Roll Dart SDK from d169af6f7d8f to 7e6c55e3aaf5 (1 revision) (flutter/engine#20196)

* 908fe01 Fix navigation message relay. (flutter/engine#20193)

* f1b3b69 Roll Dart SDK from 7e6c55e3aaf5 to 365525432a70 (2 revisions) (flutter/engine#20197)

* 8fbdd3f Fix parameter names

* 083282e Fix Implments typo
mingwandroid pushed a commit to mingwandroid/flutter that referenced this pull request Sep 6, 2020
* 7f5d044 Wait before switching surfaces (flutter/engine#20100)

* 5513273 Reland: Avoid a copy in EncodeImage (flutter/engine#20003)

* 1efdd95 Roll Dart SDK from bd528bfbd69d to ea6bde577d1c (19 revisions) (flutter/engine#20172)

* 3b0e697 Roll Skia from 8cc118dce813 to c3794dd52778 (27 revisions) (flutter/engine#20173)

* cb1a374 Roll Fuchsia Mac SDK from T2xc0OuiK... to i0zTcQ8Qb... (flutter/engine#20175)

* ed36b1a Roll Skia from c3794dd52778 to 2d01ed94605a (10 revisions) (flutter/engine#20179)

* fcc1eaf Fix iOS Keyboard stuck as UIKeyboardTypeNamePhonePad (flutter/engine#20181)

* 9c6837c Roll Skia from 2d01ed94605a to 7225788b9070 (6 revisions) (flutter/engine#20183)

* 13e993e Fix Typos (flutter/engine#19691)

* 98cfd1d Move platform specific information to `PlatformConfiguration` class (flutter/engine#19652)

* 22fb58b update nullability of drawAtlas methods and flesh out docs (flutter/engine#20176)

* bcc43df Roll Dart SDK from ea6bde577d1c to 033a81d924b9 (23 revisions) (flutter/engine#20186)

* cb4bb93 [web] increase number of shards. sync engine web tests same as flutter repo (flutter/engine#20164)

* d986b8d Enable linting in several files (flutter/engine#20134)

* 7dd092d Enable more linting (flutter/engine#20187)

* 3cc86ac Roll Dart SDK from 033a81d924b9 to ad5bcf16f1c8 (9 revisions) (flutter/engine#20191)

* 5ca8a2a Roll Dart SDK from ad5bcf16f1c8 to d169af6f7d8f (1 revision) (flutter/engine#20192)

* 4de0c04 Roll Dart SDK from d169af6f7d8f to 7e6c55e3aaf5 (1 revision) (flutter/engine#20196)

* 908fe01 Fix navigation message relay. (flutter/engine#20193)

* f1b3b69 Roll Dart SDK from 7e6c55e3aaf5 to 365525432a70 (2 revisions) (flutter/engine#20197)

* 8fbdd3f Fix parameter names

* 083282e Fix Implments typo
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants