Add api tool call for testing (#153)

* Add api tool call for testing

* Change tag

* Add debugging log

* look in each package

* debug

* take subpackage

* Run in current

* add debug

* debug

* Add argument for testing

* Switch to local copy

* Add pub get to apitool clone

* move dart install

* Revert changes

* Fix ref

* dart fix

* Switch to main branch

* use new

* use report file

* Add test argument

* Organize imports

* Nicer output

* Fix version

* Really fix

* fix path

* Make nicer

* Add severity

* Even nicer

* Fix layout

* Make bold

* Capitalize

* Really bolden

* Enable all checks

* Rev version

* Make nullsafer

* add DO_NOT_SUBMIT

* Remove empty file

* Changes as per review

* Revert "Add test argument"

This reverts commit ada4234.

* Fix call

.github/workflows/health.yaml

@@ -29,7 +29,7 @@ name: Health
 #     uses: dart-lang/ecosystem/.github/workflows/health.yaml@main
 #     with:
 #       sdk: beta
-#       checks: "version,changelog,license,coverage"
+#       checks: "version,changelog,license,coverage,breaking"
 
 on:
   workflow_call:
@@ -43,8 +43,8 @@ on:
         required: false
         type: string
       checks:
-        description: What to check for in the PR health check - any subset of "version,changelog,license,coverage"
-        default: "version,changelog,license,coverage"
+        description: What to check for in the PR health check - any subset of "version,changelog,license,coverage,breaking"
+        default: "version,changelog,license,coverage,breaking"
         type: string
         required: false
       local_debug:
@@ -92,6 +92,9 @@ 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 }} 

.github/workflows/health_internal.yaml

@@ -11,3 +11,5 @@ jobs:
     with:
       local_debug: true
       coverage_web: false
+      upload_coverage: false
+      checks: version,changelog,license,coverage,breaking

pkgs/firehose/CHANGELOG.md

@@ -1,3 +1,7 @@
+## 0.3.31
+
+- Add PR Health checks for breaking changes.
+
 ## 0.3.30
 
 - Improve support for `-dev` and `-wip` package versions.

pkgs/firehose/bin/health.dart

@@ -11,7 +11,7 @@ void main(List<String> arguments) async {
   var argParser = ArgParser()
     ..addMultiOption(
       'checks',
-      allowed: ['version', 'license', 'changelog', 'coverage'],
+      allowed: ['version', 'license', 'changelog', 'coverage', 'breaking'],
       help: 'Check PR health.',
     )
     ..addFlag(

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

@@ -4,13 +4,17 @@
 
 // ignore_for_file: always_declare_return_types
 
+import 'dart:convert';
 import 'dart:io';
 import 'dart:math';
 
 import 'package:collection/collection.dart';
+import 'package:path/path.dart' as path;
+import 'package:pub_semver/pub_semver.dart';
 
 import '../../firehose.dart';
 import '../github.dart';
+import '../repo.dart';
 import '../utils.dart';
 import 'changelog.dart';
 import 'coverage.dart';
@@ -28,6 +32,8 @@ const String _changelogBotTag = '### Changelog Entry';
 
 const String _coverageBotTag = '### Coverage';
 
+const String _breakingBotTag = '### Breaking changes';
+
 const String _prHealthTag = '## PR Health';
 
 class Health {
@@ -63,6 +69,9 @@ class Health {
       if (args.contains('coverage') &&
           !github.prLabels.contains('skip-coverage-check'))
         (Github github) => coverageCheck(github, coverageweb),
+      if (args.contains('breaking') &&
+          !github.prLabels.contains('skip-breaking-check'))
+        breakingCheck,
     ];
 
     var checked =
@@ -92,6 +101,80 @@ Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automati
     );
   }
 
+  Future<HealthCheckResult> breakingCheck(Github github) async {
+    final repo = Repository();
+    final packages = repo.locatePackages();
+    var changeForPackage = <Package, BreakingChange>{};
+    var baseDirectory = Directory('../base_repo');
+    for (var package in packages) {
+      var currentPath =
+          path.relative(package.directory.path, from: Directory.current.path);
+      var basePackage = path.relative(
+        path.join(baseDirectory.absolute.path, currentPath),
+        from: currentPath,
+      );
+      print('Look for changes in $currentPath with base $basePackage');
+      var runApiTool = Process.runSync(
+        'dart',
+        [
+          ...['pub', 'global', 'run'],
+          'dart_apitool:main',
+          'diff',
+          ...['--old', basePackage],
+          ...['--new', '.'],
+          ...['--report-format', 'json'],
+          ...['--report-file-path', 'report.json'],
+        ],
+        workingDirectory: currentPath,
+      );
+      print(runApiTool.stderr);
+      print(runApiTool.stdout);
+
+      final reportFile = File(path.join(currentPath, 'report.json'));
+      var fullReportString = reportFile.readAsStringSync();
+      var decoded = jsonDecode(fullReportString) as Map<String, dynamic>;
+      var report = decoded['report'] as Map<String, dynamic>;
+
+      var formattedChanges = const JsonEncoder.withIndent('  ').convert(report);
+      print('Breaking change report:\n$formattedChanges');
+
+      BreakingLevel breakingLevel;
+      if ((report['noChangesDetected'] as bool?) ?? false) {
+        breakingLevel = BreakingLevel.none;
+      } else {
+        if ((report['breakingChanges'] as Map? ?? {}).isNotEmpty) {
+          breakingLevel = BreakingLevel.breaking;
+        } else if ((report['nonBreakingChanges'] as Map? ?? {}).isNotEmpty) {
+          breakingLevel = BreakingLevel.nonBreaking;
+        } else {
+          breakingLevel = BreakingLevel.none;
+        }
+      }
+
+      var oldPackage = Package(
+        Directory(path.join(baseDirectory.path, currentPath)),
+        package.repository,
+      );
+      changeForPackage[package] = BreakingChange(
+        level: breakingLevel,
+        oldVersion: oldPackage.version!,
+        newVersion: package.version!,
+      );
+    }
+    return HealthCheckResult(
+      'breaking',
+      _breakingBotTag,
+      changeForPackage.values.any((element) => !element.versionIsFine)
+          ? Severity.warning
+          : Severity.info,
+      '''
+| Package | Change | Current Version | New Version | Needed Version | Looking good? |
+| :--- | :--- | ---: | ---: | ---: | ---: |
+${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()}|').join('\n')}
+''',
+    );
+  }
+
   Future<HealthCheckResult> licenseCheck(Github github) async {
     var files = await github.listFilesForPR();
     var allFilePaths = await getFilesWithoutLicenses(Directory.current);
@@ -237,6 +320,24 @@ Saving existing comment id $existingCommentId to file ${idFile.path}''');
   }
 }
 
+Version getNewVersion(BreakingLevel level, Version oldVersion) {
+  return switch (level) {
+    BreakingLevel.none => oldVersion,
+    BreakingLevel.nonBreaking => oldVersion.nextMinor,
+    BreakingLevel.breaking => oldVersion.nextBreaking,
+  };
+}
+
+enum BreakingLevel {
+  none('None'),
+  nonBreaking('Non-Breaking'),
+  breaking('Breaking');
+
+  final String name;
+
+  const BreakingLevel(this.name);
+}
+
 class HealthCheckResult {
   final String name;
   final String tag;
@@ -245,3 +346,27 @@ class HealthCheckResult {
 
   HealthCheckResult(this.name, this.tag, this.severity, this.markdown);
 }
+
+class BreakingChange {
+  final BreakingLevel level;
+  final Version oldVersion;
+  final Version newVersion;
+
+  BreakingChange({
+    required this.level,
+    required this.oldVersion,
+    required this.newVersion,
+  });
+
+  Version get suggestedNewVersion => getNewVersion(level, oldVersion);
+
+  bool get versionIsFine => newVersion == suggestedNewVersion;
+
+  String toMarkdownRow() => [
+        level.name,
+        oldVersion,
+        newVersion,
+        versionIsFine ? suggestedNewVersion : '**$suggestedNewVersion**',
+        versionIsFine ? ':heavy_check_mark:' : ':warning:'
+      ].map((e) => e.toString()).join('|');
+}

pkgs/firehose/lib/src/repo.dart

@@ -5,6 +5,7 @@
 import 'dart:io';
 
 import 'package:path/path.dart' as path;
+import 'package:pub_semver/pub_semver.dart';
 import 'package:pubspec_parse/pubspec_parse.dart';
 import 'package:yaml/yaml.dart' as yaml;
 
@@ -99,7 +100,7 @@ class Package {
 
   String get name => pubspec.name;
 
-  String? get version => pubspec.version?.toString();
+  Version? get version => pubspec.version;
 
   @override
   String toString() {

pkgs/firehose/pubspec.yaml

@@ -1,6 +1,6 @@
 name: firehose
 description: A tool to automate publishing of Pub packages from GitHub actions.
-version: 0.3.30
+version: 0.3.31
 repository: https://github.com/dart-lang/ecosystem/tree/main/pkgs/firehose
 
 environment:

pkgs/firehose/test/repo_test.dart

@@ -34,7 +34,7 @@ void main() {
       final queryParams = releaseUri.queryParameters;
       expect(queryParams['tag'], packages.calculateRepoTag(package));
       expect(queryParams['title'],
-          allOf(contains(package.name), contains(package.version)));
+          allOf(contains(package.name), contains(package.version.toString())));
       expect(queryParams['body'], package.changelog.describeLatestChanges);
     });
   });