Skip to content

Write the package config location to the test bootstrap. #150440

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.

Sign up for GitHub

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

Jump to bottom
Merged
kenzieschmoll merged 18 commits into from
Jul 9, 2024
Merged

Write the package config location to the test bootstrap. #150440

kenzieschmoll merged 18 commits into from
Jul 9, 2024

Conversation

@kenzieschmoll
Copy link
Member

@kenzieschmoll kenzieschmoll commented Jun 18, 2024 •
edited
Loading

Work towards. This is the Flutter companion to dart-lang/test#2245. Work towards dart-lang/test#2246.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jun 18, 2024
@kenzieschmoll
Copy link
Member Author

kenzieschmoll commented Jun 18, 2024

@christopherfujino @natebosch @jakemac53 can you advise on the best way to get the package config URI here?

@jakemac53
Copy link
Contributor

jakemac53 commented Jun 18, 2024 •
edited
Loading

@christopherfujino @natebosch @jakemac53 can you advise on the best way to get the package config URI here?

Is Isolate.packageConfig null? I think flutter tool might be precompiled which could cause that..... although there is likely some way to get one still (in pub we have one even though package:test is also compiled to kernel).

Unfortunately, package:package_config does not expose something to just do the package config lookup for you, see dart-lang/tools#1544 for the feature request which you might want to chime in on.

@natebosch
Copy link
Contributor

natebosch commented Jun 18, 2024

This code is in flutter_tools - does that package have any existing code related to finding or using the package config that we can reuse?

@kenzieschmoll kenzieschmoll mentioned this pull request Jun 18, 2024
Open
@christopherfujino
Copy link
Contributor

christopherfujino commented Jun 20, 2024

@christopherfujino @natebosch @jakemac53 can you advise on the best way to get the package config URI here?

Is Isolate.packageConfig null? I think flutter tool might be precompiled which could cause that..... although there is likely some way to get one still (in pub we have one even though package:test is also compiled to kernel).

We want the package_config.json for the test app, not the tool, or am I misunderstanding the use case here?

@jakemac53
Copy link
Contributor

jakemac53 commented Jun 20, 2024

We want the package_config.json for the test app, not the tool, or am I misunderstanding the use case here?

We want the one for the app, not the tool. Good point that in the flutter case, these are not the same (in the dart test case, they are the same).

@christopherfujino
Copy link
Contributor

christopherfujino commented Jun 20, 2024

@sigurdm do you have any insights on the best way to get a test app's package-config.json, since you're doing some similar work in the tool?

@sigurdm
Copy link
Contributor

sigurdm commented Jun 21, 2024 •
edited
Loading

do you have any insights on the best way to get a test app's package-config.json,

flutterProject.packageConfigFile.uri will probably work.

But I think it would be better to thread a BuildInfo object from:

flutter/packages/flutter_tools/lib/src/commands/test.dart

Line 358 in 8e4f704

final BuildInfo buildInfo = await getBuildInfo(forcedBuildMode: BuildMode.debug);

in to testRunner.runTests()

flutter/packages/flutter_tools/lib/src/commands/test.dart

Line 591 in 8e4f704

result = await testRunner.runTests(

in to loader.installHook()

flutter/packages/flutter_tools/lib/src/test/runner.dart

Line 242 in 0287c22

final loader.FlutterPlatform platform = loader.installHook(

to FlutterPlatform()

flutter/packages/flutter_tools/lib/src/test/flutter_platform.dart

Line 86 in 4e70bfa

final FlutterPlatform platform = FlutterPlatform(

And then use globals.fs.path.toUri(buildInfo.packagesPath). (will be globals.fs.path.toUri(buildInfo.packageConfigPath) after #150559)

@kenzieschmoll
Copy link
Member Author

kenzieschmoll commented Jun 25, 2024

Thanks for the suggestion @sigurdm. I have applied the changes you suggested.

@kenzieschmoll kenzieschmoll marked this pull request as ready for review June 25, 2024 16:36
@kenzieschmoll
Copy link
Member Author

kenzieschmoll commented Jun 25, 2024

@christopherfujino This is ready for review now.

@kenzieschmoll kenzieschmoll mentioned this pull request Jun 25, 2024
Merged
@kenzieschmoll kenzieschmoll mentioned this pull request Jun 25, 2024
Merged
christopherfujino
christopherfujino reviewed Jun 26, 2024
View reviewed changes
christopherfujino
christopherfujino reviewed Jun 26, 2024
View reviewed changes
@kenzieschmoll kenzieschmoll mentioned this pull request Jun 27, 2024
Closed
3 tasks
@kenzieschmoll
Copy link
Member Author

kenzieschmoll commented Jul 2, 2024

I suspect google testing will pass once #150559 makes it into g3.

christopherfujino
christopherfujino approved these changes Jul 2, 2024
View reviewed changes
Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

LGTM

@kenzieschmoll kenzieschmoll merged commit 5ebc993 into flutter:master Jul 9, 2024
@kenzieschmoll kenzieschmoll deleted the packageConfigLocation branch July 9, 2024 16:36
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 9, 2024
Closed
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 9, 2024
@engine-flutter-autoroll
5ebc993 Write the package config location to the test bootstrap. (flu…
d912ce3 
…tter/flutter#150440)
@engine-flutter-autoroll engine-flutter-autoroll mentioned this pull request Jul 9, 2024
Merged
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 9, 2024
@engine-flutter-autoroll
5ebc993 Write the package config location to the test bootstrap. (flu…
da332e9 
…tter/flutter#150440)
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jul 9, 2024
@engine-flutter-autoroll
Roll Flutter from fafd67d to 5103d75 (27 revisions) (#7084)
1faccfe 
flutter/flutter@fafd67d...5103d75

2024-07-09 [email protected] [tool] Remove some usages of deprecated usage package (flutter/flutter#151359)
2024-07-09 [email protected] Write the package config location to the test bootstrap. (flutter/flutter#150440)
2024-07-09 [email protected] [deps] Roll dart-lang/native packages (flutter/flutter#151403)
2024-07-08 [email protected] [tool] make `testUsingContext` provide a `Stdio` (with `hasTerminal` unset) override by default (flutter/flutter#151357)
2024-07-08 [email protected] Make `FittedBox` not throw when child has zero size. (flutter/flutter#150430)
2024-07-08 [email protected] Update `DataTable` documentation (flutter/flutter#151356)
2024-07-08 [email protected] `MaterialState` â�� `WidgetState` in documentation (flutter/flutter#151376)
2024-07-08 [email protected] [ios]A typical news app benchmark with bottom ad banner (flutter/flutter#150991)
2024-07-08 [email protected] Re-enable `SemanticsAction.focus` matchers (flutter/flutter#150990)
2024-07-08 [email protected] Added SliverFloatingHeader.snapMode (flutter/flutter#151289)
2024-07-08 [email protected] Factor out deprecated names in example code (flutter/flutter#151374)
2024-07-08 [email protected] Add cedric vanden bosch to authors (flutter/flutter#151313)
2024-07-08 [email protected] Roll Packages from 97bad7e to 14341d1 (5 revisions) (flutter/flutter#151417)
2024-07-08 [email protected] Update doc-import to primary configured import, _goldens_io.dart (flutter/flutter#151390)
2024-07-08 [email protected] [Reland] - Enable `explicitChildNodes` for the `AlertDialog` content (flutter/flutter#149597)
2024-07-08 [email protected] Add tests for material_state_border_side.0_test.dart (flutter/flutter#151089)
2024-07-08 [email protected] Roll Flutter Engine from ca79a56a66d7 to 69075e7e87d4 (1 revision) (flutter/flutter#151393)
2024-07-08 [email protected] Add tests for action_listener.0.dart (flutter/flutter#150606)
2024-07-07 [email protected] Roll Flutter Engine from 5ca3b856ee5a to ca79a56a66d7 (1 revision) (flutter/flutter#151387)
2024-07-06 [email protected] Roll Flutter Engine from 3600ec613a00 to 5ca3b856ee5a (1 revision) (flutter/flutter#151378)
2024-07-06 [email protected] Roll Flutter Engine from d1ebc5fde630 to 3600ec613a00 (1 revision) (flutter/flutter#151377)
2024-07-06 [email protected] Roll Flutter Engine from e6b09697df1a to d1ebc5fde630 (1 revision) (flutter/flutter#151362)
2024-07-05 49699333+dependabot[bot]@users.noreply.github.com Bump actions/upload-artifact from 4.3.3 to 4.3.4 (flutter/flutter#151354)
2024-07-05 [email protected] Roll Flutter Engine from 4ee09d3b7f3b to e6b09697df1a (2 revisions) (flutter/flutter#151352)
2024-07-05 [email protected] Add tests for color_filtered.0.dart example. (flutter/flutter#151064)
2024-07-05 [email protected] de-duplicate code in analyze.dart (flutter/flutter#151279)
2024-07-05 [email protected] Roll Packages from 754de19 to 97bad7e (1 revision) (flutter/flutter#151350)

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@christopherfujino christopherfujino christopherfujino approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@kenzieschmoll @jakemac53 @natebosch @christopherfujino @sigurdm