Skip to content

Commit

Permalink
Switch to specific ignores
Browse files Browse the repository at this point in the history
  • Loading branch information
mosuem committed Jan 12, 2024
1 parent 9919dd0 commit c7073d5
Show file tree
Hide file tree
Showing 9 changed files with 90 additions and 39 deletions.
26 changes: 18 additions & 8 deletions .github/workflows/health.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,16 @@ on:
default: false
required: false
type: boolean
ignore:
description: Which files to ignore when checking for licenses.
ignore_license:
description: Which files to ignore for the license check.
required: false
type: string
ignore_coverage:
description: Which files to ignore for the coverage check.
required: false
type: string
ignore_packages:
description: Which packages to ignore.
required: false
type: string

Expand All @@ -98,7 +106,7 @@ jobs:
warn_on: ${{ inputs.warn_on }}
local_debug: ${{ inputs.local_debug }}
use-flutter: ${{ inputs.use-flutter }}
ignore: ${{ inputs.ignore }}
ignore_packages: ${{ inputs.ignore_packages }}

changelog:
if: ${{ contains(inputs.checks, 'changelog') }}
Expand All @@ -110,7 +118,7 @@ jobs:
warn_on: ${{ inputs.warn_on }}
local_debug: ${{ inputs.local_debug }}
use-flutter: ${{ inputs.use-flutter }}
ignore: ${{ inputs.ignore }}
ignore_packages: ${{ inputs.ignore_packages }}

license:
if: ${{ contains(inputs.checks, 'license') }}
Expand All @@ -122,7 +130,8 @@ jobs:
warn_on: ${{ inputs.warn_on }}
local_debug: ${{ inputs.local_debug }}
use-flutter: ${{ inputs.use-flutter }}
ignore: ${{ inputs.ignore }}
ignore_license: ${{ inputs.ignore_license }}
ignore_packages: ${{ inputs.ignore_packages }}

coverage:
if: ${{ contains(inputs.checks, 'coverage') }}
Expand All @@ -136,7 +145,8 @@ jobs:
coverage_web: ${{ inputs.coverage_web }}
local_debug: ${{ inputs.local_debug }}
use-flutter: ${{ inputs.use-flutter }}
ignore: ${{ inputs.ignore }}
ignore_coverage: ${{ inputs.ignore_coverage }}
ignore_packages: ${{ inputs.ignore_packages }}

breaking:
if: ${{ contains(inputs.checks, 'breaking') }}
Expand All @@ -148,7 +158,7 @@ jobs:
warn_on: ${{ inputs.warn_on }}
local_debug: ${{ inputs.local_debug }}
use-flutter: ${{ inputs.use-flutter }}
ignore: ${{ inputs.ignore }}
ignore_packages: ${{ inputs.ignore_packages }}

do-not-submit:
if: ${{ contains(inputs.checks, 'do-not-submit') }}
Expand All @@ -160,7 +170,7 @@ jobs:
warn_on: ${{ inputs.warn_on }}
local_debug: ${{ inputs.local_debug }}
use-flutter: ${{ inputs.use-flutter }}
ignore: ${{ inputs.ignore }}
ignore_packages: ${{ inputs.ignore_packages }}

comment:
needs: [version, changelog, license, coverage, breaking, do-not-submit]
Expand Down
17 changes: 13 additions & 4 deletions .github/workflows/health_base.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,16 @@ on:
default: false
required: false
type: boolean
ignore:
description: The files to ignore for this check.
default: "[!*]"
ignore_license:
description: Which files to ignore for the license check.
required: false
type: string
ignore_coverage:
description: Which files to ignore for the coverage check.
required: false
type: string
ignore_packages:
description: Which packages to ignore.
required: false
type: string

Expand Down Expand Up @@ -117,7 +124,9 @@ jobs:
${{ fromJSON('{"true":"--coverage_web","false":""}')[inputs.coverage_web] }} \
--fail_on ${{ inputs.fail_on }} \
--warn_on ${{ inputs.warn_on }} \
--ignore ${{ inputs.ignore }}
--ignore_license ${{ inputs.ignore_license }} \
--ignore_coverage ${{ inputs.ignore_coverage }} \
--ignore_packages ${{ inputs.ignore_packages }}
- 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
if: ${{ '$action_state' == 1 }}
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/health_internal.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -15,4 +15,4 @@ jobs:
checks: version,changelog,license,coverage,breaking,do-not-submit
fail_on: version,changelog,do-not-submit
warn_on: license,coverage,breaking
ignore_license: '**.g.dart'
ignore: '**.g.dart'
28 changes: 23 additions & 5 deletions pkgs/firehose/bin/health.dart
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,16 @@ void main(List<String> arguments) async {
help: 'Check PR health.',
)
..addMultiOption(
'ignore',
help: 'File which should be ignored by the health workflow.',
'ignore_packages',
help: 'Which packages to ignore.',
)
..addMultiOption(
'ignore_license',
help: 'Which files to ignore for the license check.',
)
..addMultiOption(
'ignore_coverage',
help: 'Which files to ignore for the coverage check.',
)
..addMultiOption(
'warn_on',
Expand All @@ -36,12 +44,22 @@ void main(List<String> arguments) async {
var check = parsedArgs['check'] as String;
var warnOn = parsedArgs['warn_on'] as List<String>;
var failOn = parsedArgs['fail_on'] as List<String>;
var ignore = parsedArgs['ignore'] as List<String>;
var ignorePackages = parsedArgs['ignore_packages'] as List<String>;
var ignoreLicense = parsedArgs['ignore_license'] as List<String>;
var ignoreCoverage = parsedArgs['ignore_coverage'] as List<String>;
var 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.');
}
await Health(Directory.current, check, warnOn, failOn, coverageWeb, ignore)
.healthCheck();
await Health(
Directory.current,
check,
warnOn,
failOn,
coverageWeb,
ignorePackages,
ignoreLicense,
ignoreCoverage,
).healthCheck();
}
4 changes: 2 additions & 2 deletions pkgs/firehose/lib/firehose.dart
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,10 @@ Saving existing comment id $existingCommentId to file ${idFile.path}''');

Future<VerificationResults> verify(
GithubApi github, [
List<Glob> ignoredFiles = const [],
List<Glob> ignoredPackages = const [],
]) async {
var repo = Repository();
var packages = repo.locatePackages(ignoredFiles);
var packages = repo.locatePackages(ignoredPackages);

var pub = Pub();

Expand Down
6 changes: 3 additions & 3 deletions pkgs/firehose/lib/src/health/changelog.dart
Original file line number Diff line number Diff line change
Expand Up @@ -12,11 +12,11 @@ import '../repo.dart';
import '../utils.dart';

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

final files = await github.listFilesForPR(ignoredFiles);
final files = await github.listFilesForPR();

var packagesWithoutChangedChangelog =
collectPackagesWithoutChangelogChanges(packages, files);
Expand Down
6 changes: 4 additions & 2 deletions pkgs/firehose/lib/src/health/coverage.dart
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,9 @@ class Coverage {
final bool coverageWeb;
final List<Glob> ignoredFiles;

Coverage(this.coverageWeb, this.ignoredFiles);
final List<Glob> ignoredPackages;

Coverage(this.coverageWeb, this.ignoredFiles, this.ignoredPackages);

Future<CoverageResult> compareCoverages(GithubApi github) async {
var files = await github.listFilesForPR(ignoredFiles);
Expand All @@ -28,7 +30,7 @@ class Coverage {

CoverageResult compareCoveragesFor(List<GitFile> files, String basePath) {
var repository = Repository();
var packages = repository.locatePackages(ignoredFiles);
var packages = repository.locatePackages(ignoredPackages);
print('Found packages $packages at ${Directory.current}');

var filesOfInterest = files
Expand Down
38 changes: 25 additions & 13 deletions pkgs/firehose/lib/src/health/health.dart
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,21 @@ class Health {
this.warnOn,
this.failOn,
this.coverageweb,
List<String> ignored,
) : ignoredFiles = ignored.map(Glob.new).toList();
List<String> ignoredPackages,
List<String> ignoredLicense,
List<String> ignoredCoverage,
) : ignoredPackages = ignoredPackages.map(Glob.new).toList(),
ignoredFilesForCoverage = ignoredCoverage.map(Glob.new).toList(),
ignoredFilesForLicense = ignoredLicense.map(Glob.new).toList();
final github = GithubApi();

final String check;
final List<String> warnOn;
final List<String> failOn;
final bool coverageweb;
final List<Glob> ignoredFiles;
final List<Glob> ignoredPackages;
final List<Glob> ignoredFilesForLicense;
final List<Glob> ignoredFilesForCoverage;

Future<void> healthCheck() async {
// Do basic validation of our expected env var.
Expand Down Expand Up @@ -110,7 +116,8 @@ class Health {

Future<HealthCheckResult> validateCheck() async {
//TODO: Add Flutter support for PR health checks
var results = await Firehose(directory, false).verify(github, ignoredFiles);
var results =
await Firehose(directory, false).verify(github, ignoredPackages);

var markdownTable = '''
| Package | Version | Status |
Expand All @@ -128,7 +135,7 @@ Documentation at https://github.com/dart-lang/ecosystem/wiki/Publishing-automati
}

Future<HealthCheckResult> breakingCheck() async {
final filesInPR = await github.listFilesForPR(ignoredFiles);
final filesInPR = await github.listFilesForPR();
final changeForPackage = <Package, BreakingChange>{};
final baseDirectory = Directory('../base_repo');
for (var package in packagesContaining(filesInPR)) {
Expand Down Expand Up @@ -201,9 +208,11 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()}
}

Future<HealthCheckResult> licenseCheck() async {
var files = await github.listFilesForPR(ignoredFiles);
var allFilePaths =
await getFilesWithoutLicenses(Directory.current, ignoredFiles);
var files = await github.listFilesForPR(ignoredFilesForLicense);
var allFilePaths = await getFilesWithoutLicenses(
Directory.current,
ignoredFilesForLicense,
);

var groupedPaths = allFilePaths
.groupListsBy((path) => files.any((f) => f.relativePath == path));
Expand Down Expand Up @@ -245,7 +254,7 @@ ${unchangedFilesPaths.isNotEmpty ? unchangedMarkdown : ''}
}

Future<HealthCheckResult> changelogCheck() async {
var filePaths = await packagesWithoutChangelog(github, ignoredFiles);
var filePaths = await packagesWithoutChangelog(github, ignoredPackages);

final markdownResult = '''
| Package | Changed Files |
Expand All @@ -264,7 +273,7 @@ Changes to files need to be [accounted for](https://github.com/dart-lang/ecosyst

Future<HealthCheckResult> doNotSubmitCheck() async {
final body = await github.pullrequestBody();
final files = await github.listFilesForPR(ignoredFiles);
final files = await github.listFilesForPR();
print('Checking for DO_NOT${'_'}SUBMIT strings: $files');
final filesWithDNS = files
.where((file) =>
Expand Down Expand Up @@ -294,8 +303,11 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')}
}

Future<HealthCheckResult> coverageCheck() async {
var coverage =
await Coverage(coverageweb, ignoredFiles).compareCoverages(github);
var coverage = await Coverage(
coverageweb,
ignoredFilesForCoverage,
ignoredPackages,
).compareCoverages(github);

var markdownResult = '''
| File | Coverage |
Expand Down Expand Up @@ -352,7 +364,7 @@ ${isWorseThanInfo ? 'This check can be disabled by tagging the PR with `skip-${r
List<Package> packagesContaining(List<GitFile> filesInPR) {
var files = filesInPR.where((element) => element.status.isRelevant);
final repo = Repository();
return repo.locatePackages(ignoredFiles).where((package) {
return repo.locatePackages(ignoredPackages).where((package) {
var relativePackageDirectory =
path.relative(package.directory.path, from: Directory.current.path);
return files.any(
Expand Down
2 changes: 1 addition & 1 deletion pkgs/firehose/test/coverage_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ void main() {
}

class FakeHealth extends Coverage {
FakeHealth() : super(true, []);
FakeHealth() : super(true, [], []);

@override
Map<String, double> getCoverage(Package? package) {
Expand Down

0 comments on commit c7073d5

Please sign in to comment.