Skip to content
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

Migrate Linux Framework Smoke Tests to engine v2 #127683

Closed
godofredoc opened this issue May 26, 2023 · 17 comments
Closed

Migrate Linux Framework Smoke Tests to engine v2 #127683

godofredoc opened this issue May 26, 2023 · 17 comments
Labels
P2 Important issues not at the top of the work list team-infra Owned by Infrastructure team triaged-infra Triaged by Infrastructure team

Comments

@godofredoc
Copy link
Contributor

https://ci.chromium.org/p/flutter/builders/prod/Linux%20Framework%20Smoke%20Tests/12035

@godofredoc godofredoc added this to To do in Engine V2 builders via automation May 26, 2023
@godofredoc godofredoc added the team-infra Owned by Infrastructure team label May 26, 2023
@keyonghan
Copy link
Contributor

@godofredoc
Copy link
Contributor Author

@keyonghan
Copy link
Contributor

With the Archive and corresponding ninja targets, the build seems fine: https://github.com/flutter/engine/blob/442f5b768e0b52474678e501adb049b5fe061e0f/ci/builders/linux_framework_smoke.json

But the tests are consistently failing due to warning logs: https://ci.chromium.org/raw/build/logs.chromium.org/flutter/led/keyonghan_google.com/bd9aad521c426c5dc001e9df2227d748e13f2be6fe510cb447e4378b6531b51b/+/build.proto?server=chromium-swarm.appspot.com

00:15 +1: All tests passed!                                                                                                                                                                            
Logger received warning output during the run, and "--fatal-warnings" is enabled.
╔═╡ERROR╞═══════════════════════════════════════════════════════════════════════
║ Command: ../../bin/flutter test --test-randomize-ordering-seed=20230602 --fatal-warnings --dart-define=dart.vm.product=false --dart-define=dart.vm.profile=true test_profile/
║ Command exited with exit code 1 but expected zero exit code.
║ Working directory: /b/s/w/ir/cache/flutter/packages/flutter
║ stdout and stderr output:
║ (stdout/stderr output was more than 10 lines)
╚═══════════════════════════════════════════════════════════════════════════════
▌12:04:29▐ Test failed.

@godofredoc
Copy link
Contributor Author

With the Archive and corresponding ninja targets, the build seems fine: https://github.com/flutter/engine/blob/442f5b768e0b52474678e501adb049b5fe061e0f/ci/builders/linux_framework_smoke.json

But the tests are consistently failing due to warning logs: https://ci.chromium.org/raw/build/logs.chromium.org/flutter/led/keyonghan_google.com/bd9aad521c426c5dc001e9df2227d748e13f2be6fe510cb447e4378b6531b51b/+/build.proto?server=chromium-swarm.appspot.com

00:15 +1: All tests passed!                                                                                                                                                                            
Logger received warning output during the run, and "--fatal-warnings" is enabled.
╔═╡ERROR╞═══════════════════════════════════════════════════════════════════════
║ Command: ../../bin/flutter test --test-randomize-ordering-seed=20230602 --fatal-warnings --dart-define=dart.vm.product=false --dart-define=dart.vm.profile=true test_profile/
║ Command exited with exit code 1 but expected zero exit code.
║ Working directory: /b/s/w/ir/cache/flutter/packages/flutter
║ stdout and stderr output:
║ (stdout/stderr output was more than 10 lines)
╚═══════════════════════════════════════════════════════════════════════════════
▌12:04:29▐ Test failed.

Nice, this is something we need to fix if this will be the new way of running framework tests in the engine. @christopherfujino what is the recommended way to fix this? it is also impacting dart in the same way.

@whesse FYI

@christopherfujino
Copy link
Member

Hmm, this is pretty difficult to read: https://logs.chromium.org/logs/flutter/buildbucket/cr-buildbucket/8779376220126520753/+/u/Run_framework_tests_libraries_tests/stdout

@gspencergoog am I missing something here, or is the test runner failing because of unexpected warning output but not actually showing the warning?

@whesse
Copy link
Contributor

whesse commented Jun 2, 2023

The warning causing the fatal exit is the 5th line of the log, saying "artifacts will be downloaded from ..."

I've been thinking about this, and propose adding a flag that would make the custom url warning not a fatal warning. That is the least intrusive. Or maybe the custom artifact url could always be non fatal when fatal warnings is enabled.

@christopherfujino
Copy link
Member

The warning causing the fatal exit is the 5th line of the log, saying "artifacts will be downloaded from ..."

Ahh yeah, of course.

@christopherfujino
Copy link
Member

I've been thinking about this, and propose adding a flag that would make the custom url warning not a fatal warning. That is the least intrusive. Or maybe the custom artifact url could always be non fatal when fatal warnings is enabled.

In taking a quick look at the code, I suspect the former would be easier.

@whesse
Copy link
Contributor

whesse commented Jun 2, 2023

Issue #116899 is already filed about this issue as well. I was planning to fix it next week, but need consensus on a solution.

whesse added a commit to whesse/flutter that referenced this issue Jun 6, 2023
Presubmit testing and CI testing of Flutter using a custom
storage location for engine artifacts must be able to use
the --fatal-warnings flag without failing due to the custom
artifact location.

This change makes this warning always non-fatal.
An alternative change that only makes the warning non-fatal
if a command-line flag is passed to the flutter tool will
also be uploaded for review.

Bug: flutter#127683
whesse added a commit to whesse/flutter that referenced this issue Jun 6, 2023
…fatal.

Presubmit testing and CI testing of Flutter using a custom
storage location for engine artifacts must be able to use
the --fatal-warnings flag without failing due to the custom
artifact location.

This change adds an option that makes this warning non-fatal.
The new --no-fatal-storage-url-warning flag makes the --fatal-warnings
flag ignore the warning that a custom artifact download URL is
being used by setting the environment variable FLUTTER_STORAGE_BASE_URL.

Bug: flutter#127683
@whesse
Copy link
Contributor

whesse commented Jun 6, 2023

I have created two PRs, one of which adds an option to suppress this fatal warning (but keep it a non-fatal warning), and the other to unconditionally make it a non-fatal warning. Note there is already a warning in the cache implementation that is non-fatal even when the --fatal-warnings flag is given.

@keyonghan
Copy link
Contributor

The test config is ready to go (except the warning).
Here is the config: flutter/engine#42467, which uses three existing targets to cover the framework smoke tests:

  • framework_tests_widget shard
  • framework_tests_libraries shard
  • analyze adhoc validation test

Corresponding recipes support: https://flutter-review.googlesource.com/c/recipes/+/45400

After the warning issue is addressed, we can enable the target in our CI.

whesse added a commit to whesse/flutter that referenced this issue Jun 8, 2023
Presubmit testing and CI testing of Flutter using a custom
storage location for engine artifacts must be able to use
the --fatal-warnings flag without failing due to the custom
artifact location.

This change makes this warning always non-fatal.

Bug: flutter#127683
auto-submit bot pushed a commit that referenced this issue Jun 9, 2023
Presubmit testing and CI testing of Flutter using a custom storage location for engine artifacts must be able to use the --fatal-warnings flag without failing due to the custom artifact location.

This change adds an option that makes this warning non-fatal. The new --no-fatal-storage-url-warning flag makes the --fatal-warnings flag ignore the warning that a custom artifact download URL is being used by setting the environment variable FLUTTER_STORAGE_BASE_URL.

Bug: #127683

- [X ] I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
- [X ] I read the [Tree Hygiene] wiki page, which explains my responsibilities.
- [X ] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
- [X ] I signed the [CLA].
- [X ] I listed at least one issue that this PR fixes in the description above.
- [X ] I updated/added relevant documentation (doc comments with `///`).
@whesse
Copy link
Contributor

whesse commented Jun 9, 2023

The fatal warning due to the custom download URL should be gone now, my fix has landed. Led runs testing this are still pending.

@keyonghan
Copy link
Contributor

keyonghan commented Jun 9, 2023

The fatal warning due to the custom download URL should be gone now, my fix has landed. Led runs testing this are still pending.

Thanks @whesse for the fix! Validated tests are passing now: https://ci.chromium.org/raw/build/logs.chromium.org/flutter/led/keyonghan_google.com/5d35ba2b3f79764bfe58f10e4cd1bce118c3bd485ece5aff184183b232d181da/+/build.proto?server=chromium-swarm.appspot.com.

So the only blocker here on this bug is make artifacts hermetic to the builder.

@flutter-triage-bot
Copy link

This issue is assigned to @keyonghan but has had no recent status updates. Please consider unassigning this issue if it is not going to be addressed in the near future. This allows people to have a clearer picture of what work is actually planned. Thanks!

@flutter-triage-bot flutter-triage-bot bot added the Bot is counting down the days until it unassigns the issue label Oct 27, 2023
@flutter-triage-bot
Copy link

This issue was assigned to @keyonghan but has had no status updates in a long time. To remove any ambiguity about whether the issue is being worked on, the assignee was removed.

@flutter-triage-bot flutter-triage-bot bot removed the Bot is counting down the days until it unassigns the issue label Jan 6, 2024
@keyonghan keyonghan added P2 Important issues not at the top of the work list triaged-infra Triaged by Infrastructure team labels Jan 11, 2024
@keyonghan
Copy link
Contributor

The target has been removed from Engine repo. Closing as obsolete.

Engine V2 builders automation moved this from To do to Done Mar 18, 2024
Copy link

github-actions bot commented Apr 2, 2024

This thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of flutter doctor -v and a minimal reproduction of the issue.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
P2 Important issues not at the top of the work list team-infra Owned by Infrastructure team triaged-infra Triaged by Infrastructure team
Projects
Development

No branches or pull requests

4 participants