dart-lang / test

A library for writing unit tests in Dart.
https://pub.dev/packages/test
497 stars 214 forks source link

test_api 0.4.4 is incompatible with Flutter #1591

Closed jpnurmi closed 3 years ago

jpnurmi commented 3 years ago

Running flutter test fails for any Dart package that resolves its test_api dependency to 0.4.4. The Flutter SDK itself depends on 0.4.3, but pure Dart packages can pick 0.4.4.

When tests are run with the Flutter tool, the deserialized metadata message in RemoteListener.start() does not contain the allowDuplicateTestNames field. Thus, a null value is casted as non-nullable bool: https://github.com/dart-lang/test/blob/master/pkgs/test_api/lib/src/backend/remote_listener.dart#L110, added in commit https://github.com/dart-lang/test/commit/4de96a734cf466b6d706b2810ed3f623aa054bae.

Steps to reproduce:

Dart project

Flutter project

Why this matters is that for multi-package projects, there's often a mixture of Flutter and Dart packages. A convenient way to run all tests is to use a tool like Melos and let it execute flutter test in all packages. Furthermore, unlike dart test, flutter test --coverage is also conveniently able to collect code coverage for pure Dart packages too.

jakemac53 commented 3 years ago

Interesting, I wasn't aware it was even possible to use flutter test on a Dart package. This isn't a supported use case that we can guarantee will generally work, since the version constraints that would keep it working are enforced by the flutter_test package, which you have no dependency on. So I fully expect this to be broken again in the future, just as an FYI.

If you do want to ensure it continues to work for you, I would suggest manually pinning your test_api version to the version that your current flutter pins to. Note that you don't need to use dependency_overrides for this - you can just use regular dev_dependencies (and should). That will make sure you also get a compatible test_core and test version.

As far as this particular fix though I think we can accept it, as it is pretty harmless.

natebosch commented 3 years ago

Adding a dependency on flutter_test seems like the most straightforward way to make sure that the flutter test command works.

You need a dependency on test to make the dart test command work.

jakemac53 commented 3 years ago

Adding a dependency on flutter_test seems like the most straightforward way to make sure that the flutter test command works.

I don't think that you can add this dependency in a dart project?

natebosch commented 3 years ago

I don't think that you can add this dependency in a dart project?

Not if you want to dart pub get, but if you are using flutter test you may as well use flutter pub get.

jakemac53 commented 3 years ago

Not if you want to dart pub get, but if you are using flutter test you may as well use flutter pub get.

Oh interesting that does work, ya that seems like the best option then. Do you think we should still do https://github.com/dart-lang/test/pull/1592 at all? I am leaning towards not given this fix.

jakemac53 commented 3 years ago

We should probably file an issue on flutter to check for a flutter_test dependency instead?

natebosch commented 3 years ago

Do you think we should still do #1592 at all? I am leaning towards not given this fix.

I would be fine either way. I think striving for backwards compatibility in test_api as much as possible is a good thing in general, since we still have leaky abstractions and external dependencies on the details.

jakemac53 commented 3 years ago

Ok, I will go ahead and roll with it. I do think we should file an issue on flutter test to check for the dep though, as its all but guaranteed similar issues will happen at some point with no version constraints taking effect.

jakemac53 commented 3 years ago

Packages are published @jpnurmi

jpnurmi commented 3 years ago

Great, thank you! :+1:

jakemac53 commented 3 years ago

Filed https://github.com/flutter/flutter/issues/91107 for the flutter team