Use compact reporters by default for direct runs - #2685
Conversation
Since we cannot import `dart:io` for tests running on the web we historically defaulted to the expanded reporter as the only alternative to the compact reporter. This doesn't match most users preferences since it makes a verbose output in all cases. Default instead to the failures only reporter which is quieter, and add a platform specific library to automatically switch to the compact reporter when it's supported by both the Dart platform and the runtime environment.
PR HealthChangelog Entry ✔️
Changes to files need to be accounted for in their respective changelogs. This check can be disabled by tagging the PR with |
| // for details. All rights reserved. Use of this source code is governed by a | ||
| // BSD-style license that can be found in the LICENSE file. | ||
|
|
||
| export 'direct_stub.dart' if (dart.library.io) 'direct_io.dart'; |
There was a problem hiding this comment.
nit: maybe add a comment to this library explaining what it is
jakemac53
left a comment
jakemac53
left a comment
There was a problem hiding this comment.
I think this is probably OK to release as non-breaking but I wouldn't be super surprised if somebody does get broken by it 🤷♂️
I think we can plan to resolve whatever comes up if it does. Overall I expect this to have little impact because most invocations use the test runner, but for the places where it does have an impact I think it's very likely to do more good than harm.. |
Yep, broken 😆 I'll just update the tests 👍 |
Package:test changed to the compact reporter by default: dart-lang/test#2685 Opt in to the verbose reporter where possible and remove test name from the compact reporter expectations. We resolve `package:test` via `pub get` in these test projects (instead of pinning them to what we pull in via `DEPS`). So, these tests are not hermetic. We could make the tests hermetic by rewriting the pubspecs to pull in packages via DEPS in the style of https://dart-review.googlesource.com/c/sdk/+/515600/11/pkg/dartdev/test/native_assets/helpers.dart Closes: #63697 Change-Id: Id56e3ceba6b66b567cac3bd3e3e132ae68f6a45d Cq-Include-Trybots: luci.dart.try:pkg-linux-debug-try,pkg-linux-release-arm64-try,pkg-linux-release-try,pkg-mac-release-arm64-try,pkg-mac-release-try,pkg-win-release-try,pkg-win-release-arm64-try Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/518360 Reviewed-by: Slava Egorov <vegorov@google.com> Commit-Queue: Slava Egorov <vegorov@google.com>
|
This really confused me - I have PowerShell aliases to run tests, one of which filters out most of the noise with But, I'm in favour of the change (because now I don't need the |
I think it's probably a good idea for us to support more environment variables like this in the test runner in general, it makes sense to me to honor it in the direct run case as well. |
4 participants
Since we cannot import
dart:iofor tests running on the web wehistorically defaulted to the expanded reporter as the only alternative
to the compact reporter. This doesn't match most users preferences since
it makes a verbose output in all cases.
Default instead to the failures only reporter which is quieter, and add
a platform specific library to automatically switch to the compact
reporter when it's supported by both the Dart platform and the runtime
environment.