Add `ignore` flag to health workflows (#218)
* Add `ignore` flag to health workflows
* Switch to glob
* Fix multiline
* Add default glob
* Delete multiline
* Fix errors
* Fix health commenting
* Propagate ignore
* Switch to specific ignores
* Add defaults
* Add test repos
* Start adding health tests
* Add golden files
* Fix changelog
* Fix coverage
* Fix breaking
* More fixes
* Merge
* Fixes
* Move
* Fix tests
* Ignore test data licenses
* Add golden without license
* Fix license
* Rename
* Fix
* Fix analyze issue
* Add args printing
* Add defaults
* Add more defaults
* Remove empty
* Match recursive
* Add docs
* Add link to docs
diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml
index 87ce1e6..3e848b0 100644
--- a/.github/workflows/health.yaml
+++ b/.github/workflows/health.yaml
@@ -1,4 +1,4 @@
-# A CI configuration to check PR health.
+# A CI configuration to check PR health. Check the docs at https://github.com/dart-lang/ecosystem/tree/main/pkgs/firehose#health.
name: Health
@@ -27,7 +27,13 @@
# coverage_web: false
# upload_coverage: false
# use-flutter: true
-
+# use-flutter: true
+# use-flutter: true
+# ignore_license: "**.g.dart"
+# ignore_coverage: "**.mock.dart,**.g.dart"
+# ignore_packages: "pkgs/helper_package"
+# checkout_submodules: false
+# experiments: "native-assets"
on:
workflow_call:
@@ -82,6 +88,21 @@
default: false
required: false
type: boolean
+ ignore_license:
+ description: Which files to ignore for the license check.
+ default: "\"\""
+ required: false
+ type: string
+ ignore_coverage:
+ description: Which files to ignore for the coverage check.
+ default: "\"\""
+ required: false
+ type: string
+ ignore_packages:
+ description: Which packages to ignore.
+ default: "\"\""
+ required: false
+ type: string
checkout_submodules:
description: Whether to checkout submodules of git repositories.
default: false
@@ -104,6 +125,7 @@
warn_on: ${{ inputs.warn_on }}
local_debug: ${{ inputs.local_debug }}
use-flutter: ${{ inputs.use-flutter }}
+ ignore_packages: ${{ inputs.ignore_packages }}
checkout_submodules: ${{ inputs.checkout_submodules }}
changelog:
@@ -116,6 +138,7 @@
warn_on: ${{ inputs.warn_on }}
local_debug: ${{ inputs.local_debug }}
use-flutter: ${{ inputs.use-flutter }}
+ ignore_packages: ${{ inputs.ignore_packages }}
checkout_submodules: ${{ inputs.checkout_submodules }}
license:
@@ -128,6 +151,8 @@
warn_on: ${{ inputs.warn_on }}
local_debug: ${{ inputs.local_debug }}
use-flutter: ${{ inputs.use-flutter }}
+ ignore_license: ${{ inputs.ignore_license }}
+ ignore_packages: ${{ inputs.ignore_packages }}
checkout_submodules: ${{ inputs.checkout_submodules }}
coverage:
@@ -142,6 +167,8 @@
coverage_web: ${{ inputs.coverage_web }}
local_debug: ${{ inputs.local_debug }}
use-flutter: ${{ inputs.use-flutter }}
+ ignore_coverage: ${{ inputs.ignore_coverage }}
+ ignore_packages: ${{ inputs.ignore_packages }}
checkout_submodules: ${{ inputs.checkout_submodules }}
experiments: ${{ inputs.experiments }}
@@ -155,6 +182,7 @@
warn_on: ${{ inputs.warn_on }}
local_debug: ${{ inputs.local_debug }}
use-flutter: ${{ inputs.use-flutter }}
+ ignore_packages: ${{ inputs.ignore_packages }}
checkout_submodules: ${{ inputs.checkout_submodules }}
do-not-submit:
@@ -167,6 +195,7 @@
warn_on: ${{ inputs.warn_on }}
local_debug: ${{ inputs.local_debug }}
use-flutter: ${{ inputs.use-flutter }}
+ ignore_packages: ${{ inputs.ignore_packages }}
checkout_submodules: ${{ inputs.checkout_submodules }}
comment:
diff --git a/.github/workflows/health_base.yaml b/.github/workflows/health_base.yaml
index b8faf86..589eabd 100644
--- a/.github/workflows/health_base.yaml
+++ b/.github/workflows/health_base.yaml
@@ -49,6 +49,21 @@
default: false
required: false
type: boolean
+ ignore_license:
+ description: Which files to ignore for the license check.
+ default: "\"\""
+ required: false
+ type: string
+ ignore_coverage:
+ description: Which files to ignore for the coverage check.
+ default: "\"\""
+ required: false
+ type: string
+ ignore_packages:
+ description: Which packages to ignore.
+ default: "\"\""
+ required: false
+ type: string
checkout_submodules:
description: Whether to checkout submodules of git repositories.
default: false
@@ -124,6 +139,9 @@
${{ fromJSON('{"true":"--coverage_web","false":""}')[inputs.coverage_web] }} \
--fail_on ${{ inputs.fail_on }} \
--warn_on ${{ inputs.warn_on }} \
+ --ignore_license ${{ inputs.ignore_license }} \
+ --ignore_coverage ${{ inputs.ignore_coverage }} \
+ --ignore_packages ${{ inputs.ignore_packages }} \
--experiments ${{ inputs.experiments }}
- run: test -f current_repo/output/comment.md || echo $'The ${{ inputs.check }} workflow has encountered an exception and did not complete.' >> current_repo/output/comment.md
diff --git a/.github/workflows/health_internal.yaml b/.github/workflows/health_internal.yaml
index a633933..270a0b8 100644
--- a/.github/workflows/health_internal.yaml
+++ b/.github/workflows/health_internal.yaml
@@ -15,3 +15,5 @@
checks: version,changelog,license,coverage,breaking,do-not-submit
fail_on: version,changelog,do-not-submit
warn_on: license,coverage,breaking
+ ignore_license: 'pkgs/firehose/test_data'
+ ignore_coverage: 'pkgs/firehose/bin'
diff --git a/pkgs/firehose/CHANGELOG.md b/pkgs/firehose/CHANGELOG.md
index 2d232c9..ca3a401 100644
--- a/pkgs/firehose/CHANGELOG.md
+++ b/pkgs/firehose/CHANGELOG.md
@@ -1,3 +1,7 @@
+## 0.6.1
+
+- Add `ignore` flags to the health workflow.
+
## 0.6.0
- Make the health workflow testable with golden tests.
diff --git a/pkgs/firehose/README.md b/pkgs/firehose/README.md
index 51e5304..30d7abd 100644
--- a/pkgs/firehose/README.md
+++ b/pkgs/firehose/README.md
@@ -160,10 +160,12 @@
When run from a PR, this tool will check a configurable subset of the following
-* If the package versioning is correct and consistent, see `firehose` description above.
-* If a changelog entry has been added.
-* If all `.dart` files have a license header.
+* The package versioning is correct and consistent, see `firehose` description above.
+* A changelog entry has been added.
+* All `.dart` files have a license header.
* How the test coverage is affected by the PR.
+* The package versioning takes into account any breaking changes in the PR.
+* The PR contains `DO_NOT_SUBMIT` strings in the files or the description.
This tool can work with either single package repos or with mono-repos (repos
containing several packages).
@@ -186,6 +188,22 @@
# checks: "version,changelog,license,coverage"
```
+### Options
+
+| Name | Type | Description | Example |
+| ------------- | ------------- | ------------- | ------------- |
+| checks | List of strings | What to check for in the PR health check | `"version,changelog,license,coverage,breaking,do-not-submit"` |
+| fail_on | List of strings | Which checks should lead to failure | `"version,changelog,do-not-submit"` |
+| warn_on | List of strings | Which checks should not fail, but only warn | `"license,coverage,breaking"` |
+| upload_coverage | boolean | Whether to upload the coverage to [coveralls](https://coveralls.io/) | `true` |
+| coverage_web | boolean | Whether to run `dart test -p chrome` for coverage | `false` |
+| use-flutter | boolean | Whether to setup Flutter in this workflow | `false` |
+| ignore_license | List of globs | | `"**.g.dart"` |
+| ignore_coverage | List of globs | Which files to ignore for the license check | `"**.mock.dart,**.g.dart"` |
+| ignore_packages | List of globs | Which packages to ignore | `"pkgs/helper_package"` |
+| checkout_submodules | boolean | Whether to checkout submodules of git repositories | `false` |
+| experiments | List of strings | Which experiments should be enabled for Dart | `"native-assets"` |
+
### Workflow docs
The description of the common workflow for repos using this tool can be found at
diff --git a/pkgs/firehose/bin/health.dart b/pkgs/firehose/bin/health.dart
index e1260dc..5a36b21 100644
--- a/pkgs/firehose/bin/health.dart
+++ b/pkgs/firehose/bin/health.dart
@@ -16,6 +16,21 @@
help: 'Check PR health.',
)
..addMultiOption(
+ 'ignore_packages',
+ defaultsTo: [],
+ help: 'Which packages to ignore.',
+ )
+ ..addMultiOption(
+ 'ignore_license',
+ defaultsTo: [],
+ help: 'Which files to ignore for the license check.',
+ )
+ ..addMultiOption(
+ 'ignore_coverage',
+ defaultsTo: [],
+ help: 'Which files to ignore for the coverage check.',
+ )
+ ..addMultiOption(
'warn_on',
allowed: checkTypes,
help: 'Which checks to display warnings on',
@@ -33,14 +48,15 @@
'coverage_web',
help: 'Whether to run web tests for coverage',
);
- var parsedArgs = argParser.parse(arguments);
- var check = parsedArgs['check'] as String;
- var warnOn = parsedArgs['warn_on'] as List<String>;
- var failOn = parsedArgs['fail_on'] as List<String>;
- var experiments = (parsedArgs['experiments'] as List<String>)
- .where((name) => name.isNotEmpty)
- .toList();
- var coverageWeb = parsedArgs['coverage_web'] as bool;
+ final parsedArgs = argParser.parse(arguments);
+ final check = parsedArgs['check'] as String;
+ final warnOn = parsedArgs['warn_on'] as List<String>;
+ final failOn = parsedArgs['fail_on'] as List<String>;
+ final ignorePackages = _listNonEmpty(parsedArgs, 'ignore_packages');
+ final ignoreLicense = _listNonEmpty(parsedArgs, 'ignore_license');
+ final ignoreCoverage = _listNonEmpty(parsedArgs, 'ignore_coverage');
+ final experiments = _listNonEmpty(parsedArgs, 'experiments');
+ final coverageWeb = parsedArgs['coverage_web'] as bool;
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.');
@@ -51,7 +67,13 @@
warnOn,
failOn,
coverageWeb,
+ ignorePackages,
+ ignoreLicense,
+ ignoreCoverage,
experiments,
GithubApi(),
).healthCheck();
}
+
+List<String> _listNonEmpty(ArgResults parsedArgs, String key) =>
+ (parsedArgs[key] as List<String>).where((e) => e.isNotEmpty).toList();
diff --git a/pkgs/firehose/lib/firehose.dart b/pkgs/firehose/lib/firehose.dart
index f5b5c0d..3f4ce73 100644
--- a/pkgs/firehose/lib/firehose.dart
+++ b/pkgs/firehose/lib/firehose.dart
@@ -7,6 +7,8 @@
import 'dart:io';
import 'dart:math';
+import 'package:glob/glob.dart';
+
import 'src/github.dart';
import 'src/pub.dart';
import 'src/repo.dart';
@@ -90,9 +92,12 @@
github.close();
}
- Future<VerificationResults> verify(GithubApi github) async {
+ Future<VerificationResults> verify(
+ GithubApi github, [
+ List<Glob> ignoredPackages = const [],
+ ]) async {
var repo = Repository(directory);
- var packages = repo.locatePackages();
+ var packages = repo.locatePackages(ignoredPackages);
var pub = Pub();
diff --git a/pkgs/firehose/lib/src/github.dart b/pkgs/firehose/lib/src/github.dart
index e3e07b8..4904c83 100644
--- a/pkgs/firehose/lib/src/github.dart
+++ b/pkgs/firehose/lib/src/github.dart
@@ -5,7 +5,9 @@
import 'dart:io';
+import 'package:collection/collection.dart';
import 'package:github/github.dart';
+import 'package:glob/glob.dart';
import 'package:http/http.dart' as http;
import 'package:path/path.dart' as path;
@@ -107,7 +109,10 @@
return matchingComment?.id;
}
- Future<List<GitFile>> listFilesForPR(Directory directory) async =>
+ Future<List<GitFile>> listFilesForPR(
+ Directory directory, [
+ List<Glob> ignoredFiles = const [],
+ ]) async =>
await _github.pullRequests
.listFiles(repoSlug!, issueNumber!)
.map((prFile) => GitFile(
@@ -115,6 +120,8 @@
FileStatus.fromString(prFile.status!),
directory,
))
+ .where((file) =>
+ ignoredFiles.none((glob) => glob.matches(file.filename)))
.toList();
/// Write a notice message to the github log.
@@ -135,9 +142,8 @@
final FileStatus status;
final Directory directory;
- bool isInPackage(Package package) {
- return path.isWithin(package.directory.path, pathInRepository);
- }
+ bool isInPackage(Package package) =>
+ path.isWithin(package.directory.path, pathInRepository);
String get pathInRepository => path.join(directory.path, filename);
diff --git a/pkgs/firehose/lib/src/health/changelog.dart b/pkgs/firehose/lib/src/health/changelog.dart
index 3474262..95ad14a 100644
--- a/pkgs/firehose/lib/src/health/changelog.dart
+++ b/pkgs/firehose/lib/src/health/changelog.dart
@@ -4,6 +4,7 @@
import 'dart:io';
+import 'package:glob/glob.dart';
import 'package:path/path.dart' as path;
import '../github.dart';
@@ -11,12 +12,13 @@
Future<Map<Package, List<GitFile>>> packagesWithoutChangelog(
GithubApi github,
+ List<Glob> ignoredPackages,
Directory directory,
) async {
final repo = Repository(directory);
- final packages = repo.locatePackages();
+ final packages = repo.locatePackages(ignoredPackages);
- final files = await github.listFilesForPR(directory);
+ final files = await github.listFilesForPR(directory, ignoredPackages);
var packagesWithoutChangedChangelog = collectPackagesWithoutChangelogChanges(
packages,
diff --git a/pkgs/firehose/lib/src/health/coverage.dart b/pkgs/firehose/lib/src/health/coverage.dart
index 18004d2..bb0610d 100644
--- a/pkgs/firehose/lib/src/health/coverage.dart
+++ b/pkgs/firehose/lib/src/health/coverage.dart
@@ -6,6 +6,7 @@
import 'dart:io';
import 'package:collection/collection.dart';
+import 'package:glob/glob.dart';
import 'package:path/path.dart' as path;
import '../github.dart';
@@ -15,21 +16,28 @@
class Coverage {
final bool coverageWeb;
+ final List<Glob> ignoredFiles;
+ final List<Glob> ignoredPackages;
final Directory directory;
final List<String> experiments;
- Coverage(this.coverageWeb, this.directory, this.experiments);
+ Coverage(
+ this.coverageWeb,
+ this.ignoredFiles,
+ this.ignoredPackages,
+ this.directory,
+ this.experiments,
+ );
Future<CoverageResult> compareCoverages(
GithubApi github, Directory base) async {
- var files = await github.listFilesForPR(directory);
-
+ var files = await github.listFilesForPR(directory, ignoredFiles);
return compareCoveragesFor(files, base);
}
CoverageResult compareCoveragesFor(List<GitFile> files, Directory base) {
var repository = Repository(directory);
- var packages = repository.locatePackages();
+ var packages = repository.locatePackages(ignoredPackages);
print('Found packages $packages at $directory');
var filesOfInterest = files
@@ -41,7 +49,7 @@
print('The files of interest are $filesOfInterest');
var baseRepository = Repository(base);
- var basePackages = baseRepository.locatePackages();
+ var basePackages = baseRepository.locatePackages(ignoredFiles);
print('Found packages $basePackages at $base');
var changedPackages = packages
@@ -120,9 +128,12 @@
],
workingDirectory: package.directory.path,
);
- print(resultChrome.stdout);
- print(resultChrome.stderr);
+ if (resultChrome.exitCode != 0) {
+ print(resultChrome.stderr);
+ }
+ print('Dart test browser: ${resultChrome.stdout}');
}
+
print('Run tests with coverage for vm');
var resultVm = Process.runSync(
'dart',
@@ -134,8 +145,11 @@
],
workingDirectory: package.directory.path,
);
- print('dart test stdout: ${resultVm.stdout}');
- print('dart test stderr: ${resultVm.stderr}');
+ if (resultVm.exitCode != 0) {
+ print(resultVm.stderr);
+ }
+ print('Dart test VM: ${resultVm.stdout}');
+
print('Compute coverage from runs');
var resultLcov = Process.runSync(
'dart',
@@ -154,8 +168,10 @@
],
workingDirectory: package.directory.path,
);
- print('dart coverage stdout: ${resultLcov.stdout}');
- print('dart coverage stderr: ${resultLcov.stderr}');
+ if (resultLcov.exitCode != 0) {
+ print(resultLcov.stderr);
+ }
+ print('Dart coverage: ${resultLcov.stdout}');
return parseLCOV(
path.join(package.directory.path, 'coverage/lcov.info'),
relativeTo: package.repository.baseDirectory.path,
diff --git a/pkgs/firehose/lib/src/health/health.dart b/pkgs/firehose/lib/src/health/health.dart
index 94ab8d0..47baf97 100644
--- a/pkgs/firehose/lib/src/health/health.dart
+++ b/pkgs/firehose/lib/src/health/health.dart
@@ -9,6 +9,7 @@
import 'dart:math';
import 'package:collection/collection.dart';
+import 'package:glob/glob.dart';
import 'package:path/path.dart' as path;
import 'package:pub_semver/pub_semver.dart';
@@ -52,11 +53,23 @@
this.warnOn,
this.failOn,
this.coverageweb,
+ List<String> ignoredPackages,
+ List<String> ignoredLicense,
+ List<String> ignoredCoverage,
this.experiments,
this.github, {
Directory? base,
String? comment,
- }) : baseDirectory = base ?? Directory('../base_repo'),
+ }) : ignoredPackages = ignoredPackages
+ .map((pattern) => Glob(pattern, recursive: true))
+ .toList(),
+ ignoredFilesForCoverage = ignoredCoverage
+ .map((pattern) => Glob(pattern, recursive: true))
+ .toList(),
+ ignoredFilesForLicense = ignoredLicense
+ .map((pattern) => Glob(pattern, recursive: true))
+ .toList(),
+ baseDirectory = base ?? Directory('../base_repo'),
commentPath = comment ??
path.join(
directory.path,
@@ -69,6 +82,9 @@
final List<String> warnOn;
final List<String> failOn;
final bool coverageweb;
+ final List<Glob> ignoredPackages;
+ final List<Glob> ignoredFilesForLicense;
+ final List<Glob> ignoredFilesForCoverage;
final Directory baseDirectory;
final List<String> experiments;
@@ -78,7 +94,15 @@
if (!expectEnv(github.issueNumber?.toString(), 'ISSUE_NUMBER')) return;
if (!expectEnv(github.sha, 'GITHUB_SHA')) return;
- print('Start health check for the check $check');
+ print('Start health check for the check $check with');
+ print(' warnOn: $warnOn');
+ print(' failOn: $failOn');
+ print(' coverageweb: $coverageweb');
+ print(' ignoredPackages: $ignoredPackages');
+ print(' ignoredForLicense: $ignoredFilesForLicense');
+ print(' ignoredForCoverage: $ignoredFilesForCoverage');
+ print(' baseDirectory: $baseDirectory');
+ print(' experiments: $experiments');
print('Checking for $check');
if (!github.prLabels.contains('skip-$check-check')) {
final firstResult = await checkFor(check)();
@@ -121,7 +145,8 @@
Future<HealthCheckResult> validateCheck() async {
//TODO: Add Flutter support for PR health checks
- var results = await Firehose(directory, false).verify(github);
+ var results =
+ await Firehose(directory, false).verify(github, ignoredPackages);
var markdownTable = '''
| Package | Version | Status |
@@ -139,9 +164,9 @@
}
Future<HealthCheckResult> breakingCheck() async {
- final filesInPR = await github.listFilesForPR(directory);
+ final filesInPR = await github.listFilesForPR(directory, ignoredPackages);
final changeForPackage = <Package, BreakingChange>{};
- for (var package in packagesContaining(filesInPR)) {
+ for (var package in packagesContaining(filesInPR, ignoredPackages)) {
print('Look for changes in $package with base $baseDirectory');
var relativePath =
path.relative(package.directory.path, from: directory.path);
@@ -211,8 +236,11 @@
}
Future<HealthCheckResult> licenseCheck() async {
- var files = await github.listFilesForPR(directory);
- var allFilePaths = await getFilesWithoutLicenses(directory);
+ var files = await github.listFilesForPR(directory, ignoredPackages);
+ var allFilePaths = await getFilesWithoutLicenses(
+ directory,
+ [...ignoredFilesForLicense, ...ignoredPackages],
+ );
var groupedPaths = allFilePaths.groupListsBy((filePath) {
return files.any((f) => f.filename == filePath);
@@ -255,7 +283,11 @@
}
Future<HealthCheckResult> changelogCheck() async {
- var filePaths = await packagesWithoutChangelog(github, directory);
+ var filePaths = await packagesWithoutChangelog(
+ github,
+ ignoredPackages,
+ directory,
+ );
final markdownResult = '''
| Package | Changed Files |
@@ -276,14 +308,14 @@
final dns = 'DO_NOT${'_'}SUBMIT';
final body = await github.pullrequestBody();
- final files = await github.listFilesForPR(directory);
- print('Checking for $dns strings: $files');
+ final files = await github.listFilesForPR(directory, ignoredPackages);
+ print('Checking for DO_NOT${'_'}SUBMIT strings: $files');
final filesWithDNS = files
.where((file) =>
![FileStatus.removed, FileStatus.unchanged].contains(file.status))
- .where((file) => File(file.pathInRepository).existsSync())
- .where((file) =>
- File(file.pathInRepository).readAsStringSync().contains(dns))
+ .where((file) => File(file.pathInRepository)
+ .readAsStringSync()
+ .contains('DO_NOT${'_'}SUBMIT'))
.toList();
print('Found files with $dns: $filesWithDNS');
@@ -306,8 +338,13 @@
}
Future<HealthCheckResult> coverageCheck() async {
- var coverage = await Coverage(coverageweb, directory, experiments)
- .compareCoverages(github, baseDirectory);
+ var coverage = await Coverage(
+ coverageweb,
+ ignoredFilesForCoverage,
+ ignoredPackages,
+ directory,
+ experiments,
+ ).compareCoverages(github, directory);
var markdownResult = '''
| File | Coverage |
@@ -361,11 +398,13 @@
}
}
- List<Package> packagesContaining(List<GitFile> filesInPR) {
+ List<Package> packagesContaining(
+ List<GitFile> filesInPR,
+ List<Glob> ignoredPackages,
+ ) {
var files = filesInPR.where((element) => element.status.isRelevant);
- final repo = Repository(directory);
- return repo
- .locatePackages()
+ return Repository(directory)
+ .locatePackages(ignoredPackages)
.where((package) => files.any((file) =>
path.isWithin(package.directory.path, file.pathInRepository)))
.toList();
diff --git a/pkgs/firehose/lib/src/health/license.dart b/pkgs/firehose/lib/src/health/license.dart
index b7bec93..18a1e43 100644
--- a/pkgs/firehose/lib/src/health/license.dart
+++ b/pkgs/firehose/lib/src/health/license.dart
@@ -5,6 +5,7 @@
import 'dart:io';
import 'package:collection/collection.dart';
+import 'package:glob/glob.dart';
import 'package:path/path.dart' as path;
final license = '''
@@ -12,20 +13,24 @@
// 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.''';
-Future<List<String>> getFilesWithoutLicenses(Directory repositoryDir) async {
+Future<List<String>> getFilesWithoutLicenses(
+ Directory repositoryDir, List<Glob> ignoredFiles) async {
var dartFiles = await repositoryDir
.list(recursive: true)
- .where((f) => f.path.endsWith('.dart'))
+ .where((file) => file.path.endsWith('.dart'))
.toList();
print('Collecting files without license headers:');
var filesWithoutLicenses = dartFiles
.map((file) {
- var fileContents = File(file.path).readAsStringSync();
- var fileContainsCopyright = fileContents.contains('// Copyright (c)');
- if (!fileContainsCopyright) {
- var relativePath = path.relative(file.path, from: repositoryDir.path);
- print(relativePath);
- return relativePath;
+ var relativePath = path.relative(file.path, from: repositoryDir.path);
+ if (ignoredFiles.none((glob) =>
+ glob.matches(path.relative(file.path, from: repositoryDir.path)))) {
+ var fileContents = File(file.path).readAsStringSync();
+ var fileContainsCopyright = fileContents.contains('// Copyright (c)');
+ if (!fileContainsCopyright) {
+ print(relativePath);
+ return relativePath;
+ }
}
})
.whereType<String>()
diff --git a/pkgs/firehose/lib/src/repo.dart b/pkgs/firehose/lib/src/repo.dart
index 8b29aa4..1cdd1df 100644
--- a/pkgs/firehose/lib/src/repo.dart
+++ b/pkgs/firehose/lib/src/repo.dart
@@ -4,6 +4,7 @@
import 'dart:io';
+import 'package:glob/glob.dart';
import 'package:path/path.dart' as path;
import 'package:pub_semver/pub_semver.dart';
import 'package:pubspec_parse/pubspec_parse.dart';
@@ -39,9 +40,11 @@
/// `publish_to: none` key.
///
/// Once we find a package, we don't look for packages in sub-directories.
- List<Package> locatePackages() {
+ List<Package> locatePackages([List<Glob> ignore = const []]) {
final packages = <Package>[];
_recurseAndGather(baseDirectory, packages);
+ packages.removeWhere((package) => ignore.any((glob) => glob.matches(
+ path.relative(package.directory.path, from: baseDirectory.path))));
packages.sort((a, b) => a.name.compareTo(b.name));
return packages;
}
diff --git a/pkgs/firehose/pubspec.yaml b/pkgs/firehose/pubspec.yaml
index 6c8d36c..8c06421 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.6.0
+version: 0.6.1
repository: https://github.com/dart-lang/ecosystem/tree/main/pkgs/firehose
environment:
@@ -14,6 +14,7 @@
args: ^2.3.0
collection: ^1.17.2
github: ^9.20.0
+ glob: ^2.1.2
http: ^1.0.0
path: ^1.8.0
pub_semver: ^2.1.0
diff --git a/pkgs/firehose/test/coverage_test.dart b/pkgs/firehose/test/coverage_test.dart
index 51922a0..9a06663 100644
--- a/pkgs/firehose/test/coverage_test.dart
+++ b/pkgs/firehose/test/coverage_test.dart
@@ -43,7 +43,7 @@
}
class FakeHealth extends Coverage {
- FakeHealth() : super(true, Directory.current, []);
+ FakeHealth() : super(true, [], [], Directory.current, []);
@override
Map<String, double> getCoverage(Package? package) {
diff --git a/pkgs/firehose/test/health_test.dart b/pkgs/firehose/test/health_test.dart
index a0e7b13..33025f6 100644
--- a/pkgs/firehose/test/health_test.dart
+++ b/pkgs/firehose/test/health_test.dart
@@ -4,60 +4,97 @@
import 'dart:io';
+import 'package:collection/collection.dart';
import 'package:firehose/src/github.dart';
import 'package:firehose/src/health/health.dart';
import 'package:github/src/common/model/repos.dart';
+import 'package:glob/glob.dart';
import 'package:path/path.dart' as p;
import 'package:test/test.dart';
-void main() {
- test('Check health workflow against golden files', () async {
- final directory = Directory(p.join('test_data', 'test_repo'));
- var fakeGithubApi = FakeGithubApi(prLabels: [], files: [
- GitFile(
- 'pkgs/package1/bin/package1.dart',
- FileStatus.modified,
- directory,
- ),
- GitFile(
- 'pkgs/package2/lib/anotherLib.dart',
- FileStatus.added,
- directory,
- ),
- ]);
- await Process.run('dart', ['pub', 'global', 'activate', 'dart_apitool']);
- await Process.run('dart', ['pub', 'global', 'activate', 'coverage']);
- for (var check in checkTypes) {
- var comment = await checkFor(check, fakeGithubApi, directory);
- var goldenFile = File(p.join('test_data', 'golden', 'comment_$check.md'));
- var goldenComment = goldenFile.readAsStringSync();
- if (Platform.environment.containsKey('RESET_GOLDEN')) {
- goldenFile.writeAsStringSync(comment);
- } else {
- expect(comment, goldenComment);
+Future<void> main() async {
+ final directory = Directory(p.join('test_data', 'test_repo'));
+ var fakeGithubApi = FakeGithubApi(prLabels: [], files: [
+ GitFile(
+ 'pkgs/package1/bin/package1.dart',
+ FileStatus.modified,
+ directory,
+ ),
+ GitFile(
+ 'pkgs/package2/lib/anotherLib.dart',
+ FileStatus.added,
+ directory,
+ ),
+ ]);
+ await Process.run('dart', ['pub', 'global', 'activate', 'dart_apitool']);
+ await Process.run('dart', ['pub', 'global', 'activate', 'coverage']);
+
+ for (var check in checkTypes) {
+ test(
+ 'Check health workflow "$check" against golden files',
+ () async => await checkGolden(check, fakeGithubApi, directory),
+ timeout: const Timeout(Duration(minutes: 2)),
+ );
+ }
+
+ test('Ignore license test', () async {
+ await checkGolden(
+ 'license',
+ fakeGithubApi,
+ directory,
+ suffix: '_ignore_license',
+ ignoredLicense: ['pkgs/package3/**'],
+ );
+ });
+
+ test(
+ 'Ignore packages test',
+ () async {
+ for (var check in checkTypes) {
+ await checkGolden(
+ check,
+ fakeGithubApi,
+ directory,
+ suffix: '_ignore_package',
+ ignoredPackage: ['pkgs/package1'],
+ );
}
- }
- }, timeout: const Timeout(Duration(minutes: 2)));
+ },
+ timeout: const Timeout(Duration(minutes: 2)),
+ );
}
-Future<String> checkFor(
+Future<void> checkGolden(
String check,
FakeGithubApi fakeGithubApi,
- Directory directory,
-) async {
- final comment = p.join(Directory.systemTemp.path, 'comment_$check.md');
+ Directory directory, {
+ String suffix = '',
+ List<String> ignoredLicense = const [],
+ List<String> ignoredPackage = const [],
+}) async {
+ final commentPath = p.join(Directory.systemTemp.path, 'comment_$check.md');
await Health(
directory,
check,
[],
[],
false,
+ ignoredPackage,
+ ignoredLicense,
+ [],
[],
fakeGithubApi,
base: Directory(p.join('test_data', 'base_test_repo')),
- comment: comment,
+ comment: commentPath,
).healthCheck();
- return await File(comment).readAsString();
+ var comment = await File(commentPath).readAsString();
+ var goldenFile =
+ File(p.join('test_data', 'golden', 'comment_$check$suffix.md'));
+ if (Platform.environment['RESET_GOLDEN'] == '1') {
+ goldenFile.writeAsStringSync(comment);
+ } else {
+ expect(comment, goldenFile.readAsStringSync());
+ }
}
class FakeGithubApi implements GithubApi {
@@ -95,8 +132,12 @@
int? get issueNumber => 1;
@override
- Future<List<GitFile>> listFilesForPR(Directory directory) async {
- return files;
+ Future<List<GitFile>> listFilesForPR(Directory directory,
+ [List<Glob> ignoredFiles = const []]) async {
+ return files
+ .where((element) =>
+ ignoredFiles.none((p0) => p0.matches(element.filename)))
+ .toList();
}
@override
diff --git a/pkgs/firehose/test/license_test.dart b/pkgs/firehose/test/license_test.dart
index b8af03f..ac971b9 100644
--- a/pkgs/firehose/test/license_test.dart
+++ b/pkgs/firehose/test/license_test.dart
@@ -22,7 +22,7 @@
test('Check for licenses', () async {
var directory = Directory('test/');
- var filesWithoutLicenses = await getFilesWithoutLicenses(directory);
+ var filesWithoutLicenses = await getFilesWithoutLicenses(directory, []);
expect(filesWithoutLicenses,
[path.relative(fileWithoutLicense.path, from: directory.path)]);
});
diff --git a/pkgs/firehose/test_data/golden/comment_breaking_ignore_package.md b/pkgs/firehose/test_data/golden/comment_breaking_ignore_package.md
new file mode 100644
index 0000000..d336860
--- /dev/null
+++ b/pkgs/firehose/test_data/golden/comment_breaking_ignore_package.md
@@ -0,0 +1,15 @@
+### Breaking changes :warning:
+
+<details open>
+<summary>
+Details
+</summary>
+
+| Package | Change | Current Version | New Version | Needed Version | Looking good? |
+| :--- | :--- | ---: | ---: | ---: | ---: |
+|package2|Non-Breaking|1.0.0|1.0.0|**1.1.0** <br> Got "1.0.0" expected >= "1.1.0" (non-breaking changes)|:warning:|
+
+
+This check can be disabled by tagging the PR with `skip-breaking-check`
+</details>
+
diff --git a/pkgs/firehose/test_data/golden/comment_changelog_ignore_package.md b/pkgs/firehose/test_data/golden/comment_changelog_ignore_package.md
new file mode 100644
index 0000000..d7b7d12
--- /dev/null
+++ b/pkgs/firehose/test_data/golden/comment_changelog_ignore_package.md
@@ -0,0 +1,17 @@
+### Changelog Entry :exclamation:
+
+<details open>
+<summary>
+Details
+</summary>
+
+| Package | Changed Files |
+| :--- | :--- |
+| package:package2 | pkgs/package2/lib/anotherLib.dart |
+
+Changes to files need to be [accounted for](https://github.com/dart-lang/ecosystem/wiki/Changelog) in their respective changelogs.
+
+
+This check can be disabled by tagging the PR with `skip-changelog-check`
+</details>
+
diff --git a/pkgs/firehose/test_data/golden/comment_coverage_ignore_package.md b/pkgs/firehose/test_data/golden/comment_coverage_ignore_package.md
new file mode 100644
index 0000000..c6bdd11
--- /dev/null
+++ b/pkgs/firehose/test_data/golden/comment_coverage_ignore_package.md
@@ -0,0 +1,17 @@
+### Coverage :heavy_check_mark:
+
+<details>
+<summary>
+Details
+</summary>
+
+| File | Coverage |
+| :--- | :--- |
+|pkgs/package2/lib/anotherLib.dart| :green_heart: 100 % |
+
+This check for [test coverage](https://github.com/dart-lang/ecosystem/wiki/Test-Coverage) is informational (issues shown here will not fail the PR).
+
+
+
+</details>
+
diff --git a/pkgs/firehose/test_data/golden/comment_do-not-submit_ignore_package.md b/pkgs/firehose/test_data/golden/comment_do-not-submit_ignore_package.md
new file mode 100644
index 0000000..e69de29
--- /dev/null
+++ b/pkgs/firehose/test_data/golden/comment_do-not-submit_ignore_package.md
diff --git a/pkgs/firehose/test_data/golden/comment_license_ignore_license.md b/pkgs/firehose/test_data/golden/comment_license_ignore_license.md
new file mode 100644
index 0000000..4fa23f1
--- /dev/null
+++ b/pkgs/firehose/test_data/golden/comment_license_ignore_license.md
@@ -0,0 +1,39 @@
+### License Headers :exclamation:
+
+<details open>
+<summary>
+Details
+</summary>
+
+```
+// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
+// 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.
+```
+
+| Files |
+| :--- |
+|pkgs/package1/bin/package1.dart|
+|pkgs/package2/lib/anotherLib.dart|
+
+All source files should start with a [license header](https://github.com/dart-lang/ecosystem/wiki/License-Header).
+
+<details>
+<summary>
+Unrelated files missing license headers
+</summary>
+
+| Files |
+| :--- |
+|pkgs/package1/lib/package1.dart|
+|pkgs/package1/test/package1_test.dart|
+|pkgs/package2/lib/package2.dart|
+|pkgs/package2/test/package2_test.dart|
+</details>
+
+
+
+
+This check can be disabled by tagging the PR with `skip-license-check`
+</details>
+
diff --git a/pkgs/firehose/test_data/golden/comment_license_ignore_package.md b/pkgs/firehose/test_data/golden/comment_license_ignore_package.md
new file mode 100644
index 0000000..59c637d
--- /dev/null
+++ b/pkgs/firehose/test_data/golden/comment_license_ignore_package.md
@@ -0,0 +1,39 @@
+### License Headers :exclamation:
+
+<details open>
+<summary>
+Details
+</summary>
+
+```
+// Copyright (c) 2024, the Dart project authors. Please see the AUTHORS file
+// 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.
+```
+
+| Files |
+| :--- |
+|pkgs/package2/lib/anotherLib.dart|
+
+All source files should start with a [license header](https://github.com/dart-lang/ecosystem/wiki/License-Header).
+
+<details>
+<summary>
+Unrelated files missing license headers
+</summary>
+
+| Files |
+| :--- |
+|pkgs/package2/lib/package2.dart|
+|pkgs/package2/test/package2_test.dart|
+|pkgs/package3/bin/package3.dart|
+|pkgs/package3/lib/package3.dart|
+|pkgs/package3/test/package3_test.dart|
+</details>
+
+
+
+
+This check can be disabled by tagging the PR with `skip-license-check`
+</details>
+
diff --git a/pkgs/firehose/test_data/golden/comment_version_ignore_package.md b/pkgs/firehose/test_data/golden/comment_version_ignore_package.md
new file mode 100644
index 0000000..17ab16e
--- /dev/null
+++ b/pkgs/firehose/test_data/golden/comment_version_ignore_package.md
@@ -0,0 +1,18 @@
+### Package publish validation :exclamation:
+
+<details open>
+<summary>
+Details
+</summary>
+
+| Package | Version | Status |
+| :--- | ---: | :--- |
+| package:package2 | 1.0.0 | (error) pub publish dry-run failed; add the `publish-ignore-warnings` label to ignore |
+| package:package3 | 1.0.0 | (error) pub publish dry-run failed; add the `publish-ignore-warnings` label to ignore |
+
+Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automation.
+
+
+This check can be disabled by tagging the PR with `skip-version-check`
+</details>
+
