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

Conversation

matanlurey
Copy link
Contributor

@matanlurey matanlurey commented Oct 1, 2024

Does a few things:

  • Refactors run_command_test significantly to reduce global fixtures
  • Replaced stringly-typed things with enum-like objects
  • Adds a lot stronger coverage for run_command to make future refactors safer
  • Takes advantage of package:test having a workable matchers system and uses it
  • Changes return 1 into throw FatalError(...) where it makes sense in run_command

As a result of the refactoring work, I also fixed a bug: Closes flutter/flutter#147646.

@matanlurey matanlurey added the Work in progress (WIP) Not ready (yet) for review! label Oct 1, 2024
auto-submit bot pushed a commit that referenced this pull request Oct 1, 2024
Partial work towards flutter/flutter#148420, unblocks #55537.

These tests can now be more precise, and changing the fixtures no longer has side-effects on tests across the entire repository. There are about 11 other usages (there were ~50 in these) after this PR that I'll get to, as well as the hard-coded `gn desc` output, before retiring `fixtures.dart`
@matanlurey matanlurey removed the Work in progress (WIP) Not ready (yet) for review! label Oct 2, 2024
@matanlurey matanlurey marked this pull request as ready for review October 2, 2024 19:20
@matanlurey
Copy link
Contributor Author

@johnmccutchan If you are looking to kill a few minutes, would appreciate a review. Thanks!

Copy link
Contributor

@johnmccutchan johnmccutchan left a comment

Choose a reason for hiding this comment

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

I have some nits/questions.

Such a nice cleanup. Thanks for this!

@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Oct 3, 2024
@auto-submit auto-submit bot merged commit 4d3eb5b into flutter:main Oct 3, 2024
30 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 3, 2024
Copy link
Member

@zanderso zanderso left a comment

Choose a reason for hiding this comment

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

DBC

@@ -0,0 +1,65 @@
import 'dart:convert';
Copy link
Member

Choose a reason for hiding this comment

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

Needs the engine copyright header.

@@ -0,0 +1,66 @@
import 'dart:convert';
Copy link
Member

Choose a reason for hiding this comment

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

ditto.

@@ -0,0 +1,92 @@
import 'package:engine_tool/src/logger.dart';
Copy link
Member

Choose a reason for hiding this comment

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

ditto

engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Oct 3, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Oct 3, 2024
…156174)

flutter/engine@de1762d...20369c5

2024-10-03 [email protected] [Impeller] A text layout and shaping API for the standalone library. (flutter/engine#55598)
2024-10-03 [email protected] Refactor `et run` (and friends). (flutter/engine#55537)
2024-10-03 [email protected] [Impeller] Do not cull in TextFrameDispatcher::drawDisplayList if the transform has perspective (flutter/engine#55602)
2024-10-03 [email protected] iOS: Fix IOSSurfaceNoopTest (flutter/engine#55621)
2024-10-03 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[Impeller] one aiks context per app. (#55393)" (flutter/engine#55625)
2024-10-03 [email protected] Roll Fuchsia Linux SDK from YKvfbpZ38daiwJ7H0... to TnSpLq0S9XT2rHGl1... (flutter/engine#55622)
2024-10-03 [email protected] [Impeller] one aiks context per app. (flutter/engine#55393)

Also rolling transitive DEPS:
  fuchsia/sdk/core/linux-amd64 from YKvfbpZ38dai to TnSpLq0S9XT2

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-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://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
auto-submit bot pushed a commit that referenced this pull request Oct 4, 2024
nick9822 pushed a commit to nick9822/flutter that referenced this pull request Dec 18, 2024
Does a few things:

- Refactors `run_command_test` significantly to reduce global fixtures
- Replaced stringly-typed things with enum-like objects
- Adds a lot stronger coverage for `run_command` to make future refactors safer
- Takes advantage of `package:test` having a workable matchers system and uses it
- Changes `return 1` into `throw FatalError(...)` where it makes sense in `run_command`

As a result of the refactoring work, I also fixed a bug: Closes flutter#147646.
nick9822 pushed a commit to nick9822/flutter that referenced this pull request Dec 18, 2024
fluttermirroringbot pushed a commit to flutter/flutter that referenced this pull request Dec 18, 2024
…gine#55540)

Partial work towards #148420, unblocks flutter/engine#55537.

These tests can now be more precise, and changing the fixtures no longer has side-effects on tests across the entire repository. There are about 11 other usages (there were ~50 in these) after this PR that I'll get to, as well as the hard-coded `gn desc` output, before retiring `fixtures.dart`
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
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.

et run throws UnimplementedError for Android on Windows
3 participants