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 ignore flag to health workflows #218

Merged
merged 36 commits into from
Jan 29, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
0c31c59
Add `ignore` flag to health workflows
mosuem Jan 8, 2024
6b487f8
Switch to glob
mosuem Jan 8, 2024
ee185bc
Fix multiline
mosuem Jan 8, 2024
7a9ab59
Add default glob
mosuem Jan 8, 2024
cbb7266
Delete multiline
mosuem Jan 8, 2024
9f84be4
Fix errors
mosuem Jan 8, 2024
6f1c834
Fix health commenting
mosuem Jan 8, 2024
128af77
Merge branch 'main' into addIgnoredFilesToHealth
mosuem Jan 12, 2024
9919dd0
Propagate ignore
mosuem Jan 12, 2024
c7073d5
Switch to specific ignores
mosuem Jan 12, 2024
2bacc71
Add defaults
mosuem Jan 12, 2024
4e15f4d
Add test repos
mosuem Jan 15, 2024
0962184
Start adding health tests
mosuem Jan 15, 2024
da5d59b
Add golden files
mosuem Jan 15, 2024
05e0dc9
Fix changelog
mosuem Jan 15, 2024
f623f41
Fix coverage
mosuem Jan 15, 2024
93a0d44
Fix breaking
mosuem Jan 15, 2024
989649e
More fixes
mosuem Jan 15, 2024
eda4a19
Merge branch 'main' into addIgnoredFilesToHealth
mosuem Jan 23, 2024
938801e
Merge
mosuem Jan 23, 2024
1c87747
Fixes
mosuem Jan 23, 2024
075ad90
Move
mosuem Jan 23, 2024
15ce667
Fix tests
mosuem Jan 23, 2024
2b50453
Ignore test data licenses
mosuem Jan 23, 2024
b9e2d2d
Add golden without license
mosuem Jan 24, 2024
7dda2a5
Fix license
mosuem Jan 24, 2024
40d2558
Rename
mosuem Jan 24, 2024
0750244
Fix
mosuem Jan 26, 2024
aeff8e2
Fix analyze issue
mosuem Jan 26, 2024
93efe49
Add args printing
mosuem Jan 26, 2024
45f962b
Add defaults
mosuem Jan 26, 2024
ea82305
Add more defaults
mosuem Jan 26, 2024
e9f3663
Remove empty
mosuem Jan 26, 2024
e948360
Match recursive
mosuem Jan 26, 2024
d0a361e
Add docs
mosuem Jan 29, 2024
214ab4b
Add link to docs
mosuem Jan 29, 2024
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
33 changes: 31 additions & 2 deletions .github/workflows/health.yaml
Original file line number Diff line number Diff line change
@@ -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

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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 }}

Expand All @@ -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:
Expand All @@ -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:
Expand Down
18 changes: 18 additions & 0 deletions .github/workflows/health_base.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
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 @@ -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'
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.6.1

- Add `ignore` flags to the health workflow.

## 0.6.0

- Make the health workflow testable with golden tests.
Expand Down
24 changes: 21 additions & 3 deletions pkgs/firehose/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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).
Expand All @@ -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
Expand Down
38 changes: 30 additions & 8 deletions pkgs/firehose/bin/health.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,21 @@ void main(List<String> 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,
Expand All @@ -33,14 +48,15 @@ void main(List<String> 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<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.');
Expand All @@ -51,7 +67,13 @@ void main(List<String> arguments) async {
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();
9 changes: 7 additions & 2 deletions pkgs/firehose/lib/firehose.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -90,9 +92,12 @@ Saving existing comment id $existingCommentId to file ${idFile.path}''');
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();

Expand Down
14 changes: 10 additions & 4 deletions pkgs/firehose/lib/src/github.dart
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -107,14 +109,19 @@ class GithubApi {
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(
prFile.filename!,
FileStatus.fromString(prFile.status!),
directory,
))
.where((file) =>
ignoredFiles.none((glob) => glob.matches(file.filename)))
.toList();

/// Write a notice message to the github log.
Expand All @@ -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);

Expand Down
6 changes: 4 additions & 2 deletions pkgs/firehose/lib/src/health/changelog.dart
Original file line number Diff line number Diff line change
Expand Up @@ -4,19 +4,21 @@

import 'dart:io';

import 'package:glob/glob.dart';
import 'package:path/path.dart' as path;

import '../github.dart';
import '../repo.dart';

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,
Expand Down
Loading
Loading