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 ada423495503d0f90d5499af1994eee49f0c87a1.
* Fix call
diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml
index 798d1cd..b964816 100644
--- a/.github/workflows/health.yaml
+++ b/.github/workflows/health.yaml
@@ -29,7 +29,7 @@
# 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 @@
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 @@
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 }}
diff --git a/.github/workflows/health_internal.yaml b/.github/workflows/health_internal.yaml
index 036e67a..826bc48 100644
--- a/.github/workflows/health_internal.yaml
+++ b/.github/workflows/health_internal.yaml
@@ -11,3 +11,5 @@
with:
local_debug: true
coverage_web: false
+ upload_coverage: false
+ checks: version,changelog,license,coverage,breaking
diff --git a/pkgs/firehose/CHANGELOG.md b/pkgs/firehose/CHANGELOG.md
index f3d87b9..2efbc41 100644
--- a/pkgs/firehose/CHANGELOG.md
+++ b/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.
diff --git a/pkgs/firehose/bin/health.dart b/pkgs/firehose/bin/health.dart
index 25b8052..e87270b 100644
--- a/pkgs/firehose/bin/health.dart
+++ b/pkgs/firehose/bin/health.dart
@@ -11,7 +11,7 @@
var argParser = ArgParser()
..addMultiOption(
'checks',
- allowed: ['version', 'license', 'changelog', 'coverage'],
+ allowed: ['version', 'license', 'changelog', 'coverage', 'breaking'],
help: 'Check PR health.',
)
..addFlag(
diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart
index dcd2437..bb6487a 100644
--- a/pkgs/firehose/lib/src/health/health.dart
+++ b/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 _coverageBotTag = '### Coverage';
+const String _breakingBotTag = '### Breaking changes';
+
const String _prHealthTag = '## PR Health';
class Health {
@@ -63,6 +69,9 @@
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 @@
);
}
+ 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 @@
}
}
+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 @@
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('|');
+}
diff --git a/pkgs/firehose/lib/src/repo.dart b/pkgs/firehose/lib/src/repo.dart
index 8336a5a..0ca96ac 100644
--- a/pkgs/firehose/lib/src/repo.dart
+++ b/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 @@
String get name => pubspec.name;
- String? get version => pubspec.version?.toString();
+ Version? get version => pubspec.version;
@override
String toString() {
diff --git a/pkgs/firehose/pubspec.yaml b/pkgs/firehose/pubspec.yaml
index 3f16ad1..a2caf14 100644
--- a/pkgs/firehose/pubspec.yaml
+++ b/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:
diff --git a/pkgs/firehose/test/repo_test.dart b/pkgs/firehose/test/repo_test.dart
index 283a4e3..34e93ae 100644
--- a/pkgs/firehose/test/repo_test.dart
+++ b/pkgs/firehose/test/repo_test.dart
@@ -34,7 +34,7 @@
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);
});
});