Skip to content

Add failuresOnly to defineReflectiveSuite #60069

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
FMorschel opened this issue Feb 6, 2025 · 4 comments
Open

Add failuresOnly to defineReflectiveSuite #60069

FMorschel opened this issue Feb 6, 2025 · 4 comments
Labels
analyzer-test area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug

Comments

@FMorschel
Copy link
Contributor

FMorschel commented Feb 6, 2025

When you run dart test you have an interesting option to add -r failures-only which, as the name suggests, prints only failing tests and the full count at the end.

For SDK tests we can't run with dart test because it produces additional package_config.jsons that mess things up (hopefully Pub Workspaces will fix that). But in the meantime, I think we could at least add a parameter to defineReflectiveSuite that allows us the same behaviour so that when you run the tests you don't need to parse a whole array of irrelevant tests for what you are fixing.

I was working on https://dart-review.googlesource.com/c/sdk/+/408140?tab=checks and this was the output for one of the bots https://ci.chromium.org/ui/p/dart/builders/try/analyzer-linux-release-try/85955/overview.

The tests for pkg\analyzer\test\src\summary\top_level_inference_test.dart, for example, expect huge multiline texts and these take a bunch of space from the VS Code limit debug console line count and with all of the passing tests at the end, some of the relevant output was getting clipped.


CC @DanTup

@dart-github-bot
Copy link
Collaborator

Summary: Users request failuresOnly option for defineReflectiveSuite, mirroring dart test -r failures-only, to improve SDK test output readability and reduce irrelevant log clutter. This is especially helpful in debugging large test suites.

@dart-github-bot dart-github-bot added area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. type-enhancement A request for a change that isn't a bug labels Feb 6, 2025
@bwilkerson
Copy link
Member

@scheglov

@scheglov
Copy link
Contributor

scheglov commented Feb 6, 2025

I don't know if this is possible cleanly.

defineReflectiveSuite does not print anything itself.
All printing is done by package:test itself.
FWIW, I also want and use printing only failures.
To achieve this I hacked the copy of package:test in the SDK third_party.

diff --git a/pkgs/test_api/lib/src/backend/invoker.dart b/pkgs/test_api/lib/src/backend/invoker.dart
index 58f1001d..57201df8 100644
--- a/pkgs/test_api/lib/src/backend/invoker.dart
+++ b/pkgs/test_api/lib/src/backend/invoker.dart
@@ -378,7 +378,7 @@ class Invoker {
     _controller.setState(const State(Status.running, Result.success));

     _runCount++;
-    Chain.capture(() {
+    // Chain.capture(() {
       _guardIfGuarded(() {
         runZoned(() async {
           // Run the test asynchronously so that the "running" state change
@@ -415,7 +415,7 @@ class Invoker {
             zoneSpecification:
                 ZoneSpecification(print: (_, __, ___, line) => _print(line)));
       });
-    }, when: liveTest.test.metadata.chainStackTraces, errorZone: false);
+    // }, when: liveTest.test.metadata.chainStackTraces, errorZone: false);
   }

   /// Runs [callback], in a [Invoker.guard] context if [_guarded] is `true`.
diff --git a/pkgs/test_core/lib/src/scaffolding.dart b/pkgs/test_core/lib/src/scaffolding.dart
index ffcb0083..d060623b 100644
--- a/pkgs/test_core/lib/src/scaffolding.dart
+++ b/pkgs/test_core/lib/src/scaffolding.dart
@@ -3,6 +3,7 @@
 // BSD-style license that can be found in the LICENSE file.

 import 'dart:async';
+import 'dart:io';

 import 'package:meta/meta.dart' show isTest, isTestGroup;
 import 'package:path/path.dart' as p;
@@ -13,6 +14,7 @@ import 'package:test_api/src/backend/invoker.dart'; // ignore: implementation_im

 import 'runner/engine.dart';
 import 'runner/plugin/environment.dart';
+import 'runner/reporter/compact.dart';
 import 'runner/reporter/expanded.dart';
 import 'runner/runner_suite.dart';
 import 'runner/suite.dart';
@@ -60,7 +62,8 @@ Declarer get _declarer {
     var engine = Engine();
     engine.suiteSink.add(suite);
     engine.suiteSink.close();
-    ExpandedReporter.watch(engine, PrintSink(),
+//    ExpandedReporter.watch(engine, PrintSink(),
+    CompactReporter.watch(engine, stdout,
         color: true, printPath: false, printPlatform: false);

     var success = await runZoned(() => Invoker.guard(engine.run),
diff --git a/pkgs/test_core/lib/src/util/io.dart b/pkgs/test_core/lib/src/util/io.dart
index 98bb23b2..39259b29 100644
--- a/pkgs/test_core/lib/src/util/io.dart
+++ b/pkgs/test_core/lib/src/util/io.dart
@@ -20,7 +20,7 @@ import 'pretty_print.dart';

 /// The default line length for output when there isn't a terminal attached to
 /// stdout.
-const _defaultLineLength = 200;
+const _defaultLineLength = 100;

 /// Whether the test runner is running on Google-internal infrastructure.
 final bool inGoogle = Platform.version.contains('(google3)');

@athomas athomas added area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. and removed area-test Cross-cutting test issues (use area- labels for specific failures; not used for package:test). labels Feb 7, 2025
@DanTup
Copy link
Collaborator

DanTup commented Feb 10, 2025

Looks like the failures-only functionality is related to the test reporter and the reporter can only be changed when running through the test runner (eg. not running the script directly as standalone dart scripts). I tried to allow the reporter to be set via an env variable in the past that would handle this, but there were some issues (see dart-lang/test#873).

In any case, it seems that if we do run through dart test (note: we can't currently do this, I messed things up locally to verify this 🙂) it works:

Image

So I think this is unrelated to test_reflective_loader and actually just the same issue that we can't use the pkg:test runner in the SDK. I think if we can "fix" the SDK to allow this, this will just work (as will running the tests with the built-in test runner from VS Code), which I'm hopeful Pub Workspaces will do. There's an issue related to using Pub workspaces in the SDK at #56220.

@lrhn lrhn added legacy-area-analyzer Use area-devexp instead. analyzer-test and removed area-dart-cli Use area-dart-cli for issues related to the 'dart' command like tool. triage-automation See https://github.com/dart-lang/ecosystem/tree/main/pkgs/sdk_triage_bot. labels Feb 12, 2025
@bwilkerson bwilkerson added the P3 A lower priority bug or feature request label Feb 28, 2025
@johnniwinther johnniwinther added area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. and removed legacy-area-analyzer Use area-devexp instead. labels Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-test area-dart-model For issues related to conformance to the language spec in the parser, compilers or the CLI analyzer. P3 A lower priority bug or feature request type-enhancement A request for a change that isn't a bug
Projects
None yet
Development

No branches or pull requests

8 participants