Upgrades to health.yaml (#201)

* Continue `health.yaml` on error * Switch to package github * Fix tests * Rev version * Add test * Remove git dep on apitool * Fix DNS result * rename * Adapt markdown * Add failure step * Add more descriptive name * Update pkgs/firehose/CHANGELOG.md Co-authored-by: Devon Carew <[email protected]> * Update pkgs/firehose/test/github_test.dart Co-authored-by: Devon Carew <[email protected]> --------- Co-authored-by: Devon Carew <[email protected]>
dart-lang · Dec 1, 2023 · 32461a7 · 32461a7
1 parent 805ab4f
commit 32461a7
Show file tree

Hide file tree

Showing 13 changed files with 154 additions and 172 deletions.
diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml
@@ -92,15 +92,17 @@ jobs:
         run: dart pub global activate firehose
         if:  ${{ !inputs.local_debug }}
 
-      - name: Install api_tool
-        run: dart pub global activate --source git https://github.com/bmw-tech/dart_apitool
-
       - name: Install local firehose
         run: dart pub global activate --source path current_repo/pkgs/firehose/
         if:  ${{ inputs.local_debug }} 
-
+
+      - name: Install api_tool
+        run: dart pub global activate dart_apitool
+
       - name: Check PR health
+        id: healthstep
         if: ${{ github.event_name == 'pull_request' }}
+        continue-on-error: true # continue, so that the result is written as a comment. Fail in the last step
         env:
           GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
           ISSUE_NUMBER: ${{ github.event.number }}
@@ -125,3 +127,7 @@ jobs:
         with:
           name: output
           path: current_repo/output/
+
+      - name: Fail the workflow if "Check PR health" failed
+        if: steps.healthstep.outcome != 'success'
+        run: exit 1
diff --git a/pkgs/firehose/CHANGELOG.md b/pkgs/firehose/CHANGELOG.md
@@ -1,3 +1,8 @@
+## 0.4.0
+
+- Switch to using `package:github`.
+- Check for `DO_NOT${'_'}SUBMIT` strings in the PR description.
+
 ## 0.3.33
 
 - Retry calls to pub.dev API.

diff --git a/pkgs/firehose/bin/firehose.dart b/pkgs/firehose/bin/firehose.dart
@@ -33,7 +33,7 @@ void main(List<String> arguments) async {
       exit(1);
     }
 
-    final github = Github();
+    final github = GithubApi();
     if (publish && !github.inGithubContext) {
       _usage(argParser,
           error: 'Error: --publish can only be executed from within a GitHub '

diff --git a/pkgs/firehose/lib/firehose.dart b/pkgs/firehose/lib/firehose.dart
@@ -34,12 +34,12 @@ class Firehose {
   /// - provide feedback on the PR (via a PR comment) about packages which are
   ///   ready to publish
   Future<void> validate() async {
-    var github = Github();
+    var github = GithubApi();
 
     // Do basic validation of our expected env var.
     if (!expectEnv(github.githubAuthToken, 'GITHUB_TOKEN')) return;
-    if (!expectEnv(github.repoSlug, 'GITHUB_REPOSITORY')) return;
-    if (!expectEnv(github.issueNumber, 'ISSUE_NUMBER')) return;
+    if (!expectEnv(github.repoSlug?.fullName, 'GITHUB_REPOSITORY')) return;
+    if (!expectEnv(github.issueNumber?.toString(), 'ISSUE_NUMBER')) return;
     if (!expectEnv(github.sha, 'GITHUB_SHA')) return;
 
     if ((github.actor ?? '').endsWith(_botSuffix)) {
@@ -60,8 +60,6 @@ Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automati
 
     var existingCommentId = await allowFailure(
       github.findCommentId(
-        github.repoSlug!,
-        github.issueNumber!,
         user: _githubActionsUser,
         searchTerm: _publishBotTag,
       ),
@@ -92,7 +90,7 @@ Saving existing comment id $existingCommentId to file ${idFile.path}''');
     github.close();
   }
 
-  Future<VerificationResults> verify(Github github) async {
+  Future<VerificationResults> verify(GithubApi github) async {
     var repo = Repository();
     var packages = repo.locatePackages();
 
@@ -184,7 +182,7 @@ Saving existing comment id $existingCommentId to file ${idFile.path}''');
   }
 
   Future<bool> _publish() async {
-    var github = Github();
+    var github = GithubApi();
 
     if (!expectEnv(github.refName, 'GITHUB_REF_NAME')) return false;
 

diff --git a/pkgs/firehose/lib/src/github.dart b/pkgs/firehose/lib/src/github.dart
@@ -3,33 +3,44 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
-import 'dart:convert';
 import 'dart:io';
 
-import 'package:http/http.dart' as http;
+import 'package:github/github.dart';
 import 'package:path/path.dart' as path;
 
 import 'repo.dart';
 
-// TODO:(devoncarew): Consider replacing some of this class with package:github.
+class GithubApi {
+  final RepositorySlug? _repoSlug;
+
+  final int? _issueNumber;
 
-class Github {
   static Map<String, String> get _env => Platform.environment;
 
   /// When true, details of any RPC error are printed to the console.
   final bool verbose;
 
-  http.Client? _httpClient;
+  GithubApi({this.verbose = false, RepositorySlug? repoSlug, int? issueNumber})
+      : _repoSlug = repoSlug,
+        _issueNumber = issueNumber;
 
-  Github({this.verbose = false});
+  late GitHub github = githubAuthToken != null
+      ? GitHub(auth: Authentication.withToken(githubAuthToken))
+      : GitHub();
 
   String? get githubAuthToken => _env['GITHUB_TOKEN'];
 
   /// The owner and repository name. For example, `octocat/Hello-World`.
-  String? get repoSlug => _env['GITHUB_REPOSITORY'];
+  RepositorySlug? get repoSlug {
+    return _repoSlug ??
+        (_env['GITHUB_REPOSITORY'] != null
+            ? RepositorySlug.full(_env['GITHUB_REPOSITORY']!)
+            : null);
+  }
 
   /// The PR (or issue) number.
-  String? get issueNumber => _env['ISSUE_NUMBER'];
+  int? get issueNumber =>
+      _issueNumber ?? int.tryParse(_env['ISSUE_NUMBER'] ?? '');
 
   /// Any labels applied to this PR.
   List<String> get prLabels =>
@@ -52,8 +63,6 @@ class Github {
   /// The ref name of the base where the PR branched off of.
   String? get baseRef => _env['base_ref'];
 
-  http.Client get httpClient => _httpClient ??= http.Client();
-
   /// Write the given [markdownSummary] content to the GitHub
   /// `GITHUB_STEP_SUMMARY` file. This will cause the markdown output to be
   /// appended to the GitHub job summary for the current PR.
@@ -72,139 +81,45 @@ class Github {
         mode: FileMode.append);
   }
 
-  Future<String> callRestApiGet(Uri uri) async {
-    var token = githubAuthToken!;
-
-    return httpClient.get(uri, headers: {
-      'Authorization': 'Bearer $token',
-      'Accept': 'application/vnd.github+json',
-    }).then((response) {
-      return response.statusCode != 200
-          ? throw RpcException(response.reasonPhrase!)
-          : response.body;
-    });
-  }
-
-  Future<String> callRestApiPost(Uri uri, String body) async {
-    var token = githubAuthToken!;
-
-    return httpClient
-        .post(uri,
-            headers: {
-              'Authorization': 'Bearer $token',
-              'Accept': 'application/vnd.github+json',
-            },
-            body: body)
-        .then((response) {
-      if (response.statusCode != 201) {
-        if (verbose) {
-          stderr.writeln('${response.statusCode} ${response.reasonPhrase}');
-          for (var entry in response.headers.entries) {
-            stderr.writeln('${entry.key}: ${entry.value}');
-          }
-          stderr.writeln(response.body);
-        }
-        throw RpcException(response.reasonPhrase!);
-      }
-      return response.body;
-    });
-  }
-
-  Future<String> callRestApiPatch(Uri uri, String body) async {
-    var token = githubAuthToken!;
-
-    return httpClient
-        .patch(uri,
-            headers: {
-              'Authorization': 'Bearer $token',
-              'Accept': 'application/vnd.github+json',
-            },
-            body: body)
-        .then((response) {
-      return response.statusCode != 200
-          ? throw RpcException(response.reasonPhrase!)
-          : response.body;
-    });
-  }
-
-  Future<void> callRestApiDelete(Uri uri) async {
-    var token = githubAuthToken!;
-
-    return httpClient.delete(uri, headers: {
-      'Authorization': 'Bearer $token',
-      'Accept': 'application/vnd.github+json',
-    }).then((response) {
-      if (response.statusCode != 204) {
-        throw RpcException(response.reasonPhrase!);
-      }
-    });
-  }
-
   /// Find a comment on the PR matching the given criteria ([user],
   /// [searchTerm]). Return the issue ID if a matching comment is found or null
   /// if there's no match.
-  Future<int?> findCommentId(
-    String repoSlug,
-    String issueNumber, {
+  Future<int?> findCommentId({
     required String user,
     String? searchTerm,
   }) async {
-    var result = await callRestApiGet(
-      Uri.parse('https://api.github.com/repos/$repoSlug/issues/$issueNumber/'
-          'comments?per_page=100'),
+    final matchingComment = await github.issues
+        .listCommentsByIssue(repoSlug!, issueNumber!)
+        .map<IssueComment?>((comment) => comment)
+        .firstWhere(
+      (comment) {
+        final userMatch = comment?.user?.login == user;
+        final containsSearchTerm = searchTerm == null ||
+            (comment?.body?.contains(searchTerm) ?? false);
+        return userMatch && containsSearchTerm;
+      },
+      orElse: () => null,
     );
-
-    var items = jsonDecode(result) as List;
-    for (var item in items) {
-      item as Map;
-      var id = item['id'] as int;
-      var userLogin = (item['user'] as Map)['login'] as String;
-      var body = item['body'] as String;
-
-      if (userLogin != user) continue;
-      if (searchTerm != null && !body.contains(searchTerm)) continue;
-
-      return id;
-    }
-
-    return null;
+    return matchingComment?.id;
   }
 
-  Future<List<GitFile>> listFilesForPR() async {
-    var result = await callRestApiGet(
-      Uri.parse(
-          'https://api.github.com/repos/$repoSlug/pulls/$issueNumber/files'),
-    );
-    var json = jsonDecode(result) as List;
-    var files = json
-        .map((e) => e as Map<String, dynamic>)
-        .map((e) => GitFile(
-              e['filename'] as String,
-              FileStatus.values.firstWhere(
-                (element) => element.name == e['status'] as String,
-              ),
-            ))
-        .toList();
-    return files;
-  }
-
-  void close() {
-    _httpClient?.close();
-  }
+  Future<List<GitFile>> listFilesForPR() async => await github.pullRequests
+      .listFiles(repoSlug!, issueNumber!)
+      .map((prFile) =>
+          GitFile(prFile.filename!, FileStatus.fromString(prFile.status!)))
+      .toList();
 
   /// Write a notice message to the github log.
   void notice({required String message}) {
     print('::notice ::$message');
   }
-}
-
-class RpcException implements Exception {
-  final String message;
 
-  RpcException(this.message);
+  Future<String> pullrequestBody() async {
+    final pullRequest = await github.pullRequests.get(repoSlug!, issueNumber!);
+    return pullRequest.body ?? '';
+  }
 
-  @override
-  String toString() => 'RpcException: $message';
+  void close() => github.dispose();
 }
 
 class GitFile {
@@ -223,6 +138,18 @@ class GitFile {
 
   @override
   String toString() => '$filename: $status';
+
+  @override
+  bool operator ==(Object other) {
+    if (identical(this, other)) return true;
+
+    return other is GitFile &&
+        other.filename == filename &&
+        other.status == status;
+  }
+
+  @override
+  int get hashCode => filename.hashCode ^ status.hashCode;
 }
 
 enum FileStatus {
@@ -233,4 +160,7 @@ enum FileStatus {
   copied,
   changed,
   unchanged;
+
+  static FileStatus fromString(String s) =>
+      FileStatus.values.firstWhere((element) => element.name == s);
 }
diff --git a/pkgs/firehose/lib/src/health/changelog.dart b/pkgs/firehose/lib/src/health/changelog.dart
@@ -11,7 +11,7 @@ import '../repo.dart';
 import '../utils.dart';
 
 Future<Map<Package, List<GitFile>>> packagesWithoutChangelog(
-    Github github) async {
+    GithubApi github) async {
   final repo = Repository();
   final packages = repo.locatePackages();
 

diff --git a/pkgs/firehose/lib/src/health/coverage.dart b/pkgs/firehose/lib/src/health/coverage.dart
@@ -17,7 +17,7 @@ class Coverage {
 
   Coverage(this.coverageWeb);
 
-  Future<CoverageResult> compareCoverages(Github github) async {
+  Future<CoverageResult> compareCoverages(GithubApi github) async {
     var files = await github.listFilesForPR();
     var basePath = '../base_repo/';