diff --git a/.github/workflows/health.yaml b/.github/workflows/health.yaml index 87ce1e6f..3e848b01 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 @@ name: Health # 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 @@ on: 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 @@ jobs: 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 @@ jobs: 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 @@ jobs: 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 @@ jobs: 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 @@ jobs: 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 @@ jobs: 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 b8faf864..589eabd1 100644 --- a/.github/workflows/health_base.yaml +++ b/.github/workflows/health_base.yaml @@ -49,6 +49,21 @@ on: 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 @@ jobs: ${{ 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 a6339335..270a0b8b 100644 --- a/.github/workflows/health_internal.yaml +++ b/.github/workflows/health_internal.yaml @@ -15,3 +15,5 @@ jobs: 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 2d232c98..ca3a4010 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 51e53044..30d7abdf 100644 --- a/pkgs/firehose/README.md +++ b/pkgs/firehose/README.md @@ -160,10 +160,12 @@ This is a Github workflow to check PR health. 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 @@ jobs: # 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 e1260dc6..5a36b213 100644 --- a/pkgs/firehose/bin/health.dart +++ b/pkgs/firehose/bin/health.dart @@ -15,6 +15,21 @@ void main(List arguments) async { allowed: checkTypes, 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, @@ -33,14 +48,15 @@ void main(List arguments) async { '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; - var failOn = parsedArgs['fail_on'] as List; - var experiments = (parsedArgs['experiments'] as List) - .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; + final failOn = parsedArgs['fail_on'] as List; + 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 @@ void main(List arguments) async { warnOn, failOn, coverageWeb, + ignorePackages, + ignoreLicense, + ignoreCoverage, experiments, GithubApi(), ).healthCheck(); } + +List _listNonEmpty(ArgResults parsedArgs, String key) => + (parsedArgs[key] as List).where((e) => e.isNotEmpty).toList(); diff --git a/pkgs/firehose/lib/firehose.dart b/pkgs/firehose/lib/firehose.dart index f5b5c0d2..3f4ce73e 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 @@ Saving existing comment id $existingCommentId to file ${idFile.path}'''); github.close(); } - Future verify(GithubApi github) async { + Future verify( + GithubApi github, [ + List 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 e3e07b82..4904c83a 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 @@ class GithubApi { return matchingComment?.id; } - Future> listFilesForPR(Directory directory) async => + Future> listFilesForPR( + Directory directory, [ + List ignoredFiles = const [], + ]) async => await _github.pullRequests .listFiles(repoSlug!, issueNumber!) .map((prFile) => GitFile( @@ -115,6 +120,8 @@ class GithubApi { 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 @@ class GitFile { 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 34742622..95ad14ab 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 @@ import '../repo.dart'; Future>> packagesWithoutChangelog( GithubApi github, + List 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 18004d26..bb0610dc 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 @@ import 'lcov.dart'; class Coverage { final bool coverageWeb; + final List ignoredFiles; + final List ignoredPackages; final Directory directory; final List experiments; - Coverage(this.coverageWeb, this.directory, this.experiments); + Coverage( + this.coverageWeb, + this.ignoredFiles, + this.ignoredPackages, + this.directory, + this.experiments, + ); Future 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 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 @@ class Coverage { 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 @@ Get coverage for ${package.name} by running coverage in ${package.directory.path ], 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 @@ Get coverage for ${package.name} by running coverage in ${package.directory.path ], 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 @@ Get coverage for ${package.name} by running coverage in ${package.directory.path ], 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 94ab8d03..47baf974 100644 --- a/pkgs/firehose/lib/src/health/health.dart +++ b/pkgs/firehose/lib/src/health/health.dart @@ -9,6 +9,7 @@ import 'dart:io'; 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 @@ class Health { this.warnOn, this.failOn, this.coverageweb, + List ignoredPackages, + List ignoredLicense, + List 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 @@ class Health { final List warnOn; final List failOn; final bool coverageweb; + final List ignoredPackages; + final List ignoredFilesForLicense; + final List ignoredFilesForCoverage; final Directory baseDirectory; final List experiments; @@ -78,7 +94,15 @@ class Health { 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 @@ class Health { Future 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 @@ Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automati } Future breakingCheck() async { - final filesInPR = await github.listFilesForPR(directory); + final filesInPR = await github.listFilesForPR(directory, ignoredPackages); final changeForPackage = {}; - 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 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()} } Future 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 @@ ${unchangedFilesPaths.isNotEmpty ? unchangedMarkdown : ''} } Future changelogCheck() async { - var filePaths = await packagesWithoutChangelog(github, directory); + var filePaths = await packagesWithoutChangelog( + github, + ignoredPackages, + directory, + ); final markdownResult = ''' | Package | Changed Files | @@ -276,14 +308,14 @@ Changes to files need to be [accounted for](https://github.com/dart-lang/ecosyst 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 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')} } Future 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 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r } } - List packagesContaining(List filesInPR) { + List packagesContaining( + List filesInPR, + List 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 b7bec935..18a1e433 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 @@ final license = ''' // 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> getFilesWithoutLicenses(Directory repositoryDir) async { +Future> getFilesWithoutLicenses( + Directory repositoryDir, List 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() diff --git a/pkgs/firehose/lib/src/repo.dart b/pkgs/firehose/lib/src/repo.dart index 8b29aa42..1cdd1df6 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 @@ class Repository { /// `publish_to: none` key. /// /// Once we find a package, we don't look for packages in sub-directories. - List locatePackages() { + List locatePackages([List ignore = const []]) { final packages = []; _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 6c8d36cc..8c064218 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 @@ dependencies: 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 51922a09..9a066635 100644 --- a/pkgs/firehose/test/coverage_test.dart +++ b/pkgs/firehose/test/coverage_test.dart @@ -43,7 +43,7 @@ void main() { } class FakeHealth extends Coverage { - FakeHealth() : super(true, Directory.current, []); + FakeHealth() : super(true, [], [], Directory.current, []); @override Map getCoverage(Package? package) { diff --git a/pkgs/firehose/test/health_test.dart b/pkgs/firehose/test/health_test.dart index a0e7b136..33025f66 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 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 checkFor( +Future checkGolden( String check, FakeGithubApi fakeGithubApi, - Directory directory, -) async { - final comment = p.join(Directory.systemTemp.path, 'comment_$check.md'); + Directory directory, { + String suffix = '', + List ignoredLicense = const [], + List 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 @@ class FakeGithubApi implements GithubApi { int? get issueNumber => 1; @override - Future> listFilesForPR(Directory directory) async { - return files; + Future> listFilesForPR(Directory directory, + [List 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 b8af03f3..ac971b9d 100644 --- a/pkgs/firehose/test/license_test.dart +++ b/pkgs/firehose/test/license_test.dart @@ -22,7 +22,7 @@ void main() { 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 00000000..d3368608 --- /dev/null +++ b/pkgs/firehose/test_data/golden/comment_breaking_ignore_package.md @@ -0,0 +1,15 @@ +### Breaking changes :warning: + +
+ +Details + + +| Package | Change | Current Version | New Version | Needed Version | Looking good? | +| :--- | :--- | ---: | ---: | ---: | ---: | +|package2|Non-Breaking|1.0.0|1.0.0|**1.1.0**
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` +
+ 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 00000000..d7b7d12f --- /dev/null +++ b/pkgs/firehose/test_data/golden/comment_changelog_ignore_package.md @@ -0,0 +1,17 @@ +### Changelog Entry :exclamation: + +
+ +Details + + +| 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` +
+ 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 00000000..c6bdd11a --- /dev/null +++ b/pkgs/firehose/test_data/golden/comment_coverage_ignore_package.md @@ -0,0 +1,17 @@ +### Coverage :heavy_check_mark: + +
+ +Details + + +| 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). + + + +
+ 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 00000000..e69de29b 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 00000000..4fa23f1a --- /dev/null +++ b/pkgs/firehose/test_data/golden/comment_license_ignore_license.md @@ -0,0 +1,39 @@ +### License Headers :exclamation: + +
+ +Details + + +``` +// 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). + +
+ +Unrelated files missing license headers + + +| Files | +| :--- | +|pkgs/package1/lib/package1.dart| +|pkgs/package1/test/package1_test.dart| +|pkgs/package2/lib/package2.dart| +|pkgs/package2/test/package2_test.dart| +
+ + + + +This check can be disabled by tagging the PR with `skip-license-check` +
+ 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 00000000..59c637d8 --- /dev/null +++ b/pkgs/firehose/test_data/golden/comment_license_ignore_package.md @@ -0,0 +1,39 @@ +### License Headers :exclamation: + +
+ +Details + + +``` +// 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). + +
+ +Unrelated files missing license headers + + +| 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| +
+ + + + +This check can be disabled by tagging the PR with `skip-license-check` +
+ 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 00000000..17ab16e6 --- /dev/null +++ b/pkgs/firehose/test_data/golden/comment_version_ignore_package.md @@ -0,0 +1,18 @@ +### Package publish validation :exclamation: + +
+ +Details + + +| 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` +
+