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

Add a minimal example of using package:test. #51726

Merged
merged 10 commits into from
Mar 29, 2024

Conversation

matanlurey
Copy link
Contributor

@matanlurey matanlurey commented Mar 27, 2024

Work towards flutter/flutter#133569.

This PR is a proof of concept that shows we're able to use package:test in flutter/engine instead of package:litetest.

I think it also shows that, if we're going to continue to vend dependencies this way, we might want to re-think our strategy in terms of using pub as a management tool - it's quite unwieldy already. For example, here is every pubspec.yaml file in the repo:

$ find . -name 'pubspec.yaml' -exec sh -c 'echo "$0 $(wc -l < "$0")"' {} \;

# Some files omitted in third_party or similar.
./impeller/tessellator/dart/pubspec.yaml       11
./tools/const_finder/pubspec.yaml       35
./tools/api_check/pubspec.yaml       90
./tools/build_bucket_golden_scraper/pubspec.yaml       47
./tools/licenses/pubspec.yaml       53
./tools/path_ops/dart/pubspec.yaml       26
./tools/engine_tool/pubspec.yaml       76
./tools/dir_contents_diff/pubspec.yaml       19
./tools/compare_goldens/pubspec.yaml        3
./tools/golden_tests_harvester/pubspec.yaml       55
./tools/gen_web_locale_keymap/pubspec.yaml       37
./tools/githooks/pubspec.yaml       63
./tools/android_lint/pubspec.yaml       35
./tools/clang_tidy/pubspec.yaml       76
./tools/pkg/engine_repo_tools/pubspec.yaml       41
./tools/pkg/process_fakes/pubspec.yaml       36
./tools/pkg/engine_build_configs/pubspec.yaml       73
./tools/pkg/git_repo_tools/pubspec.yaml       60
./tools/header_guard_check/pubspec.yaml       70
./sky/packages/sky_engine/pubspec.yaml        8
./shell/vmservice/pubspec.yaml        8
./ci/pubspec.yaml       57
./testing/benchmark/pubspec.yaml       77
./testing/skia_gold_client/pubspec.yaml       66
./testing/pkg_test_demo/pubspec.yaml      116
./testing/smoke_test_failure/pubspec.yaml       31
./testing/dart/pubspec.yaml       71
./testing/android_background_image/pubspec.yaml       22
./testing/litetest/pubspec.yaml       33
./testing/symbols/pubspec.yaml       24
./testing/scenario_app/pubspec.yaml       67
./web_sdk/web_engine_tester/pubspec.yaml       14
./web_sdk/web_test_utils/pubspec.yaml       22
./web_sdk/pubspec.yaml       60
./lib/snapshot/pubspec.yaml        8
./lib/gpu/pubspec.yaml       14
./lib/web_ui/pubspec.yaml       60
./flutter_frontend_server/pubspec.yaml       39

I'll file a follow-up issue to discuss pub-package management in the engine.

@matanlurey matanlurey marked this pull request as ready for review March 29, 2024 00:03
@@ -0,0 +1,13 @@
import 'package:test/test.dart';
Copy link
Member

Choose a reason for hiding this comment

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

nit: Needs the engine copyright header.

(Hopefully when we finish the buildmoot, this can be enforced by a tool.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Comment on lines 21 to 25
- examples/
- impeller/
- prebuilts/
- runtime/
- shell/
Copy link
Member

Choose a reason for hiding this comment

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

Why are these excluded?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Shoot, this leaked in from another PR (the multi-pub one). Reverting.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

(I actually think it makes sense to exclude them still, but let's do it in a standalone PR so I can explain why)

@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 29, 2024
@auto-submit auto-submit bot merged commit 8480145 into flutter:main Mar 29, 2024
30 checks passed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Mar 29, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Mar 29, 2024
…146001)

flutter/engine@7176173...8480145

2024-03-29 matanlurey@users.noreply.github.com Add a minimal example of using `package:test`. (flutter/engine#51726)
2024-03-29 matanlurey@users.noreply.github.com Implement `.engine-release.version` files for engine Skia Gold tests (flutter/engine#51739)
2024-03-29 mdebbar@google.com [web] Use viewId for text editing (flutter/engine#51099)

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 jacksongardner@google.com,rmistry@google.com,zra@google.com 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

3 participants