Update to the latest package:test

[natebosch]

• Update the pinned version of test to the latest published.
• Add support for reading the source.location of messages to the JS
  interop library.
• Update host.dart to support the new communication pattern with the
  test frame. See Simplify the communication between the browser host and iframe dart-lang/test#2065
• Use Runtime.edge for the edge browser. We may deprecate or remove
  the constant. Edge is a more appropriate value for this usage.

[flutter-dashboard]

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption to this rule, contact Hixie or stuartmorgan on the #hackers channel in Chat (don't just cc them here, they won't see it! Use Discord!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

[natebosch]

It looks like there is at least one place where this code is being used against an older version of test_api.

Running `dart pub get` in 'engine/src/flutter/lib/web_ui'
dev/edge.dart:27:45: Error: Member not found: 'edge'.
  Runtime get packageTestRuntime => Runtime.edge;
                                            ^^^^

[natebosch]

I don't expect this to have any behavior change. Both Runtime instances only differ in small ways, and the edge runtime is more correct for the edge browser. The identifier is the most important bit, and the code already ensures that the identifier matches the registered platform.

Registered at

engine/lib/web_ui/dev/steps/run_suite_step.dart

Line 96 in 98d3eba

browserEnvironment.packageTestRuntime,

Selected by the args with

engine/lib/web_ui/dev/steps/run_suite_step.dart

Line 84 in 98d3eba

'--platform=${browserEnvironment.packageTestRuntime.identifier}',

There are other uses which I think are not meaningful, but I'm less certain.:
It impacts a configuration to webdriver,

engine/lib/web_ui/dev/webdriver_browser.dart

Line 65 in 98d3eba

uri: driverUri, desired: <String, dynamic>{'browserName': packageTestRuntime.identifier});

and a fragment put into the URL

engine/lib/web_ui/dev/test_platform.dart

Line 950 in 98d3eba

'browser': _browserEnvironment.packageTestRuntime.identifier

I don't expect either to have a meaningful or user visible impact, but @yjbanov might know if there is more I should be checking.

[natebosch]

It looks like there is at least one place where this code is being used against an older version of test_api.

@yjbanov - How does this package/repo handle these dependencies?

I see that package:ui has a pubspec which uses test_api as a dev_dependency and leaves the version unconstrained, despite importing that package under lib/.

engine/lib/web_ui/pubspec.yaml

Line 42 in 98d3eba

test_api: any

Is there a reason test_api isn't a normal dependency? Where can I constrain this code to only run against new enough versions of package:test_api?

[natebosch]

How does this package/repo handle these dependencies?

I would have expected the DEPS file to handle it. I don't know why we'd see an older version that causes the static failure.

engine/DEPS

Lines 480 to 481 in 98d3eba

	'src/third_party/dart/third_party/pkg/test':
	Var('dart_git') + '/test.git@367aa397ab1cb08755f8a7582ab21101ceb9d29b',

Are there places in CI we are using stale packages?

[eyebrowsoffire]

We have been meaning to switch to using the DEPS provided packages in web_ui, but we haven't done so. We are just relying on pub currently. If it simplifies things to switch some of the packages to use the path from the gclient checkout (as dart:ui does) I think that would probably be a good change

[natebosch]

We are just relying on pub currently

Hmm, I don't think I understand the strategy this repo takes to writing pubspecs... I think the intention appears to be to pin most packages to a single version? I'll try pinning to a newer package:test in the pubspecs which reference that package...

[stuartmorgan]

test-exempt: is a test

[yjbanov]

We are just relying on pub currently

Hmm, I don't think I understand the strategy this repo takes to writing pubspecs... I think the intention appears to be to pin most packages to a single version? I'll try pinning to a newer package:test in the pubspecs which reference that package...

Sorry, my bad. I should have paid attention when reviewing https://github.com/flutter/engine/pull/34986/files#diff-8cbbe86127b013f147ecc979fa6d1e02e96c45fdb7491e91bc44ea151f11805c. We do want to pin at least all the directly imported packages. Feel free to submit whatever works, and I'll do a clean-up of pubspecs as a follow-up.

[natebosch]

@yjbanov - I cannot find any relevant errors in the CI logs. Do you know where I should be looking for the cause of the failure?

What is the typical process for rolling to new version of package:test in this repo?

[natebosch]

I was able to track down the reason for the failure - some discussion in #46905 Fix is to update the copy of host.dart in this repo.

In that PR I was confused by some failures that now appear to have been intermittent or flakes.