Repository: VeryGoodOpenSource/very_good_cli (dart)

Issue:
fix: `--min-coverage` does not work when running with `--recursive`
**Description**

Currently if you run `very_good test --min-coverage 100 --recursive` it will go through each package it finds and run the tests. But if any of those packages don't have 100% test coverage there won't be an error displaying that.

Here I am running the command at the root of my project:
![image](https://user-images.githubusercontent.com/15887627/167681205-221aa8a0-1d05-44a3-99c7-d8fa6b9908d6.png)

It finds three packages, which is correct, but all three of them don't have 100% test coverage:
- `umbra_flutter`: ![image](https://user-images.githubusercontent.com/15887627/167681340-5fe86332-8c71-4150-a766-5ac2e4905688.png)
- `umbra_core`: ![image](https://user-images.githubusercontent.com/15887627/167681422-7a7d4eb3-e86e-4af9-83f2-0a6c9ac010eb.png)
- `umbra_cli`: ![image](https://user-images.githubusercontent.com/15887627/167681473-c83ed236-e26e-4617-a827-c2c5e49fbe84.png)

**Steps To Reproduce**

1. Create a repo with two packages, and ensure they don't have full test coverage
2. Run `very_good test --min-coverage 100 --recursive` at the root
3. Tests will succeed but no coverage is reported.

**Expected Behavior**
I expected that I was able to combine these flags together to run all my tests at the highest level and confirm that they all have the coverage I expected.

Related: https://github.com/VeryGoodOpenSource/very_good_cli/issues/383

Pull request:
<!--
  Thanks for contributing!

  Provide a description of your changes below and a general summary in the title

  Please look at the following checklist to ensure that your PR can be accepted quickly:
-->

## Description

Fixes bug where running `--min-coverage` with `--recursive` would not throw error if min coverage was not met.

Closes #389 

## Type of Change

<!--- Put an `x` in all the boxes that apply: -->

- [ ] ✨ New feature (non-breaking change which adds functionality)
- [x] 🛠️ Bug fix (non-breaking change which fixes an issue)
- [ ] ❌ Breaking change (fix or feature that would cause existing functionality to change)
- [ ] 🧹 Code refactor
- [ ] ✅ Build configuration change
- [ ] 📝 Documentation
- [ ] 🗑️ Chore

Patch:
diff --git a/lib/src/cli/cli.dart b/lib/src/cli/cli.dart
index 2f98a8573..49ec55b09 100644
--- a/lib/src/cli/cli.dart
+++ b/lib/src/cli/cli.dart
@@ -105,7 +105,26 @@ class _Cmd {
     required bool Function(FileSystemEntity) where,
     String cwd = '.',
   }) {
-    return Directory(cwd).listSync(recursive: true).where(where).map(run);
+    final directories =
+        Directory(cwd).listSync(recursive: true).where(where).toList()
+          ..sort((a, b) {
+            /// Linux and macOS have different sorting behaviors
+            /// regarding the order that the list of folders/files are returned.
+            /// To ensure consistency across platforms, we apply a
+            /// uniform sorting logic.
+            final aSplit = p.split(a.path);
+            final bSplit = p.split(b.path);
+            final aLevel = aSplit.length;
+            final bLevel = bSplit.length;
+
+            if (aLevel == bLevel) {
+              return aSplit.last.compareTo(bSplit.last);
+            } else {
+              return aLevel.compareTo(bLevel);
+            }
+          });
+
+    return directories.map(run);
   }
 
   static void _throwIfProcessFailed(
diff --git a/lib/src/cli/flutter_cli.dart b/lib/src/cli/flutter_cli.dart
index f4ee1cb60..2097c2409 100644
--- a/lib/src/cli/flutter_cli.dart
+++ b/lib/src/cli/flutter_cli.dart
@@ -146,15 +146,15 @@ class Flutter {
     FlutterTestRunner testRunner = flutterTest,
     GeneratorBuilder buildGenerator = MasonGenerator.fromBundle,
   }) async {
-    final lcovPath = p.join(cwd, 'coverage', 'lcov.info');
-    final lcovFile = File(lcovPath);
+    return _runCommand<int>(
+      cmd: (cwd) async {
+        final lcovPath = p.join(cwd, 'coverage', 'lcov.info');
+        final lcovFile = File(lcovPath);
 
-    if (collectCoverage && lcovFile.existsSync()) {
-      await lcovFile.delete();
-    }
+        if (collectCoverage && lcovFile.existsSync()) {
+          await lcovFile.delete();
+        }
 
-    final results = await _runCommand<int>(
-      cmd: (cwd) async {
         void noop(String? _) {}
         final target = DirectoryGeneratorTarget(Directory(p.normalize(cwd)));
         final workingDirectory = target.dir.absolute.path;
@@ -211,30 +211,33 @@ class Flutter {
             ],
             stdout: stdout ?? noop,
             stderr: stderr ?? noop,
-          ),
-        ).whenComplete(() {
-          if (optimizePerformance) {
-            File(p.join(cwd, 'test', '.test_optimizer.dart')).delete().ignore();
-          }
-        });
+          ).whenComplete(() async {
+            if (optimizePerformance) {
+              File(p.join(cwd, 'test', '.test_optimizer.dart'))
+                  .delete()
+                  .ignore();
+            }
+
+            if (collectCoverage) {
+              assert(lcovFile.existsSync(), 'coverage/lcov.info must exist');
+            }
+
+            if (minCoverage != null) {
+              final records = await Parser.parse(lcovPath);
+              final coverageMetrics = _CoverageMetrics.fromLcovRecords(
+                records,
+                excludeFromCoverage,
+              );
+              final coverage = coverageMetrics.percentage;
+
+              if (coverage < minCoverage) throw MinCoverageNotMet(coverage);
+            }
+          }),
+        );
       },
       cwd: cwd,
       recursive: recursive,
     );
-
-    if (collectCoverage) {
-      assert(lcovFile.existsSync(), 'coverage/lcov.info must exist');
-    }
-    if (minCoverage != null) {
-      final records = await Parser.parse(lcovPath);
-      final coverageMetrics = _CoverageMetrics.fromLcovRecords(
-        records,
-        excludeFromCoverage,
-      );
-      final coverage = coverageMetrics.percentage;
-      if (coverage < minCoverage) throw MinCoverageNotMet(coverage);
-    }
-    return results;
   }
 
   static T _overrideAnsiOutput<T>(bool? enableAnsiOutput, T Function() body) =>


Test patch:
diff --git a/test/src/cli/flutter_cli_test.dart b/test/src/cli/flutter_cli_test.dart
index dc5131f4c..4b13a2aa6 100644
--- a/test/src/cli/flutter_cli_test.dart
+++ b/test/src/cli/flutter_cli_test.dart
@@ -904,6 +904,130 @@ void main() {
         directory.delete(recursive: true).ignore();
       });
 
+      test(
+        'runs tests w/coverage + min-coverage 100 + recursive (pass)',
+        () async {
+          final directory = Directory.systemTemp.createTempSync();
+          File(p.join(directory.path, 'pubspec.yaml')).createSync();
+          Directory(p.join(directory.path, 'test')).createSync();
+
+          final nestedDirectory = Directory(p.join(directory.path, 'test'))
+            ..createSync();
+          File(p.join(nestedDirectory.path, 'pubspec.yaml')).createSync();
+          Directory(p.join(nestedDirectory.path, 'test')).createSync();
+
+          await expectLater(
+            Flutter.test(
+              cwd: directory.path,
+              collectCoverage: true,
+              minCoverage: 100,
+              recursive: true,
+              stdout: stdoutLogs.add,
+              stderr: stderrLogs.add,
+              testRunner: testRunner(
+                Stream.fromIterable(
+                  [
+                    const DoneTestEvent(success: true, time: 0),
+                    const ExitTestEvent(exitCode: 0, time: 0),
+                  ],
+                ),
+                onStart: () {
+                  File(p.join(directory.path, 'coverage', 'lcov.info'))
+                    ..createSync(recursive: true)
+                    ..writeAsStringSync(lcov100);
+                  File(p.join(nestedDirectory.path, 'coverage', 'lcov.info'))
+                    ..createSync(recursive: true)
+                    ..writeAsStringSync(lcov100);
+                },
+              ),
+              logger: logger,
+            ),
+            completion(equals([ExitCode.success.code, ExitCode.success.code])),
+          );
+
+          expect(
+            stdoutLogs,
+            unorderedEquals([
+              'Running "flutter test" in '
+                  '${p.dirname(nestedDirectory.path)}...\n',
+              contains('All tests passed!'),
+              'Running "flutter test" in ${p.dirname(directory.path)}...\n',
+              contains('All tests passed!'),
+            ]),
+          );
+          expect(testRunnerArgs, equals(['--coverage', '--coverage']));
+
+          directory.delete(recursive: true).ignore();
+          nestedDirectory.delete(recursive: true).ignore();
+        },
+      );
+
+      test(
+        'runs tests w/coverage + min-coverage 100 + recursive (fail)',
+        () async {
+          final directory = Directory.systemTemp.createTempSync();
+          File(p.join(directory.path, 'pubspec.yaml')).createSync();
+          Directory(p.join(directory.path, 'test')).createSync();
+
+          final nestedDirectory = Directory(p.join(directory.path, 'test'))
+            ..createSync();
+          File(p.join(nestedDirectory.path, 'pubspec.yaml')).createSync();
+          Directory(p.join(nestedDirectory.path, 'test')).createSync();
+
+          await expectLater(
+            Flutter.test(
+              cwd: directory.path,
+              collectCoverage: true,
+              minCoverage: 100,
+              recursive: true,
+              stdout: stdoutLogs.add,
+              stderr: stderrLogs.add,
+              testRunner: testRunner(
+                Stream.fromIterable(
+                  [
+                    const DoneTestEvent(success: true, time: 0),
+                    const ExitTestEvent(exitCode: 0, time: 0),
+                  ],
+                ),
+                onStart: () {
+                  File(p.join(directory.path, 'coverage', 'lcov.info'))
+                    ..createSync(recursive: true)
+                    ..writeAsStringSync(lcov100);
+                  File(p.join(nestedDirectory.path, 'coverage', 'lcov.info'))
+                    ..createSync(recursive: true)
+                    ..writeAsStringSync(lcov95);
+                },
+              ),
+              logger: logger,
+            ),
+            throwsA(
+              isA<MinCoverageNotMet>().having(
+                (e) => e.coverage,
+                'coverage',
+                95.0,
+              ),
+            ),
+          );
+
+          expect(
+            stdoutLogs,
+            unorderedEquals([
+              'Running "flutter test" in '
+                  '${p.dirname(directory.path)}...\n',
+              contains('All tests passed!'),
+              'Running "flutter test" in '
+                  '${p.dirname(nestedDirectory.path)}...\n',
+              contains('All tests passed!'),
+            ]),
+          );
+          expect(stderrLogs, isEmpty);
+          expect(testRunnerArgs, equals(['--coverage', '--coverage']));
+
+          directory.delete(recursive: true).ignore();
+          nestedDirectory.delete(recursive: true).ignore();
+        },
+      );
+
       test('runs tests w/optimizations (passing)', () async {
         final directory = Directory.systemTemp.createTempSync();
         final originalVars = <String, dynamic>{'package-root': directory.path};