Skip to content

Commit

Permalink
Prepare for review
Browse files Browse the repository at this point in the history
  • Loading branch information
mosuem committed Oct 24, 2024
1 parent 123810b commit 75666ad
Show file tree
Hide file tree
Showing 5 changed files with 21 additions and 43 deletions.
4 changes: 1 addition & 3 deletions .github/workflows/health.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ on:
# stable release of the Dart SDK.
sdk:
description: >-
The channel, or a specific version from a channel, to install
('2.19.0','stable', 'beta', 'dev'). Using one of the three channels
will give you the latest version published to that channel.
The Dart SDK version, either a semver or one of `dev`, `stable` etc.
default: "stable"
required: false
type: string
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/health_base.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ on:
required: false
flutter_packages:
description: List of packages depending on Flutter.
default: ''
default: "\"\""
required: false
type: string
ignore_license:
Expand Down
5 changes: 0 additions & 5 deletions pkgs/firehose/lib/src/health/coverage.dart
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,6 @@ class Coverage {
this.experiments,
);

Future<CoverageResult> compareCoverages(
List<GitFile> files, Directory base) async {
return compareCoveragesFor(files, base);
}

CoverageResult compareCoveragesFor(List<GitFile> files, Directory base) {
var repository = Repository(directory);
var packages = repository.locatePackages(ignore: ignoredPackages);
Expand Down
49 changes: 16 additions & 33 deletions pkgs/firehose/lib/src/health/health.dart
Original file line number Diff line number Diff line change
Expand Up @@ -52,29 +52,21 @@ class Health {
Directory? base,
String? comment,
this.log = printLogger,
}) : ignoredPackages = ignoredPackages
.map((e) {
print('IGNORED:PACKAGE $e');
return e;
})
.map((pattern) => Glob(pattern, recursive: true))
.toList(),
flutterPackages = flutterPackages
.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(),
}) : ignoredPackages = toGlobs(ignoredPackages),
flutterPackages = toGlobs(flutterPackages),
ignoredFilesForCoverage = toGlobs(ignoredCoverage),
ignoredFilesForLicense = toGlobs(ignoredLicense),
baseDirectory = base ?? Directory('../base_repo'),
commentPath = comment ??
path.join(
directory.path,
'output',
'comment.md',
);

static List<Glob> toGlobs(List<String> ignoredPackages) =>
ignoredPackages.map((pattern) => Glob(pattern, recursive: true)).toList();

final GithubApi github;

final Check check;
Expand Down Expand Up @@ -214,10 +206,6 @@ ${changeForPackage.entries.map((e) => '|${e.key.name}|${e.value.toMarkdownRow()}

Future<HealthCheckResult> leakingCheck() async {
var filesInPR = await listFilesInPRorAll(ignoredPackages);
// DO-NOT-SUBMIT
print('Files: ${filesInPR.map((e) => e.filename).join(', ')}');
print(
'Files: ${filesInPR.map((e) => e.filename).where((element) => element.contains('swiftgen')).join(', ')}');
final leaksForPackage = <Package, List<String>>{};
for (var package in packagesContaining(filesInPR)) {
log('Look for leaks in $package');
Expand Down Expand Up @@ -318,14 +306,12 @@ ${unchangedFilesPaths.isNotEmpty ? unchangedMarkdown : ''}
);
}

bool healthYamlChanged(List<GitFile> files) {
return files
.where((file) =>
[FileStatus.added, FileStatus.modified].contains(file.status))
.any((file) =>
file.filename.endsWith('health.yaml') ||
file.filename.endsWith('health.yml'));
}
bool healthYamlChanged(List<GitFile> files) => files
.where((file) =>
[FileStatus.added, FileStatus.modified].contains(file.status))
.any((file) =>
file.filename.endsWith('health.yaml') ||
file.filename.endsWith('health.yml'));

Future<HealthCheckResult> changelogCheck() async {
var filePaths = await packagesWithoutChangelog(
Expand Down Expand Up @@ -388,10 +374,7 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')}

Future<List<GitFile>> listFilesInPRorAll(List<Glob> ignore) async {
final files = await github.listFilesForPR(directory, ignore);
if (healthYamlChanged(files)) {
return await _getAllFiles(ignore);
}
return files;
return healthYamlChanged(files) ? await _getAllFiles(ignore) : files;
}

Future<List<GitFile>> _getAllFiles(List<Glob> ignored) async =>
Expand All @@ -413,7 +396,7 @@ ${filesWithDNS.map((e) => e.filename).map((e) => '|$e|').join('\n')}
);

var files = await listFilesInPRorAll(ignoredPackages);
var coverageResult = await coverage.compareCoverages(files, directory);
var coverageResult = coverage.compareCoveragesFor(files, directory);

var markdownResult = '''
| File | Coverage |
Expand Down
4 changes: 3 additions & 1 deletion pkgs/firehose/test/health_test.dart
Original file line number Diff line number Diff line change
Expand Up @@ -62,8 +62,10 @@ Future<void> main() async {
),
timeout: const Timeout(Duration(minutes: 2)),
);

test(
'Check health workflow "${check.name}" against golden files',
'Check health workflow "${check.name}" against golden files '
'with health.yaml changed itself',
() async => await checkGolden(
check,
fakeGithubApi([
Expand Down

0 comments on commit 75666ad

Please sign in to comment.