I think all local tests are passing now. (perhaps we should refactor some tests, so package_config.json doesn't have to be created uniquely for so many tests....)

Not sure what happens in the customer tests, but I think it might be unrelated to this change.

Gentle ping on this one

@spydon I know you are one of the melos maintainers. I would suggest you to keep track of this pull request. I would say this is a blocker to release the workspaces support to melos.

@spydon I know you are one of the melos maintainers. I would suggest you to keep track of this pull request. I would say this is a blocker to release the workspaces support to melos.

Thanks, I'm already subscribed to it. I've created one of the issues above. :)

Do you think you could help me out with the customer_testing tests?

I cannot understand how my change could cause errors like:

 00:11 +8: /b/s/w/ir/x/t/flutter_customer_testing.super_editor.UDMNRS/tests/super_text_layout/test/super_text_test.dart: SuperText RenderSuperTextLayout calculates line count for one line of text
| ══╡ EXCEPTION CAUGHT BY FLUTTER TEST FRAMEWORK ╞════════════════════════════════════════════════════
| The following TestFailure was thrown running a test:
| Expected: <1>
|   Actual: <2>
|
| When the exception was thrown, this was the stack:
| #4      main.<anonymous closure>.<anonymous closure>.<anonymous closure>.<anonymous closure> (file:///b/s/w/ir/x/t/flutter_customer_testing.super_editor.UDMNRS/tests/super_text_layout/test/super_text_test.dart:203:11)
| <asynchronous suspension>
| #5      testWidgets.<anonymous closure>.<anonymous closure> (package:flutter_test/src/widget_tester.dart:193:15)
| <asynchronous suspension>
| #6      TestWidgetsFlutterBinding._runTestBody (package:flutter_test/src/binding.dart:1064:5)
| <asynchronous suspension>
| <asynchronous suspension>
| (elided one frame from package:stack_trace)

But also I don't want to land this with red tests I don't understand...

From what I can tell, the tests depend on the Roboto font being available (src). Before, it was getting bundled from the golden_toolkit package.

To verify yourself, clone the super_editor repo and navigate to the failing test (https://github.com/superlistapp/super_editor/blob/dc7013f2f9be145d655e891eb2502baf2d6795a1/super_text_layout/test/super_text_test.dart#L192). To see what assets are getting bundled within the test code, add

final assets = (await AssetManifest.loadFromAssetBundle(rootBundle))
  .listAssets();
print(assets.join('\n'));

When on master, I see

packages/golden_toolkit/fonts/Roboto-Regular.ttf

which I don't see when I have this PR checked out.

I understand now, thanks!

The super_text_layout package dev_depends on golden_toolkit which has the font as an asset. This PR does not allow you to use assets from your dev_dependencies.

So in general when testing, you might want to use assets from your dev_dependencies, but not when building for deployment I had not really realized that.

I see three ways forward:

1. We teach flutter_tool to bundle different assets when testing?
   I wonder if this is realistic? Not sure exactly what this would require. @andrewkolos @bkonyi
2. We could also revert that part of this PR, so that you still bundle all assets from dev_dependencies (would no longer solve package assets declared under dev_dependency shouldn't be included in release bundle #79261)
3. We could also require all assets to come from dependencies, and update the customer test to have golden_toolkit in its dependencies

So in general when testing, you might want to use assets from your dev_dependencies, but not when building for deployment I had not really realized that.

Yeah, I was thinking the same. There are probably edge cases where including a font from a dev dependency would unexpectedly cause a test to pass (i.e. the font was meant to be included from a regular dependency or the assets section in your pubspec); but I expect such a case would be extremely rare. That is to say, I think it is fine to continue to include assets from dev dependencies in tests—at least for now.

I think including assets from dev dependencies in tests is fine since I assume our main concern here is reducing bundle size in shipped applications.

Attached g3fix https://critique.corp.google.com/cl/721374511

Thanks for addressing the comments! LGTM!

Seems #161826 and #161343 are interferring with this.

@camsim99 do you know the desired semantics around plugins and assets from dev-dependencies? Should they be included in debug builds but not in release? Or should they be included for testing only (or not at all)?

Seems #161826 and #161343 are interferring with this.

@camsim99 do you know the desired semantics around plugins and assets from dev-dependencies? Should they be included in debug builds but not in release? Or should they be included for testing only (or not at all)?

My understanding is that we are working towards what I think #79261 proposes, i.e. assets from dev dependencies should be included in debug builds but not release. The changes I made are irrespective to testing I think. cc @matanlurey for input

@matanlurey are you ok with landing this in this state?
I'm not sure it is really meaningful to separate the last changes.

Hi @andrewkolos @matanlurey @bkonyi
Many thanks for your contribution into reviewing the PR to make it even better

Could you clarify when it will be possible to merge it?

autosubmit label was removed for flutter/flutter/160443, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@sigurdm I suppose this PR could not land on the current flutter stable release, right?
Do you have any idea if this is going to be cherry-picked to the current stable version or will we need to wait for the next stable release? :')

When will it be possible to have this fix on stable

When will it be possible to have this fix on stable

Since it's in beta now I would guess that it will land in stable around the end of August.