Migrate customer_testing to sharded tests. #138659

godofredoc · 2023-11-18T02:05:58Z

This will allow Dart Team to run customer tests as part of monorepo and will be a step forward to remove ad_hoc tests.

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

christopherfujino · 2023-11-20T22:57:32Z

dev/bots/test.dart

looks like git checkout HEAD is a no-op https://github.com/git/git/blob/ad7044857660af7ffaaf8fbbccc77b817d1b938f/builtin/checkout.c#L624 (HEAD is what we have currently checked out), thus let's only run the command if revision != null:

Suggested change

final String revision = env['REVISION'] ?? 'HEAD';

await runCommand(

'git',

<String>[

'checkout',

revision,

],

workingDirectory: flutterRoot,

);

final String? revision = env['REVISION'];

if (revision != null) {

await runCommand(

'git',

const <String>[

'checkout',

revision,

],

workingDirectory: flutterRoot,

);

}

in what cases would env['REVISION'] not be set? just when run locally?

looks like git checkout HEAD is a no-op (HEAD is what we have currently checked out), thus let's only run the command if revision != null:

Applied the suggestion.

christopherfujino · 2023-11-20T23:02:22Z

TESTOWNERS

is this change effectively commenting out the customer_testing test owner logic? if so, why? if not, how does this work?

Test owners validation is using "# " for shard validations.

christopherfujino · 2023-11-20T23:04:24Z

.ci.yaml

so, we're expecting presubmits to not reflect this change, because compiling ci.yaml to lucicfg configs happens post-submit, right?

That is correct, I've been running them manually with led https://luci-milo.appspot.com/raw/build/logs.chromium.org/flutter/led/godofredoc_google.com/c8307d6b5bb5f03b08393c1a5e1aaedc9ef2c2064b14521627832c38c09cc26a/+/build.proto

This is the run for mac: https://chromium-swarm.appspot.com/task?id=6607d9ba068fc310

And this one for win: https://chromium-swarm.appspot.com/task?id=6607dbd699281310

looks like win is failing:

║ ProcessException: The system cannot find the file specified. ║ ║ Command: ci.bat ║ #0 _ProcessImpl._start (dart:io-patch/process_patch.dart:402:33) ║ #1 Process.start (dart:io-patch/process_patch.dart:38:20) ║ #2 startCommand (file:///C:/b/s/w/ir/x/w/flutter/dev/bots/run_command.dart:97:47) ║ #3 runCommand (file:///C:/b/s/w/ir/x/w/flutter/dev/bots/run_command.dart:165:33) ║ #4 _runCustomerTesting (file:///C:/b/s/w/ir/x/w/flutter/dev/bots/test.dart:1583:9) ║ <asynchronous suspension> ║ #5 _runFromList (file:///C:/b/s/w/ir/x/w/flutter/dev/bots/test.dart:2175:5) ║ <asynchronous suspension> ║ #6 main (file:///C:/b/s/w/ir/x/w/flutter/dev/bots/test.dart:247:5) ║ <asynchronous suspension> ║ ║ The test.dart script should be corrected to catch this error and call foundError(). ║ Some tests are likely to have been skipped.

Taking a look, I realized I didn't run on win before.

The problem was that windows wants bat files to be executed using full path rather than a relative one.

This will allow Dart Team to run customer tests as part of monorepo. Bug: dart-lang/sdk#51042

christopherfujino · 2023-11-20T23:08:27Z

dev/customer_testing/ci.bat

can you explain this diff? i have no batch foo.

oh wait, nvm, i see the diff on find_commit.dart

Is sending two more parameters to find_commit.

find_commit <first repo checkout in this case flutter/flutter> <branch of first repo (master)> <second repo checkout in this case flutter/tests> <branch of second repo(main)>

ahh i see, confusingly in the old version, the first arg was optional. this LGTM

christopherfujino

LGTM, other than the failing windows LED build.

christopherfujino · 2023-11-20T23:31:06Z

dev/customer_testing/ci.bat

ahh i see, confusingly in the old version, the first arg was optional. this LGTM

…38659)

This will allow Dart Team to run customer tests as part of monorepo and will be a step forward to remove ad_hoc tests. Bug: dart-lang/sdk#51042 Bug: flutter#115476

The migration of customer tests to sharded tests adds a step that checks out the current tip-of-tree of the framework repo, removing local changes. This does not work with monorepo testing, which modifies engine.version, and does not work with local testing of a branch. The sharded tests should already be running with the correct checkout of the framework repo. If the REVISION environment variable is set, the framework checkout will still be reset to check out that revision. These commands were migrated from the existing shell script to the sharded tester in flutter#138659 Bug: dart-lang/sdk#51042

whesse · 2024-01-05T10:21:48Z

The command that checks out HEAD of the framework repo if no REVISON env variable is set should not be needed when running as a sharded test.
The sharded tests already have the framework repo checked out with the right contents. In the case of monorepo testing, and for local testing of changes, there are local changes to the checkout that must not be deleted before running tests.
These cause the test to fail, when the checkout of HEAD conflicts with them, or else are improperly overridden.
In the monorepo testing, engine.version is modified to check out the correct artifacts from the custom path.

I have uploaded a PR to remove this as #141013

The migration of customer tests to sharded tests adds a step that checks out the current tip-of-tree of the framework repo, removing local changes. This does not work with monorepo testing, which modifies engine.version, and does not work with local testing of a branch. The sharded tests should already be running with the correct checkout of the framework repo. If the REVISION environment variable is set, the framework checkout will still be reset to check out that revision. These commands were migrated from the existing shell script to the sharded tester in #138659 Bug: dart-lang/sdk#51042

…38659)

godofredoc requested review from HansMuller and christopherfujino November 20, 2023 22:55

christopherfujino reviewed Nov 20, 2023

View reviewed changes

godofredoc added 8 commits November 20, 2023 15:07

Migrate customer_testing to sharded tests.

69b3566

This will allow Dart Team to run customer tests as part of monorepo. Bug: dart-lang/sdk#51042

Update test owners file.

37cc7cb

Use env variable as a constant.

37c3213

Update find_commit.

3a9f1a0

Ensure env is calculated outside of run command.

a6812ce

Use platform to get envs.

fe79064

Fix conditional assignment.

a55383f

Apply code review suggestion.

4ca92b3

christopherfujino reviewed Nov 20, 2023

View reviewed changes

christopherfujino approved these changes Nov 20, 2023

View reviewed changes

dev/customer_testing/ci.bat Outdated

Copy link

Contributor

christopherfujino Nov 20, 2023

Choose a reason for hiding this comment

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

ahh i see, confusingly in the old version, the first arg was optional. this LGTM

godofredoc added 10 commits November 20, 2023 16:04

Ensure ci.bat is run from current directory.

5fff776

Fix run_command relative path calculation.

d9dce22

Fix relative path.

2f57395

Fix relative working directory in startCommand.

e3b204d

Add logs.

71d71e2

Update windows command.

b0572a1

Fix command.

a39c43f

Use cmd.

3a56b46

Try powershell.

0297d98

Use full path to win script.

df88fc4

goderbauer added framework flutter/packages/flutter repository. See also f: labels. team-infra Owned by Infrastructure team labels Nov 21, 2023

godofredoc added 2 commits November 27, 2023 10:01

Merge branch 'master' into migrate_customer_testing

c5c9f15

Merge branch 'master' into migrate_customer_testing

7d17ba0

godofredoc added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 27, 2023

godofredoc added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 28, 2023

auto-submit bot merged commit fe7299d into flutter:master Nov 28, 2023

engine-flutter-autoroll mentioned this pull request Nov 28, 2023

Roll Flutter from 6bf3ccd47d5d to 7794715a4548 (19 revisions) flutter/packages#5502

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 28, 2023

fe7299d Migrate customer_testing to sharded tests. (flutter/flutter#1…

d814ac7

…38659)

engine-flutter-autoroll mentioned this pull request Nov 28, 2023

Roll Flutter from 6bf3ccd47d5d to 91274cf60030 (20 revisions) flutter/packages#5504

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 28, 2023

fe7299d Migrate customer_testing to sharded tests. (flutter/flutter#1…

30864a6

…38659)

engine-flutter-autoroll mentioned this pull request Nov 28, 2023

Roll Flutter from 6bf3ccd47d5d to edf278658ba3 (21 revisions) flutter/packages#5506

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 28, 2023

fe7299d Migrate customer_testing to sharded tests. (flutter/flutter#1…

d4fba65

…38659)

engine-flutter-autoroll mentioned this pull request Nov 28, 2023

Roll Flutter from 6bf3ccd47d5d to 358e39137f3e (23 revisions) flutter/packages#5507

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 28, 2023

fe7299d Migrate customer_testing to sharded tests. (flutter/flutter#1…

03c90c4

…38659)

engine-flutter-autoroll mentioned this pull request Nov 28, 2023

Roll Flutter from 6bf3ccd47d5d to 92ad558e257d (24 revisions) flutter/packages#5509

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 28, 2023

fe7299d Migrate customer_testing to sharded tests. (flutter/flutter#1…

09638cc

…38659)

engine-flutter-autoroll mentioned this pull request Nov 28, 2023

Roll Flutter from 6bf3ccd47d5d to 89edac404184 (25 revisions) flutter/packages#5510

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 28, 2023

fe7299d Migrate customer_testing to sharded tests. (flutter/flutter#1…

6fdb5c5

…38659)

engine-flutter-autoroll mentioned this pull request Nov 28, 2023

Roll Flutter from 6bf3ccd47d5d to 18b239fbde3a (26 revisions) flutter/packages#5511

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 28, 2023

fe7299d Migrate customer_testing to sharded tests. (flutter/flutter#1…

f7ef463

…38659)

engine-flutter-autoroll mentioned this pull request Nov 29, 2023

Manual roll Flutter from 6bf3ccd47d5d to 20eb3ea1318f (39 revisions) flutter/packages#5513

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 29, 2023

fe7299d Migrate customer_testing to sharded tests. (flutter/flutter#1…

f82b880

…38659)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 29, 2023

fe7299d Migrate customer_testing to sharded tests. (flutter/flutter#1…

68a06fc

…38659)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 29, 2023

fe7299d Migrate customer_testing to sharded tests. (flutter/flutter#1…

dd4e5e3

…38659)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 29, 2023

fe7299d Migrate customer_testing to sharded tests. (flutter/flutter#1…

a2686ea

…38659)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 29, 2023

fe7299d Migrate customer_testing to sharded tests. (flutter/flutter#1…

4062fc3

…38659)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 29, 2023

fe7299d Migrate customer_testing to sharded tests. (flutter/flutter#1…

00f343f

…38659)

whesse mentioned this pull request Jan 5, 2024

Do not reset framework checkout before running customer tests #141013

Merged

8 tasks

godofredoc deleted the migrate_customer_testing branch January 5, 2024 17:45

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 16, 2024

fe7299d Migrate customer_testing to sharded tests. (flutter/flutter#1…

4b02931

…38659)

-  final String revision = env['REVISION'] ?? 'HEAD';
-  await runCommand(
-    'git',
-    <String>[
-      'checkout',
-      revision,
-    ],
-    workingDirectory: flutterRoot,
-  );
+  final String? revision = env['REVISION'];
+  if (revision != null) {
+    await runCommand(
+      'git',
+      const <String>[
+        'checkout',
+        revision,
+      ],
+      workingDirectory: flutterRoot,
+    );
+  }

Migrate customer_testing to sharded tests. #138659

Migrate customer_testing to sharded tests. #138659

Uh oh!

Conversation

godofredoc commented Nov 18, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

christopherfujino Nov 20, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

whesse commented Jan 5, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

godofredoc commented Nov 18, 2023 •

edited

Loading

christopherfujino Nov 20, 2023 •

edited

Loading