Add ignore flag to health workflows (#218)

* Add `ignore` flag to health workflows

* Switch to glob

* Fix multiline

* Add default glob

* Delete multiline

* Fix errors

* Fix health commenting

* Propagate ignore

* Switch to specific ignores

* Add defaults

* Add test repos

* Start adding health tests

* Add golden files

* Fix changelog

* Fix coverage

* Fix breaking

* More fixes

* Merge

* Fixes

* Move

* Fix tests

* Ignore test data licenses

* Add golden without license

* Fix license

* Rename

* Fix

* Fix analyze issue

* Add args printing

* Add defaults

* Add more defaults

* Remove empty

* Match recursive

* Add docs

* Add link to docs

Author: mosuem

.github/workflows/health.yaml

@@ -1,4 +1,4 @@
-# A CI configuration to check PR health.
+# A CI configuration to check PR health. Check the docs at https://github.com/dart-lang/ecosystem/tree/main/pkgs/firehose#health.
 
 name: Health
 
@@ -27,7 +27,13 @@ name: Health
 #       coverage_web: false
 #       upload_coverage: false
 #       use-flutter: true
-
+#       use-flutter: true
+#       use-flutter: true
+#       ignore_license: "**.g.dart"
+#       ignore_coverage: "**.mock.dart,**.g.dart"
+#       ignore_packages: "pkgs/helper_package"
+#       checkout_submodules: false
+#       experiments: "native-assets"
 
 on:
   workflow_call:
@@ -82,6 +88,21 @@ on:
         default: false
         required: false
         type: boolean
+      ignore_license:
+        description: Which files to ignore for the license check.
+        default: "\"\""
+        required: false
+        type: string
+      ignore_coverage:
+        description: Which files to ignore for the coverage check.
+        default: "\"\""
+        required: false
+        type: string
+      ignore_packages:
+        description: Which packages to ignore.
+        default: "\"\""
+        required: false
+        type: string
       checkout_submodules:
         description: Whether to checkout submodules of git repositories.
         default: false
@@ -104,6 +125,7 @@ jobs:
       warn_on: ${{ inputs.warn_on }}
       local_debug: ${{ inputs.local_debug }}
       use-flutter: ${{ inputs.use-flutter }}
+      ignore_packages: ${{ inputs.ignore_packages }}
       checkout_submodules: ${{ inputs.checkout_submodules }}
 
   changelog:
@@ -116,6 +138,7 @@ jobs:
       warn_on: ${{ inputs.warn_on }}
       local_debug: ${{ inputs.local_debug }}
       use-flutter: ${{ inputs.use-flutter }}
+      ignore_packages: ${{ inputs.ignore_packages }}
       checkout_submodules: ${{ inputs.checkout_submodules }}
 
   license:
@@ -128,6 +151,8 @@ jobs:
       warn_on: ${{ inputs.warn_on }}
       local_debug: ${{ inputs.local_debug }}
       use-flutter: ${{ inputs.use-flutter }}
+      ignore_license: ${{ inputs.ignore_license }}
+      ignore_packages: ${{ inputs.ignore_packages }}
       checkout_submodules: ${{ inputs.checkout_submodules }}
 
   coverage:
@@ -142,6 +167,8 @@ jobs:
       coverage_web: ${{ inputs.coverage_web }}
       local_debug: ${{ inputs.local_debug }}
       use-flutter: ${{ inputs.use-flutter }}
+      ignore_coverage: ${{ inputs.ignore_coverage }}
+      ignore_packages: ${{ inputs.ignore_packages }}
       checkout_submodules: ${{ inputs.checkout_submodules }}
       experiments: ${{ inputs.experiments }}
 
@@ -155,6 +182,7 @@ jobs:
       warn_on: ${{ inputs.warn_on }}
       local_debug: ${{ inputs.local_debug }}
       use-flutter: ${{ inputs.use-flutter }}
+      ignore_packages: ${{ inputs.ignore_packages }}
       checkout_submodules: ${{ inputs.checkout_submodules }}
 
   do-not-submit:
@@ -167,6 +195,7 @@ jobs:
       warn_on: ${{ inputs.warn_on }}
       local_debug: ${{ inputs.local_debug }}
       use-flutter: ${{ inputs.use-flutter }}
+      ignore_packages: ${{ inputs.ignore_packages }}
       checkout_submodules: ${{ inputs.checkout_submodules }}
 
   comment:

.github/workflows/health_base.yaml

@@ -49,6 +49,21 @@ on:
         default: false
         required: false
         type: boolean
+      ignore_license:
+        description: Which files to ignore for the license check.
+        default: "\"\""
+        required: false
+        type: string
+      ignore_coverage:
+        description: Which files to ignore for the coverage check.
+        default: "\"\""
+        required: false
+        type: string
+      ignore_packages:
+        description: Which packages to ignore.
+        default: "\"\""
+        required: false
+        type: string
       checkout_submodules:
         description: Whether to checkout submodules of git repositories.
         default: false
@@ -124,6 +139,9 @@ jobs:
             ${{ fromJSON('{"true":"--coverage_web","false":""}')[inputs.coverage_web] }} \
             --fail_on ${{ inputs.fail_on }} \
             --warn_on ${{ inputs.warn_on }} \
+            --ignore_license ${{ inputs.ignore_license }} \
+            --ignore_coverage ${{ inputs.ignore_coverage }} \
+            --ignore_packages ${{ inputs.ignore_packages }} \
             --experiments ${{ inputs.experiments }}
 
       - run: test -f current_repo/output/comment.md || echo $'The ${{ inputs.check }} workflow has encountered an exception and did not complete.' >> current_repo/output/comment.md

.github/workflows/health_internal.yaml

@@ -15,3 +15,5 @@ jobs:
       checks: version,changelog,license,coverage,breaking,do-not-submit
       fail_on: version,changelog,do-not-submit
       warn_on: license,coverage,breaking
+      ignore_license: 'pkgs/firehose/test_data'
+      ignore_coverage: 'pkgs/firehose/bin'

pkgs/firehose/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.6.1
+
+- Add `ignore` flags to the health workflow.
+
 ## 0.6.0
 
 - Make the health workflow testable with golden tests.

pkgs/firehose/README.md

@@ -160,10 +160,12 @@ This is a Github workflow to check PR health.
 
 When run from a PR, this tool will check a configurable subset of the following
 
-* If the package versioning is correct and consistent, see `firehose` description above.
-* If a changelog entry has been added.
-* If all `.dart` files have a license header.
+* The package versioning is correct and consistent, see `firehose` description above.
+* A changelog entry has been added.
+* All `.dart` files have a license header.
 * How the test coverage is affected by the PR.
+* The package versioning takes into account any breaking changes in the PR.
+* The PR contains `DO_NOT_SUBMIT` strings in the files or the description.
 
 This tool can work with either single package repos or with mono-repos (repos
 containing several packages).
@@ -186,6 +188,22 @@ jobs:
 #     checks: "version,changelog,license,coverage"
 ```
 
+### Options
+
+| Name | Type | Description | Example |
+| ------------- | ------------- | ------------- | ------------- |
+| checks  | List of strings  | What to check for in the PR health check | `"version,changelog,license,coverage,breaking,do-not-submit"` |
+| fail_on  | List of strings  | Which checks should lead to failure | `"version,changelog,do-not-submit"` |
+| warn_on  | List of strings  | Which checks should not fail, but only warn | `"license,coverage,breaking"` |
+| upload_coverage  | boolean  | Whether to upload the coverage to [coveralls](https://coveralls.io/) | `true` |
+| coverage_web  | boolean  | Whether to run `dart test -p chrome` for coverage | `false` |
+| use-flutter  | boolean  | Whether to setup Flutter in this workflow | `false` |
+| ignore_license  | List of globs  | | `"**.g.dart"` |
+| ignore_coverage  | List of globs  | Which files to ignore for the license check | `"**.mock.dart,**.g.dart"` |
+| ignore_packages  | List of globs  | Which packages to ignore | `"pkgs/helper_package"` |
+| checkout_submodules  | boolean  | Whether to checkout submodules of git repositories | `false` |
+| experiments  | List of strings  | Which experiments should be enabled for Dart | `"native-assets"` |
+
 ### Workflow docs
 
 The description of the common workflow for repos using this tool can be found at

pkgs/firehose/bin/health.dart

@@ -15,6 +15,21 @@ void main(List<String> arguments) async {
       allowed: checkTypes,
       help: 'Check PR health.',
     )
+    ..addMultiOption(
+      'ignore_packages',
+      defaultsTo: [],
+      help: 'Which packages to ignore.',
+    )
+    ..addMultiOption(
+      'ignore_license',
+      defaultsTo: [],
+      help: 'Which files to ignore for the license check.',
+    )
+    ..addMultiOption(
+      'ignore_coverage',
+      defaultsTo: [],
+      help: 'Which files to ignore for the coverage check.',
+    )
     ..addMultiOption(
       'warn_on',
       allowed: checkTypes,
@@ -33,14 +48,15 @@ void main(List<String> arguments) async {
       'coverage_web',
       help: 'Whether to run web tests for coverage',
     );
-  var parsedArgs = argParser.parse(arguments);
-  var check = parsedArgs['check'] as String;
-  var warnOn = parsedArgs['warn_on'] as List<String>;
-  var failOn = parsedArgs['fail_on'] as List<String>;
-  var experiments = (parsedArgs['experiments'] as List<String>)
-      .where((name) => name.isNotEmpty)
-      .toList();
-  var coverageWeb = parsedArgs['coverage_web'] as bool;
+  final parsedArgs = argParser.parse(arguments);
+  final check = parsedArgs['check'] as String;
+  final warnOn = parsedArgs['warn_on'] as List<String>;
+  final failOn = parsedArgs['fail_on'] as List<String>;
+  final ignorePackages = _listNonEmpty(parsedArgs, 'ignore_packages');
+  final ignoreLicense = _listNonEmpty(parsedArgs, 'ignore_license');
+  final ignoreCoverage = _listNonEmpty(parsedArgs, 'ignore_coverage');
+  final experiments = _listNonEmpty(parsedArgs, 'experiments');
+  final coverageWeb = parsedArgs['coverage_web'] as bool;
   if (warnOn.toSet().intersection(failOn.toSet()).isNotEmpty) {
     throw ArgumentError('The checks for which warnings are displayed and the '
         'checks which lead to failure must be disjoint.');
@@ -51,7 +67,13 @@ void main(List<String> arguments) async {
     warnOn,
     failOn,
     coverageWeb,
+    ignorePackages,
+    ignoreLicense,
+    ignoreCoverage,
     experiments,
     GithubApi(),
   ).healthCheck();
 }
+
+List<String> _listNonEmpty(ArgResults parsedArgs, String key) =>
+    (parsedArgs[key] as List<String>).where((e) => e.isNotEmpty).toList();

pkgs/firehose/lib/firehose.dart

@@ -7,6 +7,8 @@
 import 'dart:io';
 import 'dart:math';
 
+import 'package:glob/glob.dart';
+
 import 'src/github.dart';
 import 'src/pub.dart';
 import 'src/repo.dart';
@@ -90,9 +92,12 @@ Saving existing comment id $existingCommentId to file ${idFile.path}''');
     github.close();
   }
 
-  Future<VerificationResults> verify(GithubApi github) async {
+  Future<VerificationResults> verify(
+    GithubApi github, [
+    List<Glob> ignoredPackages = const [],
+  ]) async {
     var repo = Repository(directory);
-    var packages = repo.locatePackages();
+    var packages = repo.locatePackages(ignoredPackages);
 
     var pub = Pub();
 

pkgs/firehose/lib/src/github.dart

@@ -5,7 +5,9 @@
 
 import 'dart:io';
 
+import 'package:collection/collection.dart';
 import 'package:github/github.dart';
+import 'package:glob/glob.dart';
 import 'package:http/http.dart' as http;
 import 'package:path/path.dart' as path;
 
@@ -107,14 +109,19 @@ class GithubApi {
     return matchingComment?.id;
   }
 
-  Future<List<GitFile>> listFilesForPR(Directory directory) async =>
+  Future<List<GitFile>> listFilesForPR(
+    Directory directory, [
+    List<Glob> ignoredFiles = const [],
+  ]) async =>
       await _github.pullRequests
           .listFiles(repoSlug!, issueNumber!)
           .map((prFile) => GitFile(
                 prFile.filename!,
                 FileStatus.fromString(prFile.status!),
                 directory,
               ))
+          .where((file) =>
+              ignoredFiles.none((glob) => glob.matches(file.filename)))
           .toList();
 
   /// Write a notice message to the github log.
@@ -135,9 +142,8 @@ class GitFile {
   final FileStatus status;
   final Directory directory;
 
-  bool isInPackage(Package package) {
-    return path.isWithin(package.directory.path, pathInRepository);
-  }
+  bool isInPackage(Package package) =>
+      path.isWithin(package.directory.path, pathInRepository);
 
   String get pathInRepository => path.join(directory.path, filename);
 

pkgs/firehose/lib/src/health/changelog.dart

@@ -4,19 +4,21 @@
 
 import 'dart:io';
 
+import 'package:glob/glob.dart';
 import 'package:path/path.dart' as path;
 
 import '../github.dart';
 import '../repo.dart';
 
 Future<Map<Package, List<GitFile>>> packagesWithoutChangelog(
   GithubApi github,
+  List<Glob> ignoredPackages,
   Directory directory,
 ) async {
   final repo = Repository(directory);
-  final packages = repo.locatePackages();
+  final packages = repo.locatePackages(ignoredPackages);
 
-  final files = await github.listFilesForPR(directory);
+  final files = await github.listFilesForPR(directory, ignoredPackages);
 
   var packagesWithoutChangedChangelog = collectPackagesWithoutChangelogChanges(
     packages,