Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add api tool call for testing #153

Merged
merged 42 commits into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
b2d4ff7
Add api tool call for testing
mosuem Aug 29, 2023
29cc8b6
Change tag
mosuem Aug 29, 2023
22fbb6c
Add debugging log
mosuem Aug 29, 2023
d14f30e
look in each package
mosuem Aug 29, 2023
c893a6f
debug
mosuem Aug 29, 2023
54b00b7
take subpackage
mosuem Aug 29, 2023
ed7c17c
Run in current
mosuem Aug 29, 2023
597f95d
add debug
mosuem Aug 29, 2023
1602049
debug
mosuem Aug 29, 2023
d04f966
Add argument for testing
mosuem Aug 29, 2023
e6d2c5d
Switch to local copy
mosuem Aug 30, 2023
11c1093
Add pub get to apitool clone
mosuem Aug 30, 2023
e694a6f
move dart install
mosuem Aug 30, 2023
42f750d
Revert changes
mosuem Aug 30, 2023
5a30607
Fix ref
mosuem Sep 11, 2023
ea62622
Merge branch 'main' into apitToolHealth
mosuem Sep 11, 2023
ab4cdb4
dart fix
mosuem Sep 11, 2023
36aac52
Switch to main branch
mosuem Sep 12, 2023
7c6d881
Merge branch 'main' into apitToolHealth
mosuem Sep 27, 2023
2cbfdc8
use new
mosuem Sep 27, 2023
459b25c
use report file
mosuem Sep 27, 2023
ada4234
Add test argument
mosuem Sep 27, 2023
5542840
Organize imports
mosuem Sep 27, 2023
0ad660a
Nicer output
mosuem Sep 27, 2023
510abf9
Fix version
mosuem Sep 27, 2023
dab1b08
Really fix
mosuem Sep 27, 2023
6f8bdc4
fix path
mosuem Sep 27, 2023
078566c
Make nicer
mosuem Sep 27, 2023
7fd83a7
Add severity
mosuem Sep 27, 2023
bdf61fa
Even nicer
mosuem Sep 27, 2023
9ebdbab
Fix layout
mosuem Sep 27, 2023
bf6746a
Make bold
mosuem Sep 27, 2023
b88a2d1
Capitalize
mosuem Sep 27, 2023
f4e9682
Really bolden
mosuem Sep 27, 2023
f0e944c
Enable all checks
mosuem Sep 27, 2023
26c1ba9
Rev version
mosuem Sep 27, 2023
ca1d300
Make nullsafer
mosuem Sep 27, 2023
2538134
add DO_NOT_SUBMIT
mosuem Sep 27, 2023
14c9a94
Remove empty file
mosuem Sep 27, 2023
82c2a4f
Changes as per review
mosuem Sep 28, 2023
a2c044c
Revert "Add test argument"
mosuem Sep 28, 2023
8702ab8
Fix call
mosuem Sep 28, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions .github/workflows/health.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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:
Expand Down Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take it that this tool isn't published to pub?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is - the version I want is just not published yet.


- name: Install local firehose
run: dart pub global activate --source path current_repo/pkgs/firehose/
if: ${{ inputs.local_debug }}
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/health_internal.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -11,3 +11,5 @@ jobs:
with:
local_debug: true
coverage_web: false
upload_coverage: false
checks: version,changelog,license,coverage,breaking
4 changes: 4 additions & 0 deletions pkgs/firehose/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
## 0.3.31
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just fyi there are a few PRs underway rev'ing firehose -


- Add PR Health checks for breaking changes.

## 0.3.30

- Improve support for `-dev` and `-wip` package versions.
Expand Down
2 changes: 1 addition & 1 deletion pkgs/firehose/bin/health.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
125 changes: 125 additions & 0 deletions pkgs/firehose/lib/src/health/health.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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 {
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this run finds breaking changes, should we re-run this report in another format and dump the results to stdout?

...['--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);
Expand Down Expand Up @@ -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;
Expand All @@ -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('|');
}
3 changes: 2 additions & 1 deletion pkgs/firehose/lib/src/repo.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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() {
Expand Down
2 changes: 1 addition & 1 deletion pkgs/firehose/pubspec.yaml
Original file line number Diff line number Diff line change
@@ -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:
Expand Down
2 changes: 1 addition & 1 deletion pkgs/firehose/test/repo_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
});
Expand Down