Add warn_on and fail_on

.github/workflows/health.yaml

@@ -47,6 +47,16 @@ on:
         default: "version,changelog,license,coverage,breaking,do-not-submit"
         type: string
         required: false
+      fail_on:
+        description: Which checks should lead to failure - any subset of "version,changelog,license,coverage,breaking,do-not-submit"
+        default: "version,changelog,license,coverage,breaking,do-not-submit"
+        type: string
+        required: false
+      warn_on:
+        description: Which checks should not fail, but only warn - any subset of "version,changelog,license,coverage,breaking,do-not-submit"
+        default: "version,changelog,license,coverage,breaking,do-not-submit"
+        type: string
+        required: false
       local_debug:
         description: Whether to use a local copy of package:firehose - only for debug
         default: false
@@ -119,7 +129,7 @@ jobs:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           ISSUE_NUMBER: ${{ github.event.number }}
           PR_LABELS: "${{ join(github.event.pull_request.labels.*.name) }}"
-        run: cd current_repo/ && dart pub global run firehose:health --checks ${{ inputs.checks }} ${{ fromJSON('{"true":"--coverage_web","false":""}')[inputs.coverage_web] }}
+        run: cd current_repo/ && dart pub global run firehose:health --checks ${{ inputs.checks }} ${{ fromJSON('{"true":"--coverage_web","false":""}')[inputs.coverage_web] }} --fail_on ${{ inputs.fail_on }} --warn_on ${{ inputs.warn_on }}
 
       - name: Upload coverage to Coveralls
         if: ${{ inputs.upload_coverage }}

pkgs/firehose/bin/health.dart

@@ -11,23 +11,32 @@ void main(List<String> arguments) async {
   var argParser = ArgParser()
     ..addMultiOption(
       'checks',
-      allowed: [
-        'version',
-        'license',
-        'changelog',
-        'coverage',
-        'breaking',
-        'do-not-submit',
-      ],
+      allowed: checkTypes,
       help: 'Check PR health.',
     )
+    ..addMultiOption(
+      'warn_on',
+      allowed: checkTypes,
+      help: 'Which checks to display warnings on',
+    )
+    ..addMultiOption(
+      'fail_on',
+      allowed: checkTypes,
+      help: 'Which checks should lead to workflow failure',
+    )
     ..addFlag(
       'coverage_web',
       help: 'Whether to run web tests for coverage',
     );
   var parsedArgs = argParser.parse(arguments);
   var checks = parsedArgs['checks'] as List<String>;
+  var warnOn = parsedArgs['warn_on'] as List<String>;
+  var failOn = parsedArgs['fail_on'] as List<String>;
   var coverageWeb = parsedArgs['coverage_web'] as bool;
-
-  await Health(Directory.current).healthCheck(checks, coverageWeb);
+  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.');
+  }
+  await Health(Directory.current, checks, warnOn, failOn, coverageWeb)
+      .healthCheck();
 }

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

@@ -38,14 +38,33 @@ const String _breakingBotTag = '### Breaking changes';
 
 const String _prHealthTag = '## PR Health';
 
+const checkTypes = <String>[
+  'version',
+  'license',
+  'changelog',
+  'coverage',
+  'breaking',
+  'do-not-submit',
+];
+
 class Health {
   final Directory directory;
 
-  Health(this.directory);
-
-  Future<void> healthCheck(List args, bool coverageweb) async {
-    var github = GithubApi();
-
+  Health(
+    this.directory,
+    this.checks,
+    this.warnOn,
+    this.failOn,
+    this.coverageweb,
+  );
+  final github = GithubApi();
+
+  final List<String> checks;
+  final List<String> warnOn;
+  final List<String> failOn;
+  final bool coverageweb;
+
+  Future<void> healthCheck() async {
     // Do basic validation of our expected env var.
     if (!expectEnv(github.repoSlug?.fullName, 'GITHUB_REPOSITORY')) return;
     if (!expectEnv(github.issueNumber?.toString(), 'ISSUE_NUMBER')) return;
@@ -56,35 +75,39 @@ class Health {
       return;
     }
 
-    print('Start health check for the checks $args');
-    var checks = [
-      if (args.contains('version') &&
-          !github.prLabels.contains('skip-validate-check'))
-        validateCheck,
-      if (args.contains('license') &&
-          !github.prLabels.contains('skip-license-check'))
-        licenseCheck,
-      if (args.contains('changelog') &&
-          !github.prLabels.contains('skip-changelog-check'))
-        changelogCheck,
-      if (args.contains('coverage') &&
-          !github.prLabels.contains('skip-coverage-check'))
-        (GithubApi github) => coverageCheck(github, coverageweb),
-      if (args.contains('breaking') &&
-          !github.prLabels.contains('skip-breaking-check'))
-        breakingCheck,
-      if (args.contains('do-not-submit') &&
-          !github.prLabels.contains('skip-do-not-submit-check'))
-        doNotSubmitCheck,
-    ];
+    print('Start health check for the checks $checks');
     final results = <HealthCheckResult>[];
-    for (var check in checks) {
-      results.add(await check(github));
+    for (final name in checkTypes) {
+      if (checks.contains(name) &&
+          !github.prLabels.contains('skip-$name-check')) {
+        final firstResult = await checkFor(name)();
+        final HealthCheckResult finalResult;
+        if (warnOn.contains(name) && firstResult.severity == Severity.error) {
+          finalResult = firstResult.withSeverity(Severity.warning);
+        } else if (failOn.contains(name) &&
+            firstResult.severity == Severity.warning) {
+          finalResult = firstResult.withSeverity(Severity.error);
+        } else {
+          finalResult = firstResult;
+        }
+        results.add(finalResult);
+      }
     }
     await writeInComment(github, results);
   }
 
-  Future<HealthCheckResult> validateCheck(GithubApi github) async {
+  Future<HealthCheckResult> Function() checkFor(String checkType) =>
+      switch (checkType) {
+        'version' => validateCheck,
+        'license' => licenseCheck,
+        'changelog' => changelogCheck,
+        'coverage' => coverageCheck,
+        'breaking' => breakingCheck,
+        'do-not-submit' => doNotSubmitCheck,
+        String() => throw ArgumentError(),
+      };
+
+  Future<HealthCheckResult> validateCheck() async {
     //TODO: Add Flutter support for PR health checks
     var results = await Firehose(directory, false).verify(github);
 
@@ -104,7 +127,7 @@ Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automati
     );
   }
 
-  Future<HealthCheckResult> breakingCheck(GithubApi github) async {
+  Future<HealthCheckResult> breakingCheck() async {
     final filesInPR = await github.listFilesForPR();
     final changeForPackage = <Package, BreakingChange>{};
     final baseDirectory = Directory('../base_repo');
@@ -178,7 +201,7 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()}
     return breakingLevel;
   }
 
-  Future<HealthCheckResult> licenseCheck(GithubApi github) async {
+  Future<HealthCheckResult> licenseCheck() async {
     var files = await github.listFilesForPR();
     var allFilePaths = await getFilesWithoutLicenses(Directory.current);
 
@@ -222,7 +245,7 @@ ${unchangedFilesPaths.isNotEmpty ? unchangedMarkdown : ''}
     );
   }
 
-  Future<HealthCheckResult> changelogCheck(GithubApi github) async {
+  Future<HealthCheckResult> changelogCheck() async {
     var filePaths = await packagesWithoutChangelog(github);
 
     final markdownResult = '''
@@ -241,7 +264,7 @@ Changes to files need to be [accounted for](https://github.com/dart-lang/ecosyst
     );
   }
 
-  Future<HealthCheckResult> doNotSubmitCheck(GithubApi github) async {
+  Future<HealthCheckResult> doNotSubmitCheck() async {
     final body = await github.pullrequestBody();
     final files = await github.listFilesForPR();
     print('Checking for DO_NOT${'_'}SUBMIT strings: $files');
@@ -273,11 +296,8 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')}
     );
   }
 
-  Future<HealthCheckResult> coverageCheck(
-    GithubApi github,
-    bool coverageWeb,
-  ) async {
-    var coverage = await Coverage(coverageWeb).compareCoverages(github);
+  Future<HealthCheckResult> coverageCheck() async {
+    var coverage = await Coverage(coverageweb).compareCoverages(github);
 
     var markdownResult = '''
 | File | Coverage |
@@ -384,6 +404,13 @@ class HealthCheckResult {
   final String? markdown;
 
   HealthCheckResult(this.name, this.title, this.severity, this.markdown);
+
+  HealthCheckResult withSeverity(Severity severity) => HealthCheckResult(
+        name,
+        title,
+        severity,
+        markdown,
+      );
 }
 
 class BreakingChange {