Add error for empty groups

pkgs/test/CHANGELOG.md

@@ -1,4 +1,7 @@
-## 1.24.9-wip
+## 1.25.0-wip
+
+* Consider `group` with no test cases, but with another callback such as
+  `setUp`, to be test failures.
 
 ## 1.24.8
 

pkgs/test/pubspec.yaml

@@ -1,5 +1,5 @@
 name: test
-version: 1.24.9-wip
+version: 1.25.0-wip
 description: >-
   A full featured library for writing and running Dart tests across platforms.
 repository: https://github.com/dart-lang/test/tree/master/pkgs/test
@@ -35,7 +35,7 @@ dependencies:
 
   # Use an exact version until the test_api and test_core package are stable.
   test_api: 0.6.1
-  test_core: 0.5.8
+  test_core: 0.5.9
 
   typed_data: ^1.3.0
   web_socket_channel: ^2.0.0

pkgs/test/test/runner/configuration/duplicate_names_test.dart

@@ -18,65 +18,93 @@ void main() {
 
   group('duplicate names', () {
     group('can be disabled for', () {
-      for (var function in ['group', 'test']) {
-        test('${function}s', () async {
-          await d
-              .file('dart_test.yaml',
-                  jsonEncode({'allow_duplicate_test_names': false}))
-              .create();
-
-          var testName = 'test';
-          await d.file('test.dart', '''
-          import 'package:test/test.dart';
-
-          void main() {
-            $function("$testName", () {});
-            $function("$testName", () {});
-          }
-        ''').create();
-
-          var test = await runTest([
-            'test.dart',
-            '--configuration',
-            p.join(d.sandbox, 'dart_test.yaml')
-          ]);
-
-          expect(
-              test.stdout,
-              emitsThrough(contains(
-                  'A test with the name "$testName" was already declared.')));
-
-          await test.shouldExit(1);
-        });
-      }
+      test('groups', () async {
+        await d
+            .file('dart_test.yaml',
+                jsonEncode({'allow_duplicate_test_names': false}))
+            .create();
+
+        await d.file('test.dart', _identicalGroupnames).create();
+
+        var test = await runTest([
+          'test.dart',
+          '--configuration',
+          p.join(d.sandbox, 'dart_test.yaml')
+        ]);
+
+        expect(
+            test.stdout,
+            emitsThrough(contains(
+                'A test with the name "identical name" was already declared.')));
+
+        await test.shouldExit(1);
+      });
+      test('tests', () async {
+        await d
+            .file('dart_test.yaml',
+                jsonEncode({'allow_duplicate_test_names': false}))
+            .create();
+
+        await d.file('test.dart', _identicalTestNames).create();
+
+        var test = await runTest([
+          'test.dart',
+          '--configuration',
+          p.join(d.sandbox, 'dart_test.yaml')
+        ]);
+
+        expect(
+            test.stdout,
+            emitsThrough(contains(
+                'A test with the name "identical name" was already declared.')));
+
+        await test.shouldExit(1);
+      });
     });
     group('are allowed by default for', () {
-      for (var function in ['group', 'test']) {
-        test('${function}s', () async {
-          var testName = 'test';
-          await d.file('test.dart', '''
-          import 'package:test/test.dart';
-
-          void main() {
-            $function("$testName", () {});
-            $function("$testName", () {});
-
-            // Needed so at least one test runs when testing groups.
-            test('a test', () {
-              expect(true, isTrue);
-            });
-          }
-        ''').create();
-
-          var test = await runTest(
-            ['test.dart'],
-          );
-
-          expect(test.stdout, emitsThrough(contains('All tests passed!')));
-
-          await test.shouldExit(0);
-        });
-      }
+      test('groups', () async {
+        await d.file('test.dart', _identicalGroupnames).create();
+
+        var test = await runTest(
+          ['test.dart'],
+        );
+
+        expect(test.stdout, emitsThrough(contains('All tests passed!')));
+
+        await test.shouldExit(0);
+      });
+      test('tests', () async {
+        await d.file('test.dart', _identicalTestNames).create();
+
+        var test = await runTest(
+          ['test.dart'],
+        );
+
+        expect(test.stdout, emitsThrough(contains('All tests passed!')));
+
+        await test.shouldExit(0);
+      });
     });
   });
 }
+
+const _identicalTestNames = '''
+import 'package:test/test.dart';
+
+void main() {
+  test('identical name', () {});
+  test('identical name', () {});
+}
+''';
+const _identicalGroupnames = '''
+import 'package:test/test.dart';
+
+void main() {
+  group('identical name', () {
+    test('foo', () {});
+  });
+  group('identical name', () {
+    test('bar', () {});
+  });
+}
+''';

pkgs/test_api/CHANGELOG.md

@@ -1,5 +1,8 @@
 ## 0.6.2-wip
 
+* Consider `group` with no test cases, but with another callback such as
+  `setUp`, to be test failures.
+
 ## 0.6.1
 
 * Drop support for null unsafe Dart, bump SDK constraint to `3.0.0`.

pkgs/test_api/lib/src/backend/declarer.dart

@@ -13,6 +13,7 @@ import 'group_entry.dart';
 import 'invoker.dart';
 import 'metadata.dart';
 import 'test.dart';
+import 'test_failure.dart';
 
 /// A class that manages the state of tests as they're declared.
 ///
@@ -327,6 +328,16 @@ class Declarer {
       }
       return entry;
     }).toList();
+    if (_parent != null &&
+        entries.isEmpty &&
+        (_setUps.isNotEmpty ||
+            _setUpAlls.isNotEmpty ||
+            _tearDowns.isNotEmpty ||
+            _tearDownAlls.isNotEmpty)) {
+      entries.add(LocalTest(_name ?? 'Empty group', _metadata, () {
+        throw TestFailure('No tests declared in group');
+      }));
+    }
 
     return Group(_name ?? '', entries,
         metadata: _metadata,

pkgs/test_api/test/backend/declarer_test.dart

@@ -410,6 +410,31 @@ void main() {
       });
     });
 
+    test('disallows empty groups if there is another lifecycle callback',
+        () async {
+      var entries = declare(() {
+        group('group', () {
+          setUp(() {});
+        });
+      });
+
+      expect(entries, hasLength(1));
+      var testGroup = entries.single as Group;
+      expect(testGroup.entries, hasLength(1));
+      await _runTest(testGroup.entries.single as Test, shouldFail: true);
+    });
+    test('allows fully empty groups', () async {
+      var entries = declare(() {
+        group('group', () {
+          // Might be empty with a TODO or while editing
+        });
+      });
+
+      expect(entries, hasLength(1));
+      var testGroup = entries.single as Group;
+      expect(testGroup.entries, isEmpty);
+    });
+
     group('.setUp()', () {
       test('is scoped to the group', () async {
         var setUpRun = false;

pkgs/test_core/CHANGELOG.md

@@ -1,3 +1,8 @@
+## 0.5.9-wip
+
+* Consider `group` with no test cases, but with another callback such as
+  `setUp`, to be test failures.
+
 ## 0.5.8
 
 * Move scaffolding definitions to a non-deprecated library.

pkgs/test_core/pubspec.yaml

@@ -1,5 +1,5 @@
 name: test_core
-version: 0.5.8
+version: 0.5.9-wip
 description: A basic library for writing tests and running them on the VM.
 repository: https://github.com/dart-lang/test/tree/master/pkgs/test_core