Add integration test for asset transformation feature by andrewkolos · Pull Request #145715 · flutter/flutter

andrewkolos · 2024-03-25T18:11:53Z

In service of #143348

This adds a simple integration test for the new asset transformation feature.

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.
I followed the breaking change policy and added Data Driven Fixes where supported.
All existing and new tests are passing.

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

andrewkolos · 2024-04-03T22:29:38Z

Looks like using vector_graphics_compiler (as a transformer) and vector_graphics for seeing if the asset was transformed correctly by trying to render it won't work.

vector_graphics has a dependency on package http ^1.0.0, but googleapis_auth depends on ^0.13.0

While it would have been nice to use a more real-world-style example in the integration test, adding a bunch of transitive dependencies probably wasn't the right move here.

I'm considering instead using a contrived local path dependency as a transformer for this test.

andrewkolos · 2024-04-05T03:12:42Z

I ran this in the try pool: https://chromium-swarm.appspot.com/task?id=68c50593530ecb10

christopherfujino · 2024-04-05T06:13:43Z

I ran this in the try pool: https://chromium-swarm.appspot.com/task?id=68c50593530ecb10

This looks like Linux analyze?

dev/devicelab/bin/tasks/asset_transformation_test.dart

dev/integration_tests/asset_transformation/capitalizer_transformer/CHANGELOG.md

dev/integration_tests/asset_transformation/integration_test/integration_test.dart

.ci.yaml

TESTOWNERS

.ci.yaml

jmagman · 2024-04-05T20:57:30Z

dev/integration_tests/asset_transformation/.gitignore

Also, since this is such a simple project, consider not adding a full integration_test project and instead making one on-the-fly as other tools integration tests do:

https://github.com/flutter/flutter/blob/master/packages/flutter_tools/test/integration.shard/test_data/basic_project.dart
https://github.com/flutter/flutter/blob/master/packages/flutter_tools/test/integration.shard/test_data/stateless_stateful_project.dart
https://github.com/flutter/flutter/blob/master/packages/flutter_tools/test/integration.shard/test_data/deferred_components_project.dart

Hmm, this seems like a decent idea, but perhaps there is value is keeping the project on repo—we could use it as part of the documentation for the feature on flutter.dev. For example we do this for flavors:

For examples of build flavors for iOS, macOS, and Android, check out the integration test samples in the Flutter repo.

I imagine myself writing a "how to write your own asset-transforming package" section the flutter.dev documentation for the feature, and users might appreciate being able to see a complete example (and not just a snippet). Let me know what you think.

Maybe we should add this to https://github.com/flutter/samples.

There is a baseline cost to every flutter project that we keep in the flutter/flutter repo (both pubspec deps and also maintaining the platform-specific directories.

If you go with generating this on the fly, I would recommend not using dart create to create the dart transformer project, but instead just programmatically creating the files you need, so that upstream changes to the dart create ... output won't break the flutter/flutter tree.

TIL of the existence of flutter/samples.

I'll 1) proceed with Jenn's suggestion and 2) consider adding a sample to flutter/samples when writing the flutter.dev documentation.

Addressed in create test project on-the-fly instead of having it in-repo

Hmm, this seems like a decent idea, but perhaps there is value is keeping the project on repo—we could use it as part of the documentation for the feature on flutter.dev. For example we do this for flavors:

For examples of build flavors for iOS, macOS, and Android, check out the integration test samples in the Flutter repo.

I imagine myself writing a "how to write your own asset-transforming package" section the flutter.dev documentation for the feature, and users might appreciate being able to see a complete example (and not just a snippet). Let me know what you think.

Have you reached out to DevRel? They could help with messaging this. In any case, at the point you need a complete example for documentation, and not integration tests, I agree that samples would be a better spot for it.

Have you reached out to DevRel? They could help with messaging this.

Good idea

christopherfujino · 2024-04-18T21:22:08Z

@andrewkolos what's the status on this PR?

andrewkolos · 2024-04-26T20:37:11Z

@andrewkolos what's the status on this PR?

@christopherfujino, IIRC all comments have been addressed and this is ready for review.

christopherfujino

LGTM

…r/flutter#145715)

flutter/flutter@2e80670...f9933b6 2024-04-27 [email protected] Roll Flutter Engine from 5205e3683a0a to 20fb62ba1455 (1 revision) (flutter/flutter#147449) 2024-04-27 [email protected] Roll Flutter Engine from e14649ea0c80 to 5205e3683a0a (1 revision) (flutter/flutter#147448) 2024-04-27 [email protected] Roll Flutter Engine from 87f489c1bed4 to e14649ea0c80 (1 revision) (flutter/flutter#147446) 2024-04-27 [email protected] Roll Flutter Engine from cecf5aa8a778 to 87f489c1bed4 (1 revision) (flutter/flutter#147445) 2024-04-27 [email protected] Roll Flutter Engine from 8af10eba3ef3 to cecf5aa8a778 (1 revision) (flutter/flutter#147444) 2024-04-27 [email protected] [macOS] Eliminate flutter_gallery_macos__start_up benchmark (flutter/flutter#147442) 2024-04-27 [email protected] Roll Flutter Engine from bc055398f42a to 8af10eba3ef3 (1 revision) (flutter/flutter#147441) 2024-04-27 [email protected] Roll Flutter Engine from c410180e5bba to bc055398f42a (7 revisions) (flutter/flutter#147440) 2024-04-26 [email protected] Add tests for character_activator.0.dart API example. (flutter/flutter#147384) 2024-04-26 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.25.2 to 3.25.3 (flutter/flutter#147437) 2024-04-26 [email protected] Add integration test for asset transformation feature (flutter/flutter#145715) 2024-04-26 [email protected] Added missing tests for Table api example `table.0.dart`. (flutter/flutter#147318) 2024-04-26 [email protected] Catch any `FileSystemException` thrown when trying to read the template manifest during `flutter create` (flutter/flutter#145620) 2024-04-26 [email protected] fixes `CupertinoFullscreenDialogTransition` leaks (flutter/flutter#147168) 2024-04-26 [email protected] Fix helperMaxLines and errorMaxLines documentation (flutter/flutter#147409) 2024-04-26 [email protected] Refactor route focus node creation (flutter/flutter#147390) 2024-04-26 [email protected] Roll Packages from fde908d to dd01140 (5 revisions) (flutter/flutter#147420) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: 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

…r/flutter#145715)

andrewkolos added the tool Affects the "flutter" command-line tool. See also t: labels. label Mar 25, 2024

github-actions bot added a: desktop Running on desktop and removed tool Affects the "flutter" command-line tool. See also t: labels. labels Mar 25, 2024

andrewkolos force-pushed the asset-transformers-integration-testing branch from 319e29c to dd5271f Compare March 25, 2024 18:30

andrewkolos force-pushed the asset-transformers-integration-testing branch from 679f61a to e066881 Compare April 4, 2024 03:10

github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Apr 4, 2024

andrewkolos marked this pull request as ready for review April 5, 2024 03:13

andrewkolos requested a review from christopherfujino as a code owner April 5, 2024 03:13

christopherfujino reviewed Apr 5, 2024

View reviewed changes

dev/devicelab/bin/tasks/asset_transformation_test.dart Outdated Show resolved Hide resolved

christopherfujino reviewed Apr 5, 2024

View reviewed changes

dev/integration_tests/asset_transformation/capitalizer_transformer/CHANGELOG.md Outdated Show resolved Hide resolved

christopherfujino reviewed Apr 5, 2024

View reviewed changes

dev/integration_tests/asset_transformation/integration_test/integration_test.dart Outdated Show resolved Hide resolved

keyonghan reviewed Apr 5, 2024

View reviewed changes

.ci.yaml Outdated Show resolved Hide resolved

TESTOWNERS Outdated Show resolved Hide resolved

jmagman reviewed Apr 5, 2024

View reviewed changes

.ci.yaml Outdated Show resolved Hide resolved

github-actions bot removed the a: desktop Running on desktop label Apr 5, 2024

andrewkolos requested a review from christopherfujino April 5, 2024 20:31

jmagman reviewed Apr 5, 2024

View reviewed changes

andrewkolos added 10 commits April 22, 2024 08:52

create new project for integration test

457be6c

add test svg asset to project

b812c9d

add integration test

d199318

use custom local path dependency for transformer

05c36f6

convert to simple flutter build test

177ff5b

Update TESTOWNERS

6f1db0b

fix license headers

a9103b3

+

1160155

++

4f4a7d1

++

a886818

andrewkolos added 11 commits April 22, 2024 08:52

experiment again

c41826e

hmm

c6076bc

redo test as a normal tools integration test

b159d28

undo TESTOWNERS change

a227c94

cleanup

63af5ef

nit

6a62b8d

tidy up

ec47b6e

make analyze.dart happy

dc87e14

create test project on-the-fly instead of having it in-repo

ab0b084

rename a variable :)

bf866f0

run flutter upgrade-packages --force-upgrade

9a1217c

andrewkolos force-pushed the asset-transformers-integration-testing branch from 01f65a1 to 9a1217c Compare April 22, 2024 15:55

andrewkolos added autosubmit Merge PR when tree becomes green via auto submit App and removed autosubmit Merge PR when tree becomes green via auto submit App labels Apr 22, 2024

christopherfujino approved these changes Apr 26, 2024

View reviewed changes

auto-submit bot merged commit 290dadb into flutter:master Apr 26, 2024

engine-flutter-autoroll mentioned this pull request Apr 27, 2024

Roll Flutter from 2e806700b928 to f9933b6994e4 (17 revisions) flutter/packages#6619

Closed

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

290dadb Add integration test for asset transformation feature (flutte…

a1fae80

…r/flutter#145715)

engine-flutter-autoroll mentioned this pull request Apr 27, 2024

Roll Flutter from 2e806700b928 to f9933b6994e4 (17 revisions) flutter/packages#6620

Merged

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

290dadb Add integration test for asset transformation feature (flutte…

e726ed0

…r/flutter#145715)

andrewkolos deleted the asset-transformers-integration-testing branch April 29, 2024 18:17

andrewkolos mentioned this pull request Apr 30, 2024

☂️ Implement pluggable transformation (via packages) of assets at build time #143348

Closed

13 tasks

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

290dadb Add integration test for asset transformation feature (flutte…

ea3df96

…r/flutter#145715)

Conversation

andrewkolos commented Mar 25, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

andrewkolos commented Apr 3, 2024

Uh oh!

andrewkolos commented Apr 5, 2024

Uh oh!

christopherfujino commented Apr 5, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jmagman Apr 5, 2024

Choose a reason for hiding this comment

Uh oh!

andrewkolos Apr 5, 2024

Choose a reason for hiding this comment

Uh oh!

christopherfujino Apr 5, 2024

Choose a reason for hiding this comment

Uh oh!

christopherfujino Apr 5, 2024

Choose a reason for hiding this comment

Uh oh!

andrewkolos Apr 5, 2024

Choose a reason for hiding this comment

Uh oh!

andrewkolos Apr 5, 2024

Choose a reason for hiding this comment

Uh oh!

jmagman Apr 5, 2024

Choose a reason for hiding this comment

Uh oh!

andrewkolos Apr 5, 2024 • 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!

christopherfujino commented Apr 18, 2024

Uh oh!

andrewkolos commented Apr 26, 2024

Uh oh!

christopherfujino left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

andrewkolos commented Mar 25, 2024 •

edited

Loading

christopherfujino commented Apr 5, 2024 •

edited

Loading

andrewkolos Apr 5, 2024 •

edited

Loading