Now that we're in a monorepo, it makes sense to use the in-tree engine
by default when using `--local-engine` or `--local-web-sdk` rather than
looking for a sibling directory like we used to.
This PR rolls in a number of breaking changes from dart-lang/native:
* `BuildMode` is no longer part of the protocol, so Flutter no longer
passes it in.
* This means all code dealing with the name conflict between
`native_assets_cli.BuildMode` and `flutter_tools.BuildMode` has been
cleaned up.
* Also, the logs no longer mention the build mode.
* The tests still exercise both modes, because linking only happens in
release mode.
* `OS` is no longer part of the main protocol, but of the "code"
"protocol extension".
* The code now aligns more with `OS?` being nullable in a bunch of
places, since it is nullable if there's no code assets.
* The OS-specific config is nested in an object per OS.
* `CCompilerConfig`s fields are non-nullable now.
* So instead of passing an object with nullable fields around, a null
instead of the object is returned in various places.
* `FileSystem` is now passed in to the native assets builder.
This PR contains no feature changes.
This PR will need to be followed up by restricting what environment
variables are passed in (similar to
https://github.com/dart-lang/native/pull/1764), I will do this in a
follow up PR.
Tests:
* All existing features should be covered by existing tests.
Work towards https://github.com/flutter/flutter/issues/143299.
Work towards https://github.com/flutter/flutter/issues/160043.
---
This PR implements, end-to-end, support for `matchesGoldenFile` when (a)
running with `package:integration_test` (b) on a device, such as an
Android emulator, Android device, iOS simulator, or iOS device, where
the _runner_ of a test file does not have process and local-file system
access.
There are multiple parts to this PR; I could make it smaller than 1K
lines, but the bulk of that is tests, and it would mean landing PRs that
are incomplete and unused, which does not seem useful - so instead here
is a quick overview of the PR's contents - questions/feedback welcome,
and I am willing to break code out or land incremental refactors if
requested.
1. Augmented `flutter_platform.dart` (used for iOS and Android), similar
to
[`flutter_web_platform.dart`](1398dc7eec/packages/flutter_tools/lib/src/test/flutter_web_platform.dart (L117-L128)),
now creates and uses
[`test_golden_comparator.dart`](https://github.com/flutter/flutter/blob/master/packages/flutter_tools/lib/src/test/test_golden_comparator.dart)
to proxy calls (coming from the VM service protocol) for golden-file
updates and comparisons to a `flutter_tester` process. A full
explanation of how (or why) it works this way is too hard to include
here, but see https://github.com/flutter/flutter/pull/160215 for more
details.
1. Added `VmServiceProxyGoldenFileComparator`, which is a currently
unused (outside of a single e2e test) comparator that forwards calls to
`compare` and `update` to the VM service protocol (of which, the other
side of this is implemented above, in `flutter_platform.dart`. The idea
is that this comparator would be used automatically when running in an
integration test on a device that requires it (similar to how web works
today), but that is **not** wired up yet and requires additional work in
`flutter_tools`.
1. Added two unit tests (of both the client and server), and a full
e2e-test using it to run `matchesGoldenFile`.
Now that "auto" is not supported anymore, it makes more sense to make
this dart define false by default. And there's no need to pass
`FLUTTER_WEB_AUTO_DETECT=false` anymore.
dart_style 3.0.1 comes with some minor style fixes:
https://github.com/dart-lang/dart_style/blob/main/CHANGELOG.md#301
This PR applies this fixes in bulk across the repository. (Otherwise,
the next person touching these files would have been the one updating
them to the new format by running the formatter).
Work around: https://github.com/flutter/flutter/issues/160689.
I locally verified that we have not regressed what this was testing
(that just invoking `//flutter/bin/dart` will not build the flutter
tool).
In the future a hook may be invoked multiple times with different
`supportedAssetTypes` (soon to be renamed to `buildAssetTypes`).
The hook should only emit those asset types that are in
`supportedAssetTypes` - anything else is an error. Right now flutter
happens to invoke hooks only with `Code` asset types, but more asset
types are coming, for which this PR is a preparation for.
The integration test framework that waits for transitions and
(optionally) takes actions on transitions allows to match patterns.
If one uses a RegExp pattern than the framework only checks whether a
line contains the given RegExp pattern.
If one uses a String pattern it matches it exactly.
=> We add a `Barrier.contains()` and `Multiple.contains()` that allow
matching a line with if it contains the String (just like in RegExp)
=> This makes tests simpler as they don't have to know about the exact
padding of progres bar etc. Those may be irrelevant for the purpose of
the integration test and only complicate it.
This auto-formats all *.dart files in the repository outside of the
`engine` subdirectory and enforces that these files stay formatted with
a presubmit check.
**Reviewers:** Please carefully review all the commits except for the
one titled "formatted". The "formatted" commit was auto-generated by
running `dev/tools/format.sh -a -f`. The other commits were hand-crafted
to prepare the repo for the formatting change. I recommend reviewing the
commits one-by-one via the "Commits" tab and avoiding Github's "Files
changed" tab as it will likely slow down your browser because of the
size of this PR.
---------
Co-authored-by: Kate Lovett <katelovett@google.com>
Co-authored-by: LongCatIsLooong <31859944+LongCatIsLooong@users.noreply.github.com>
Part of https://github.com/flutter/flutter/issues/102983.
`<FlutterManifest>.generateSyntheticPackage` _never_ meant generate a
synthetic package 😒, it only meant "we _might_ need to generate a
synthetic package because localizations are being generated and the
default, unless otherwise specified, is to generate a synthetic
package".
Renamed as `generateLocalizations` and added some strategic TODOs in
places where removing the `package:flutter_gen` feature
(https://github.com/flutter/flutter/issues/102983) will allow us to
cleanup this erroneous code and technical debt.
Simplified a bit code (just a refactor) in the process, and fixes a bug
that `flutter packages get` would generate internationalization files
even if `flutter: generate: true` was not present in `pubspec.yaml` that
was revealed as part of fixing this up.
/cc @sigurdm.
Without this change, the order that tests run matter and it's easy to
add tests without remembering this flag.
(There are existing tests that forgot it too, they just happen to work
if shuffled in a way that, well, works)
Towards https://github.com/flutter/flutter/issues/160257.
I intentionally skipped `create_test.dart`, as that requires updating
the generated template which uses l10n, a feature changing. I'll do that
in the "big bang" PR that finally enables the feature to avoid getting
us into a bad state.
Part of https://github.com/flutter/flutter/issues/160043, makes it
easier to add https://github.com/flutter/flutter/pull/160131.
This PR has no functional changes to any of the code, but does refactor
both the code and tests:
- Makes a number of always non-null but not migrated to non-null
properties, well, not-null
- Creates two concrete methods (`update` and `compare` versus a
positional nullable boolean)
- Uses type signatures instead of `String?` to explain the possible
results of the methods
- Renames the mysterious `shellPath` variable to `flutterTesterBinPath`
- Expands and rewrites internally-facing doc comments
- Moves `WebRenderer` environment variable setting to
`flutter_web_platform.dart`
- Makes the tests have less duplication, and check for update/compare
cases
After this PR, I can use it in the non-web branch of the Flutter tool
without any hacks or TODOS :)
/cc @eyebrowsoffire (trivial web refactoring), @camsim99 (changes being
made to tool).
When running `dart format` over these lines the `// ignore` ended up on
a line where it wasn't properly ignoring the lint. This adjusts the
placement of `// ignore`s so they will continue to ignore the right
thing even after the code is auto formatted.
I am hoping that if we do this now the large PR that formats the entire
repo will go in smoother without manual intervention.
Work towards https://github.com/flutter/flutter/issues/160257.
Unlike some of the other PRs, this test explicitly _opts-out_ of the
flag, as the test itself is testing whether the now deprecated feature
works.
The previous attempt at this fix was assuming that the tool's file
system was a `LocalFileSystem`, but in reality it's a `LocalFileSystem`
wrapped in an `ErrorHandlingFileSystem`. This change takes this into
account.
Fixes https://github.com/flutter/flutter/issues/160082
Fixes https://github.com/flutter/flutter/issues/156962
---------
Co-authored-by: Andrew Kolos <andrewrkolos@gmail.com>
The generated widget_preview_scaffold project needs to explicitly
reference the assets from the parent project's pubspec.yaml. This change
updates flutter widget-preview start to read the parent project's
pubspec.yaml and add references to the assets listed to the
widget_preview_scaffold's pubspec.yaml. If generate: true is set in the
parent project, a reference to the autogenerated flutter_gen package is
manually added to the widget_preview_scaffold's package_config.json.
<!--
Thanks for filing a pull request!
Reviewers are typically assigned within a week of filing a request.
To learn more about code review, see our documentation on Tree Hygiene:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
-->
**Description**
While exploring some semi-related stuff, found these 2 tests using
outdated regex which does not work because AGP version in modern
templates is set in `settings.gradle.kts` and in form of
`com.android.application` instead of `com.android.tools.build:gradle`.
Apart from that, in `android_plugin_example_app_build_test.dart` deleted
all lines regarding version change (instead of comply with new way of
declaring plugin) because for a long time it's didn't work anyway:
`replaceAll` haven't find any matches and test ran on latest AGP from
template. More than that, attempt to adapt this test to modern AGP setup
failed because build is not working with AGP < 8 (I lost logs with
actual error for this case, but I believe I can reproduce if anyone
wants)
in `native_assets_agp_version_test`:
- Fixed version to comply with AGP versioning format, which is
`major.minor.patch`.
- Updated regex and version changing logic to work with
`com.android.application` format and `settings.gradle.kts` file. I
believe that version updating is desired behavior here, unlike in
`android_plugin_example_app_build_test.dart`.
- Updated `kts` syntax for declaring flavors in `build.gradle.kts` and
updated regex-based updating of this file (didn't work previously
because there wasn't actual writing to file)
didn't list any issues because there're not any regarding these tests
(or maybe I just failed to find). In any case, I think that this doesn't
require issue because fix is kinda trivial and motivation is clear.
## Pre-launch Checklist
- [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].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [x] I updated/added relevant documentation (doc comments with `///`).
- [x] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [x] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.
If you need help, consider asking for advice on the #hackers-new channel
on [Discord].
<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This is the initial tooling work for Flutter Widget Previews, adding two
commands: `flutter widget-preview start` and `flutter widget-preview
clean`.
The `start` command currently only checks to see if
`.dart_tool/widget_preview_scaffold/` exists and creates a new Flutter
project using the widget_preview_scaffold template if one isn't found.
The widget_preview_scaffold template currently only contains some
placeholder files and will be updated to include additional code
required by the scaffold.
The `clean` command simply deletes `.dart_tool/widget_preview_scaffold/`
if it's found.
This change also includes some refactoring of the `create` command in
order to share some project creation logic without requiring `flutter
widget-preview start` to spawn a new process simply to run `flutter
create -t widget_preview .dart_tool/widget_preview_scaffold`.
Related issue: https://github.com/flutter/flutter/issues/115704
---------
Co-authored-by: Andrew Kolos <andrewrkolos@gmail.com>
Native asset tests use `flutter create --no-pub --template=package_ffi`.
The template used for this is checked in. It then adds extra
dependencies to checked-in packages in flutter/flutter (which have
pinned deps) in those generated templates.
It then pins all dependencies in the modified template project. This can
lead to constraint violations when flutter updates pinned dependencies,
because the template uses old constraints (which are turned from `^x` to
`=x`) and the additional dependency on flutter/flutter checked in
package brings in different pinned dependencies.
In a previous PR we already made this more robust by using flutter's
pinned versions over the the versions from the template (that are
changed from `^x` to `=x`).
Though a new upgrade of flutters pinned packages reveals that this isn't
quite sufficient: The template uses `test` at `^X`. The additional
dependency to `link_hook` doesn't depend on `test`. It therefore turns
it into `=X`. BUT `link_hooks` has a non-dev dependency on `test_core`
which is incompatible with `=X`.
=> So we relax this even more by prefering to choose (pinned) versions
of the flutter/flutter `link_hook` dependencies and (new) dev
dependencies over the template dependencies.
=> This will make use use the pinned `test` version from `link_hooks`
instead of from the template.
