Skip to content

[ci] Add LUCI web platform tests #4391

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

Merged
merged 3 commits into from
Jul 7, 2023

Conversation

stuartmorgan-g
Copy link
Contributor

Adds web platform tests for LUCI in bringup mode.

The Cirrus version of the test starts chromedriver at the beginning and just leaves it running, which is fine since it's a fresh docker image each run. For LUCI we don't want that behavior though, so instead this adds tooling support for running chromedriver as part of running the integration tests, controllable with a flag. This will also be useful for running locally.

Part of flutter/flutter#114373

@stuartmorgan-g stuartmorgan-g changed the title [ci] Add LUIC web platform tests [ci] Add LUCI web platform tests Jul 6, 2023
@stuartmorgan-g stuartmorgan-g force-pushed the luci-web-platform-initial branch from ce65413 to d57cd74 Compare July 6, 2023 20:21
@stuartmorgan-g
Copy link
Contributor Author

@ditman FYI on the new --run-chromedriver flag for drive-examples, which may be useful for you.

.ci.yaml Outdated
recipe: packages/packages
timeout: 60
properties:
add_recipes_cq: "true"
Copy link
Contributor

Choose a reason for hiding this comment

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

I am wondering if we want to enforce these targets in the recipes CQ. To me, existing packages targets (with add_recipes_cq: true) should be enough to cover recipes change. I would suggest we skip this property here. This way, test specific regression will not block recipes CI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can remove it.

For my future reference, what's the criteria for add_recipes_cq? Is it just a judgement call about whether the test depends on some aspect of the recipe that's not already covered?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, exactly. Since all these packages targets share the same recipes, and it's not necessary to enable all targets from recipes side.

properties:
add_recipes_cq: "true"
target_file: web_platform_tests.yaml
cores: "32"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this necessary for these targets?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

These were in the heavy workload in Cirrus, and even then needed to be sharded. I've been mapping heavy workload to 32 cores based on our previous discussion.

If there's a good way to analyze runs after the fact I'm happy to tune this later based on actual usage.

Copy link
Member

@ditman ditman left a comment

Choose a reason for hiding this comment

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

This LGTM, make CI and @keyonghan happy, and ship it!

Copy link
Contributor

@keyonghan keyonghan left a comment

Choose a reason for hiding this comment

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

LGTM

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 6, 2023
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 6, 2023
@auto-submit
Copy link
Contributor

auto-submit bot commented Jul 6, 2023

auto label is removed for flutter/packages, pr: 4391, due to - The status or check suite android-platform_tests CHANNEL:master PACKAGE_SHARDING:--shardIndex 0 --shardCount 8 has failed. Please fix the issues identified (or deflake) before re-applying this label.

@stuartmorgan-g stuartmorgan-g added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 7, 2023
@auto-submit auto-submit bot merged commit 9bcf4bf into flutter:main Jul 7, 2023
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Jul 7, 2023
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Jul 7, 2023
flutter/packages@771ec9b...9bcf4bf

2023-07-07 [email protected] [ci] Add LUCI web platform tests (flutter/packages#4391)
2023-07-07 [email protected] [webview_flutter] Enable unawaited_futures lint (flutter/packages#4271)
2023-07-07 [email protected] [ci] Add partial LUCI version of repo_checks (flutter/packages#4389)
2023-07-06 49699333+dependabot[bot]@users.noreply.github.com Bump ossf/scorecard-action from 2.1.3 to 2.2.0 (flutter/packages#4302)
2023-07-06 [email protected] [webview_flutter_android][webview_flutter_wkwebview] Fixes unawaited_futures violations (flutter/packages#4354)
2023-07-06 [email protected] [local_auth] Update Windows Pigeon version (flutter/packages#4388)
2023-07-06 [email protected] [url_launcher] Remove nested third_party safari checker (flutter/packages#4330)
2023-07-06 [email protected] [ci] Add partial LUCI Android platform tests (flutter/packages#4381)
2023-07-06 [email protected] [ci] Switch `master` Linux custom package tests to LUCI (flutter/packages#4386)
2023-07-06 [email protected] [go_router] Exposes package-level privates (flutter/packages#4380)
2023-07-06 [email protected] [file_selector] Update to 1.0 (flutter/packages#4362)
2023-07-06 [email protected] Roll Flutter from 35085c3 to bc49cd1 (14 revisions) (flutter/packages#4387)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages-flutter-autoroll
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://bugs.chromium.org/p/skia/issues/entry?template=Autoroller+Bug

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
@ditman
Copy link
Member

ditman commented Jul 12, 2023

I just noticed that this PR added a - file to the root of the repo that probably shouldn't be there :)

@stuartmorgan-g stuartmorgan-g deleted the luci-web-platform-initial branch July 12, 2023 10:08
@stuartmorgan-g
Copy link
Contributor Author

I wonder how I did that. I'll clean it up in the next LUCI PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
autosubmit Merge PR when tree becomes green via auto submit App
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants