Initial config for framework_smoke test #42467

zanderso · 2023-07-05T17:12:27Z

As a workaround before the build graph is designed and implemented. The gn step can specify a different --target-dir to avoid conflicts: https://github.com/flutter/engine/blob/main/tools/gn#L952.

zanderso · 2023-06-06T17:36:58Z

I'm a bit confused about how this is working. The v1 recipe does a complete host_debug_unopt build, but this appears to be only building the Dart SDK archive.

The tests are using FLUTTER_STORAGE_BASE_URL to refer to the artifacts, but I would have thought that there wouldn't be a full set of artifacts there since they're not all getting built by this ninja step.

Maybe the tests in the framework repo only need the Dart SDK? If that's the case, then if there's a change in the framework repo which causes the tests below to need more engine artifacts, then that framework change will break the engine tree. To avoid that, I think the engine v2 recipe should match the engine v1 behavior, and build all of the Engine artifacts, and not just the Dart SDK archive.

As far as I understand this is running the framework unit tests, if other artifacts apart from dart are needed I'd expect the tests to go to a different category e.g. widget test.

This also brings the question if this test is adding value to the engine? is there any history of this test identifying issues with the engine?

nvm, I see that widgets shard is already included.

The dart-sdk-linux-x64.zip is downloaded when running flutter update-packages -v, before running the tests.

To confirm for v1 test behavior: do we know what artifacts these two test steps (v1 consists of only these two) consume in addition to the dart sdk?

flutter analyze --flutter-repo (under path flutter)

flutter test (under path flutter/packages/flutter)

If there is no clear answer, does that mean we have to use the --local-engine for the v2 as we do not know what artifacts to upload to gcs? But --local-engine is not recommended for the v2 version per @godofredoc

As one example, I'm surprised that these tests don't use flutter_tester. Maybe @goderbauer knows what artifacts these tests need?

I would expect this to require the dart sdk and the sky_engine package for dart:ui. I am not sure, how the analyzer consumes dart:ui, though.

both dart sdk and sky_engine are downloaded from the newly built/uploaded artifacts (https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8779089292276869393/+/u/download_dependencies/stdout)

This runs widget tests, so I would expect this to also need the flutter_tester binary.

The log shows all tests passed (ignore the warning), though we are not using flutter_tester. Does this mean it has been handled somewhere else, or it is not necessary?

This runs widget tests

Actually this seems not the case. See one log for v1 framework_smoke_tests, where unit tests under cupertino, painting, etc. are also executed. And these are covered under libraries.

both dart sdk and sky_engine are downloaded from the newly built/uploaded artifacts (https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8779089292276869393/+/u/download_dependencies/stdout)

If the build target and archive config only says to build and upload the Dart SDK, why does sky_engine also get built and uploaded?

Ah, I see why. We have another config linux_unopt.json, which builds and uploads sky_engine.

Will this cause contamination between targets? Do we need to separate artifacts based on configs? /cc @godofredoc

The log shows all tests passed (ignore the warning), though we are not using flutter_tester. Does this mean it has been handled somewhere else, or it is not necessary?

I ran one of the tests in that log locally with flutter test -v test/widgets/animated_padding_test.dart and as the log indicates it is running the flutter_tester binary, see the Starting flutter_tester process line in the log below. So, from somewhere it is getting a flutter_tester binary.

Log

% flutter test -v test/widgets/animated_padding_test.dart main [ +7 ms] executing: [/Users/goderbauer/dev/flutter/] git -c log.showSignature=false log -n 1 --pretty=format:%H [ +61 ms] Exit code 0 from: git -c log.showSignature=false log -n 1 --pretty=format:%H [ ] 42cf3e371a27b4882200b1f7ec7657e43a7cf249 [ ] executing: [/Users/goderbauer/dev/flutter/] git tag --points-at 42cf3e371a27b4882200b1f7ec7657e43a7cf249 [ +82 ms] Exit code 0 from: git tag --points-at 42cf3e371a27b4882200b1f7ec7657e43a7cf249 [ +2 ms] executing: [/Users/goderbauer/dev/flutter/] git describe --match *.*.* --long --tags 42cf3e371a27b4882200b1f7ec7657e43a7cf249 [ +85 ms] Exit code 0 from: git describe --match *.*.* --long --tags 42cf3e371a27b4882200b1f7ec7657e43a7cf249 [ ] 3.11.0-19.0.pre-21-g42cf3e371a [ +3 ms] executing: [/Users/goderbauer/dev/flutter/] git symbolic-ref --short HEAD [ +22 ms] Exit code 0 from: git symbolic-ref --short HEAD [ ] main [ +2 ms] executing: sw_vers -productName [ +57 ms] Exit code 0 from: sw_vers -productName [ ] macOS [ ] executing: sw_vers -productVersion [ +48 ms] Exit code 0 from: sw_vers -productVersion [ ] 13.4 [ ] executing: sw_vers -buildVersion [ +50 ms] Exit code 0 from: sw_vers -buildVersion [ ] 22F66 [ ] executing: uname -m [ +33 ms] Exit code 0 from: uname -m [ ] x86_64 [ +13 ms] executing: sysctl hw.optional.arm64 [ +34 ms] Exit code 1 from: sysctl hw.optional.arm64 [ ] sysctl: unknown oid 'hw.optional.arm64' [ +35 ms] executing: [/Users/goderbauer/dev/flutter/] git rev-parse --abbrev-ref --symbolic @{upstream} [ +21 ms] Exit code 0 from: git rev-parse --abbrev-ref --symbolic @{upstream} [ ] origin/main [ ] executing: [/Users/goderbauer/dev/flutter/] git ls-remote --get-url origin [ +51 ms] Exit code 0 from: git ls-remote --get-url origin [ ] [email protected]:goderbauer/flutter.git [ +24 ms] Found 1 files which will be executed as Widget Tests. [ +11 ms] Artifact Instance of 'AndroidGenSnapshotArtifacts' is not required, skipping update. [ ] Artifact Instance of 'AndroidInternalBuildArtifacts' is not required, skipping update. [ ] Artifact Instance of 'IOSEngineArtifacts' is not required, skipping update. [ ] Artifact Instance of 'FlutterWebSdk' is not required, skipping update. [ ] Artifact Instance of 'LegacyCanvasKitRemover' is not required, skipping update. [ +5 ms] Artifact Instance of 'WindowsEngineArtifacts' is not required, skipping update. [ ] Artifact Instance of 'MacOSEngineArtifacts' is not required, skipping update. [ ] Artifact Instance of 'LinuxEngineArtifacts' is not required, skipping update. [ ] Artifact Instance of 'LinuxFuchsiaSDKArtifacts' is not required, skipping update. [ ] Artifact Instance of 'MacOSFuchsiaSDKArtifacts' is not required, skipping update. [ ] Artifact Instance of 'FlutterRunnerSDKArtifacts' is not required, skipping update. [ ] Artifact Instance of 'FlutterRunnerDebugSymbols' is not required, skipping update. [ +92 ms] Artifact Instance of 'MaterialFonts' is not required, skipping update. [ ] Artifact Instance of 'GradleWrapper' is not required, skipping update. [ ] Artifact Instance of 'AndroidGenSnapshotArtifacts' is not required, skipping update. [ ] Artifact Instance of 'AndroidInternalBuildArtifacts' is not required, skipping update. [ ] Artifact Instance of 'IOSEngineArtifacts' is not required, skipping update. [ ] Artifact Instance of 'FlutterWebSdk' is not required, skipping update. [ ] Artifact Instance of 'LegacyCanvasKitRemover' is not required, skipping update. [ ] Artifact Instance of 'FlutterSdk' is not required, skipping update. [ ] Artifact Instance of 'WindowsEngineArtifacts' is not required, skipping update. [ ] Artifact Instance of 'MacOSEngineArtifacts' is not required, skipping update. [ ] Artifact Instance of 'LinuxEngineArtifacts' is not required, skipping update. [ ] Artifact Instance of 'LinuxFuchsiaSDKArtifacts' is not required, skipping update. [ ] Artifact Instance of 'MacOSFuchsiaSDKArtifacts' is not required, skipping update. [ ] Artifact Instance of 'FlutterRunnerSDKArtifacts' is not required, skipping update. [ ] Artifact Instance of 'FlutterRunnerDebugSymbols' is not required, skipping update. [ ] Artifact Instance of 'IosUsbArtifacts' is not required, skipping update. [ ] Artifact Instance of 'IosUsbArtifacts' is not required, skipping update. [ ] Artifact Instance of 'IosUsbArtifacts' is not required, skipping update. [ ] Artifact Instance of 'IosUsbArtifacts' is not required, skipping update. [ ] Artifact Instance of 'IosUsbArtifacts' is not required, skipping update. [ ] Artifact Instance of 'FontSubsetArtifacts' is not required, skipping update. [ ] Artifact Instance of 'PubDependencies' is not required, skipping update. [ +85 ms] Skipping pub get: version match. [ +499 ms] running test package with arguments: [--chain-stack-traces, --, file:///Users/goderbauer/dev/flutter/packages/flutter/test/widgets/animated_padding_test.dart?] 00:00 +0: loading /Users/goderbauer/dev/flutter/packages/flutter/test/widgets/animated_padding_test.dart [ +349 ms] test 0: starting test /Users/goderbauer/dev/flutter/packages/flutter/test/widgets/animated_padding_test.dart [ +9 ms] Discovered flutter_test_config.dart in /Users/goderbauer/dev/flutter/packages/flutter/test [ +4 ms] Compiler will use the following file as its incremental dill file: /var/folders/s2/rdqjsnjx4vq97147345w30x80044f6/T/flutter_tools.Vcjb8k/flutter_test_compiler.Xqa4mu/output.dill [ ] Listening to compiler controller... [ +19 ms] Compiling file:///var/folders/s2/rdqjsnjx4vq97147345w30x80044f6/T/flutter_tools.Vcjb8k/flutter_test_listener.hRyEsZ/listener.dart [ +90 ms] /Users/goderbauer/dev/flutter/bin/cache/dart-sdk/bin/dart --disable-dart-dev /Users/goderbauer/dev/flutter/bin/cache/dart-sdk/bin/snapshots/frontend_server.dart.snapshot --sdk-root /Users/goderbauer/dev/flutter/bin/cache/artifacts/engine/common/flutter_patched_sdk/ --incremental --no-print-incremental-dependencies --target=flutter --experimental-emit-debug-metadata -DFLUTTER_WEB_AUTO_DETECT=true --output-dill /var/folders/s2/rdqjsnjx4vq97147345w30x80044f6/T/flutter_tools.Vcjb8k/flutter_test_compiler.Xqa4mu/output.dill --packages /Users/goderbauer/dev/flutter/packages/flutter/.dart_tool/package_config.json -Ddart.vm.profile=false -Ddart.vm.product=false --enable-asserts --track-widget-creation --initialize-from-dill /Users/goderbauer/dev/flutter/packages/flutter/build/test_cache/build/c075001b96339384a97db4862b8ab8db.cache.dill.track.dill --verbosity=error [ +49 ms] <- compile file:///var/folders/s2/rdqjsnjx4vq97147345w30x80044f6/T/flutter_tools.Vcjb8k/flutter_test_listener.hRyEsZ/listener.dart 00:18 +0: loading /Users/goderbauer/dev/flutter/packages/flutter/test/widgets/animated_padding_test.dart [+18008 ms] <- accept [ ] <- reset [ ] Compiling file:///var/folders/s2/rdqjsnjx4vq97147345w30x80044f6/T/flutter_tools.Vcjb8k/flutter_test_listener.hRyEsZ/listener.dart took 18148ms [ +1 ms] test 0: starting test device [ +6 ms] test 0: awaiting connection to test device [ +2 ms] test 0: VM Service uri is not available [ +7 ms] test 0: test harness socket server is running at port:59126 [ +3 ms] Using this directory for fonts configuration: /var/folders/s2/rdqjsnjx4vq97147345w30x80044f6/T/flutter_tools.Vcjb8k/flutter_test_fonts.UjuSbt [ +1 ms] test 0: Starting flutter_tester process with command=[/Users/goderbauer/dev/flutter/bin/cache/artifacts/engine/darwin-x64/flutter_tester, --disable-vm-service, --enable-checked-mode, --verify-entry-points, --enable-software-rendering, --skia-deterministic-rendering, --enable-dart-profiling, --non-interactive, --use-test-fonts, --disable-asset-fonts, --packages=/Users/goderbauer/dev/flutter/packages/flutter/.dart_tool/package_config.json, --flutter-assets-dir=/Users/goderbauer/dev/flutter/packages/flutter/build/unit_test_assets, /var/folders/s2/rdqjsnjx4vq97147345w30x80044f6/T/flutter_tools.Vcjb8k/flutter_test_listener.hRyEsZ/listener.dart.dill], environment={FLUTTER_TEST: true, FONTCONFIG_FILE: /var/folders/s2/rdqjsnjx4vq97147345w30x80044f6/T/flutter_tools.Vcjb8k/flutter_test_fonts.UjuSbt/fonts.conf, SERVER_PORT: 59126, APP_NAME: flutter, UNIT_TEST_ASSETS: /Users/goderbauer/dev/flutter/packages/flutter/build/unit_test_assets} [ +30 ms] test 0: Started flutter_tester process at pid 68559 00:19 +0: loading /Users/goderbauer/dev/flutter/packages/flutter/test/widgets/animated_padding_test.dart [+1704 ms] test 0: connected to test device, now awaiting test result [ ] test 0: Waiting for test harness or tests to finish 00:21 +3: AnimatedPadding animated padding clamped to positive values [+1505 ms] test 0: Test harness is no longer needed by test process [ ] test 0: finished [ +1 ms] test 0: cleaning up... [ ] test 0: ensuring test device is terminated. [ ] test 0: Terminating flutter_tester process [ +1 ms] test 0: Shutting down DevTools server [ +1 ms] test 0: Test process is no longer needed by test harness [ +3 ms] test 0: Shutting down test harness socket server [ +15 ms] test 0: flutter_tester process at pid 68559 exited with code=-9 [ ] test 0: deleting temporary directory [ +4 ms] test 0: finished 00:21 +3: All tests passed! [ +13 ms] Deleting /var/folders/s2/rdqjsnjx4vq97147345w30x80044f6/T/flutter_tools.Vcjb8k/flutter_test_compiler.Xqa4mu... [ +4 ms] killing pid 68524 [ +100 ms] Deleting /var/folders/s2/rdqjsnjx4vq97147345w30x80044f6/T/flutter_tools.Vcjb8k/flutter_test_fonts.UjuSbt... [ +4 ms] test package returned with exit code 0 [ ] Runtime for phase TestRunner: Wall-clock: 0:00:21.957568; combined: 0:00:21.957662. [ ] Runtime for phase Compile: Wall-clock: 0:00:18.148791; combined: 0:00:18.148880. [ ] Runtime for phase Run: Wall-clock: 0:00:03.260665; combined: 0:00:03.260669. [ ] Runtime for phase CoverageTotal: Wall-clock: 0:00:00.000000; combined: 0:00:00.000000. [ ] Runtime for phase CoverageCollect: Wall-clock: 0:00:00.000000; combined: 0:00:00.000000. [ ] Runtime for phase CoverageParseJson: Wall-clock: 0:00:00.000000; combined: 0:00:00.000000. [ ] Runtime for phase CoverageAddHitmap: Wall-clock: 0:00:00.000000; combined: 0:00:00.000000. [ ] Runtime for phase CoverageDataCollect: Wall-clock: 0:00:00.000000; combined: 0:00:00.000000. [ ] Runtime for phase WatcherFinishedTest: Wall-clock: 0:00:00.000851; combined: 0:00:00.000853. [ +13 ms] "flutter test" took 22,668ms. [ +277 ms] ensureAnalyticsSent: 252ms [ ] Running 1 shutdown hook [ +1 ms] Shutdown hooks complet

Thanks @goderbauer for the explanation. Confirmed using led: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8778925221459687985/+/u/Run_framework_tests_widgets_tests/stdout

Same as sky_engine, the flutter_tester is downloaded via linux-x64/artifacts.zip, which is also built/uploaded in other targets.

zanderso · 2023-06-06T17:38:41Z

It would be really helpful to have comments or links to the test commands that this will actually run. One advantage of the engine v1 recipe was that it was totally unambiguous what commands would be run. For example here: https://flutter.googlesource.com/recipes/+/refs/heads/main/recipes/engine/framework_smoke.py#85

For example, it would be great if the config here was self-documenting, and it was obvious how to add more tests. Right now, these test configs are more-or-less full of magic strings, and it's not clear where to mine more magic strings from.

Json does not support comments. To be able to add comments the configs will need to get migrated to yaml or proto. To mitigate this I added docs to the README file in build configuration directory.

The magic strings are coming from the framework test runner. Unfortunately it has no owner, no structure and zero documentation.

Json does not support comments. To be able to add comments the configs will need to get migrated to yaml or proto. To mitigate this I added docs to the README file in build configuration directory.

This is usually accomplished by adding non-interpreted fields like "_comment".

The magic strings are coming from the framework test runner. Unfortunately it has no owner, no structure and zero documentation.

@keyonghan How did you find the strings that are in this PR?

To be honest, this is not a trivial thing even though I have been relatively familiar with ci.yaml, recipes, and some parts of test runners..

This is how I did:

Identify the tests from v1 framework smoke tests: flutter analyze + flutter test

For flutter analyze

First search across flutter repository test runners for analyze related tests, no good match

Then search existing flutter post-submit tests/builders/targets which are related to analyze, find Linux analyzer_benchmark and Linux analyze

Looking into these two analyze tests, I find Linux analyze is the closest, where flutter analyze --flutter-repo is covered.

Based on the recipes which Linux analyze is using, I find validation related properties/strings

For flutter test

Based on the path this test runs against, pacakges/flutter, search related codes in existing test runners (dev/bots/test.dart)

Find related shards: framework_tests, and subshards: widget, libraries, mise and slow.

Then look through the detailed list of the tests flutter test runs, and match to what those subshards cover

Finalize widget and libraries, which cover all the tests in v1.

As @godofredoc mentions, missing ownership of these test runners makes it difficult to achieve this.

I don't think what I'm asking has anything to do with ownership of the tests.

All I want to know is where do these strings come from.

For example, it looks like below, the "shard", and "subshard" fields come from the framework .ci.yaml file here:

https://github.com/flutter/flutter/blob/master/.ci.yaml#L498.

Can you please add a note somewhere in this file that the "shard" and "subshard" fields come from the framework .ci.yaml file?

Added a comment for both shard and validation.

-Original file line number
+Diff line change
@@ -0,0 +1,72 @@
+    {
+        "builds": [
+            {
+                "archives": [
+                    {
+                        "name": "host_debug_unopt",
+                        "base_path": "out/host_debug_unopt/zip_archives/",
+                        "type": "gcs",
+                        "include_paths": [
+                            "out/host_debug_unopt/zip_archives/dart-sdk-linux-x64.zip"
+                        ],
+                        "realm": "production"
+                    }
+                ],
+                "drone_dimensions": [
+                    "device_type=none",
+                    "os=Linux"
+                ],
+                "gn": [
+                    "--unoptimized",
+                    "--prebuilt-dart-sdk"
+                ],
+                "name": "host_debug_unopt",
+                "ninja": {
+                    "config": "host_debug_unopt",
+                    "targets": [
+                        "flutter/build/archives:dart_sdk_archive"
+                    ]
+                }
+            }
+        ],
+        "tests": [
+            {
+                "name": "test: lint host_debug widgets",
+                "drone_dimensions": [
+                    "device_type=none",
+                    "os=Linux"
+                ],
+                "dependencies": [
+                    "host_debug_unopt"
+                ],
+                "shard": "framework_tests",
+                "subshard": "widgets"
+            },
+            {
+                "name": "test: lint host_debug library",
+                "drone_dimensions": [
+                    "device_type=none",
+                    "os=Linux"
+                ],
+                "dependencies": [
+                    "host_debug_unopt"
+                ],
+                "_comment": "shard/subshard strings are defined in framework test runner: dev/bots/test.dart, and are consistent with properties defined in framework .ciyaml targets",
+                "shard": "framework_tests",
+                "subshard": "libraries"
+            },
+            {
+                "name": "test: flutter analyze",
+                "drone_dimensions": [
+                    "device_type=none",
+                    "os=Linux"
+                ],
+                "dependencies": [
+                    "host_debug_unopt"
+                ],
+                "_comment": "validation strings are consistent with properties defined in framework .ciyaml targets",
+                "validation": "analyze",
+                "validation_name": "Analyze"
+            }
+        ]
+    }

Initial config for framework_smoke test #42467

Uh oh!

Initial config for framework_smoke test #42467

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

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!

keyonghan Jun 6, 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!

keyonghan Jun 6, 2023 •

edited

Loading