Skip to content

Commit

Permalink
Add more fine grained ignore capabilities to health workflow
Browse files Browse the repository at this point in the history
  • Loading branch information
mosuem committed Dec 17, 2024
1 parent 5ca7c41 commit cfb17cf
Show file tree
Hide file tree
Showing 10 changed files with 129 additions and 93 deletions.
28 changes: 28 additions & 0 deletions .github/workflows/health.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,11 @@ name: Health
# upload_coverage: false
# flutter_packages: "pkgs/my_flutter_package"
# ignore_license: "**.g.dart"
# ignore_changelog: ""
# ignore_coverage: "**.mock.dart,**.g.dart"
# ignore_breaking: ""
# ignore_leaking: ""
# ignore_donotsubmit: ""
# ignore_packages: "pkgs/helper_package"
# checkout_submodules: false
# experiments: "native-assets"
Expand Down Expand Up @@ -97,11 +101,31 @@ on:
default: "\"\""
required: false
type: string
ignore_changelog:
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_breaking:
description: Which files to ignore for the license check.
default: "\"\""
required: false
type: string
ignore_leaking:
description: Which files to ignore for the license check.
default: "\"\""
required: false
type: string
ignore_donotsubmit:
description: Which files to ignore for the license check.
default: "\"\""
required: false
type: string
ignore_packages:
description: Which packages to ignore.
default: "\"\""
Expand Down Expand Up @@ -129,6 +153,7 @@ jobs:
warn_on: ${{ inputs.warn_on }}
local_debug: ${{ inputs.local_debug }}
flutter_packages: ${{ inputs.flutter_packages }}
ignore_changelog: ${{ inputs.ignore_changelog }}
ignore_packages: ${{ inputs.ignore_packages }}
checkout_submodules: ${{ inputs.checkout_submodules }}

Expand Down Expand Up @@ -173,6 +198,7 @@ jobs:
warn_on: ${{ inputs.warn_on }}
local_debug: ${{ inputs.local_debug }}
flutter_packages: ${{ inputs.flutter_packages }}
ignore_breaking: ${{ inputs.ignore_breaking }}
ignore_packages: ${{ inputs.ignore_packages }}
checkout_submodules: ${{ inputs.checkout_submodules }}

Expand All @@ -186,6 +212,7 @@ jobs:
warn_on: ${{ inputs.warn_on }}
local_debug: ${{ inputs.local_debug }}
flutter_packages: ${{ inputs.flutter_packages }}
ignore_donotsubmit: ${{ inputs.ignore_donotsubmit }}
ignore_packages: ${{ inputs.ignore_packages }}
checkout_submodules: ${{ inputs.checkout_submodules }}

Expand All @@ -199,6 +226,7 @@ jobs:
warn_on: ${{ inputs.warn_on }}
local_debug: ${{ inputs.local_debug }}
flutter_packages: ${{ inputs.flutter_packages }}
ignore_leaking: ${{ inputs.ignore_leaking }}
ignore_packages: ${{ inputs.ignore_packages }}
checkout_submodules: ${{ inputs.checkout_submodules }}

Expand Down
26 changes: 25 additions & 1 deletion .github/workflows/health_base.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -62,11 +62,31 @@ on:
default: "\"\""
required: false
type: string
ignore_changelog:
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_breaking:
description: Which files to ignore for the license check.
default: "\"\""
required: false
type: string
ignore_leaking:
description: Which files to ignore for the license check.
default: "\"\""
required: false
type: string
ignore_donotsubmit:
description: Which files to ignore for the license check.
default: "\"\""
required: false
type: string
ignore_packages:
description: Which packages to ignore.
default: "\"\""
Expand Down Expand Up @@ -149,9 +169,13 @@ jobs:
--fail_on ${{ inputs.fail_on }} \
--warn_on ${{ inputs.warn_on }} \
--flutter_packages ${{ inputs.flutter_packages }} \
--ignore_packages ${{ inputs.ignore_packages }} \
--ignore_license ${{ inputs.ignore_license }} \
--ignore_changelog ${{ inputs.ignore_changelog }} \
--ignore_coverage ${{ inputs.ignore_coverage }} \
--ignore_packages ${{ inputs.ignore_packages }} \
--ignore_breaking ${{ inputs.ignore_breaking }} \
--ignore_leaking ${{ inputs.ignore_leaking }} \
--ignore_donotsubmit ${{ inputs.ignore_donotsubmit }} \
--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
21 changes: 10 additions & 11 deletions pkgs/firehose/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -229,17 +229,16 @@ jobs:

| 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,leaking"` |
| 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,leaking"` |
| 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"` |
| `checks` | List of strings | What to check for in the PR health check | `"version,changelog,license,coverage,breaking,do-not-submit,leaking"` |
| `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,leaking"` |
| `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` |
| `flutter_packages` | List of strings | List of packages depending on Flutter | `"pkgs/intl_flutter"` |
| `ignore_*` | List of globs | Files to ignore, where `*` can be `license`, `changelog`, `coverage`, `breaking`, `leaking`, or `donotsubmit` | `"**.g.dart"` |
| `ignore_packages` | List of globs | Which packages to ignore completely | `"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

Expand Down
28 changes: 12 additions & 16 deletions pkgs/firehose/bin/health.dart
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import 'package:firehose/src/github.dart';
import 'package:firehose/src/health/health.dart';

void main(List<String> arguments) async {
var checkTypes = Check.values.map((c) => c.name);
var checkTypes = Check.values.map((c) => c.displayName);
var argParser = ArgParser()
..addOption(
'check',
Expand All @@ -21,16 +21,6 @@ void main(List<String> arguments) async {
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 @@ -54,15 +44,22 @@ void main(List<String> arguments) async {
defaultsTo: [],
help: 'The Flutter packages in this repo',
);
for (var check in Check.values) {
argParser.addMultiOption(
'ignore_${check.name}',
defaultsTo: [],
help: 'Which files to ignore for the ${check.displayName} check.',
);
}
final parsedArgs = argParser.parse(arguments);
final checkStr = parsedArgs.option('check');
final check = Check.values.firstWhere((c) => c.name == checkStr);
final check = Check.values.firstWhere((c) => c.displayName == checkStr);
final warnOn = parsedArgs.multiOption('warn_on');
final failOn = parsedArgs.multiOption('fail_on');
final flutterPackages = _listNonEmpty(parsedArgs, 'flutter_packages');
final ignorePackages = _listNonEmpty(parsedArgs, 'ignore_packages');
final ignoreLicense = _listNonEmpty(parsedArgs, 'ignore_license');
final ignoreCoverage = _listNonEmpty(parsedArgs, 'ignore_coverage');
final ignoredFor = Map.fromEntries(Check.values
.map((c) => MapEntry(c, _listNonEmpty(parsedArgs, 'ignore_${c.name}'))));
final experiments = _listNonEmpty(parsedArgs, 'experiments');
final coverageWeb = parsedArgs.flag('coverage_web');
if (warnOn.toSet().intersection(failOn.toSet()).isNotEmpty) {
Expand All @@ -76,8 +73,7 @@ void main(List<String> arguments) async {
failOn,
coverageWeb,
ignorePackages,
ignoreLicense,
ignoreCoverage,
ignoredFor,
experiments,
GithubApi(),
flutterPackages,
Expand Down
7 changes: 3 additions & 4 deletions pkgs/firehose/lib/src/health/changelog.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,12 @@ import '../repo.dart';

Future<Map<Package, List<GitFile>>> packagesWithoutChangelog(
GithubApi github,
List<Glob> ignoredPackages,
List<Glob> ignored,
Directory directory,
) async {
final repo = Repository(directory);
final packages = repo.locatePackages(ignore: ignoredPackages);

final files = await github.listFilesForPR(directory, ignoredPackages);
final packages = repo.locatePackages(ignore: ignored);
final files = await github.listFilesForPR(directory, ignored);

var packagesWithoutChangedChangelog = collectPackagesWithoutChangelogChanges(
packages,
Expand Down
13 changes: 5 additions & 8 deletions pkgs/firehose/lib/src/health/coverage.dart
Original file line number Diff line number Diff line change
Expand Up @@ -16,38 +16,35 @@ import 'lcov.dart';

class Coverage {
final bool coverageWeb;
final List<Glob> ignoredFiles;
final List<Glob> ignoredPackages;
final List<Glob> ignored;
final Directory directory;
final List<String> experiments;
final String dartExecutable;

Coverage(
this.coverageWeb,
this.ignoredFiles,
this.ignoredPackages,
this.ignored,
this.directory,
this.experiments,
this.dartExecutable,
);

CoverageResult compareCoveragesFor(List<GitFile> files, Directory base) {
var repository = Repository(directory);
var packages = repository.locatePackages(ignore: ignoredPackages);
var packages = repository.locatePackages(ignore: ignored);
print('Found packages $packages at $directory');

var filesOfInterest = files
.where((file) => path.extension(file.filename) == '.dart')
.where((file) => file.status != FileStatus.removed)
.where((file) => isInSomePackage(packages, file.filename))
.where((file) => isNotATest(packages, file.filename))
.where(
(file) => ignoredFiles.none((glob) => glob.matches(file.filename)))
.where((file) => ignored.none((glob) => glob.matches(file.filename)))
.toList();
print('The files of interest are $filesOfInterest');

var baseRepository = Repository(base);
var basePackages = baseRepository.locatePackages(ignore: ignoredPackages);
var basePackages = baseRepository.locatePackages(ignore: ignored);
print('Found packages $basePackages at $base');

var changedPackages = packages
Expand Down
Loading

0 comments on commit cfb17cf

Please sign in to comment.