`dart test` fails with an incorrect message when there are issues compiling package:test

[devoncarew]

When running dart test, if there are issues with the front end compiling package:test, we emit the compilation messages, but then emit misleading messages about adding a dependency on package:test. This project already has a correct dep on test.

See the last two lines of the output below:

dart test       
Precompiling executable... (5.3s)
Failed to precompile test:test:
../../../.pub-cache/hosted/pub.dartlang.org/http_multi_server-2.1.0/lib/src/multi_headers.dart:97:8: Error: The method 'MultiHeaders.add' has fewer named arguments than those of overridden method 'HttpHeaders.add'.
  void add(String name, Object value) {
       ^
org-dartlang-sdk:///sdk/lib/_http/http.dart:699:8: Context: This is the overridden method ('add').
  void add(String name, Object value,
       ^
../../../.pub-cache/hosted/pub.dartlang.org/http_multi_server-2.1.0/lib/src/multi_headers.dart:129:8: Error: The method 'MultiHeaders.set' has fewer named arguments than those of overridden method 'HttpHeaders.set'.
  void set(String name, Object value) {
       ^
org-dartlang-sdk:///sdk/lib/_http/http.dart:708:8: Context: This is the overridden method ('set').
  void set(String name, Object value,
       ^
Failed to precompile test:test:
../../../.pub-cache/hosted/pub.dartlang.org/http_multi_server-2.1.0/lib/src/multi_headers.dart:97:8: Error: The method 'MultiHeaders.add' has fewer named arguments than those of overridden method 'HttpHeaders.add'.
  void add(String name, Object value) {
       ^
org-dartlang-sdk:///sdk/lib/_http/http.dart:699:8: Context: This is the overridden method ('add').
  void add(String name, Object value,
       ^
../../../.pub-cache/hosted/pub.dartlang.org/http_multi_server-2.1.0/lib/src/multi_headers.dart:129:8: Error: The method 'MultiHeaders.set' has fewer named arguments than those of overridden method 'HttpHeaders.set'.
  void set(String name, Object value) {
       ^
org-dartlang-sdk:///sdk/lib/_http/http.dart:708:8: Context: This is the overridden method ('set').
  void set(String name, Object value,
       ^
You need to add a dependency on package:test.
Try running `dart pub add test`.

cc @sigurdm, @bkonyi

[sigurdm]

Either we make the error message more vague ("Make sure you have a working dependency on package:test"), or we need the pub getExecutableForCommand to have a more fine grained error reporting.

cc @jonasfj

[jonasfj]

Maybe failure to compile should print something like:

Compilation of failed, consider reporting issue with package:<package>.

This generally applies to dart run <package>[:<executable>].

I guess we need getExecutableForCommand to throw either:

• CommandResolutionFailedException, if it can't find the executable,
• CommandCompilationFailedException, if the executable can't be compiled (this doesn't happen today, we just throw CommandResolutionFailedException).

[sigurdm]

I think to do the correct handling we need to have subclasses for the different cases:

• The string contained: '.' '/' or '': file not found
• the string was <package>:<binary> or <package>, and <package> was not in dependencies: package not found
• the string was <package>:<binary> and binary was not found: binary was not found in package.
• an error happened during compilation of binary: compilation error
• Some other error happened (Not sure we have anything in this category)

And only in the second case dart test should suggest adding test to the dependencies. In the other cases it cannot do much more than printing the error. This should hopefully be very rare.

We could also consider failure to compile test:test so esoteric, that we don't really want to give it special handling....

[jonasfj]

We could also consider failure to compile test:test so esoteric, that we don't really want to give it special handling....

Yeah, this really should be rare that old packages are incompatible with newer SDKs. Which is the underlying problem here.

On the flip side: providing better error messages in general would help people doing local development and using dart run <package> on path-dependencies.

[sigurdm]

On the flip side: providing better error messages in general would help people doing local development and using dart run on path-dependencies.

The textual error message is good enough currently: (https://github.com/dart-lang/pub/blob/master/lib/src/executable.dart#L270-L338). It is only programmatically that we don't distinguish between the cases. Such that dart test knows what suggestion to give.

[devoncarew]

We could verify that there is a correct dep before we run - then we can emit that message (You need to add a dependency on package:test. Try running 'dart pub add test'.) w/o risk of it being a false positive.

I think then we could just pass through any failures from the pub run test w/o trying to infer what the cause of the problem was.