Skip to content

Commit 925bea8

Browse files
stuartmorgan-gstuartmorgan-g
authored
[pigeon] Validate generated files in CI (#3224)
* Add autoformatting option to generation * Extract to helper * Add validation * Test change to demonstrate behavior * Actually fail when validation fails * Revert "Test change to demonstrate behavior" This reverts commit 7c47d56.
1 parent 3094867 commit 925bea8

File tree

3 files changed

+138
-4
lines changed

3 files changed

+138
-4
lines changed

‎packages/pigeon/tool/generate.dart‎

Lines changed: 31 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,19 +12,47 @@
1212

1313
import 'dart:io' show Platform, exit;
1414

15+
import 'package:args/args.dart';
1516
import 'package:path/path.dart' as p;
1617

1718
import 'shared/generation.dart';
1819

20+
const String _helpFlag = 'help';
21+
const String _formatFlag = 'format';
22+
1923
Future<void> main(List<String> args) async {
24+
final ArgParser parser = ArgParser()
25+
..addFlag(_formatFlag, abbr: 'f', help: 'Autoformats after generation.')
26+
..addFlag(_helpFlag,
27+
negatable: false, abbr: 'h', help: 'Print this reference.');
28+
29+
final ArgResults argResults = parser.parse(args);
30+
if (argResults.wasParsed(_helpFlag)) {
31+
print('''
32+
usage: dart run tool/generate.dart [options]
33+
34+
${parser.usage}''');
35+
exit(0);
36+
}
37+
2038
final String baseDir = p.dirname(p.dirname(Platform.script.toFilePath()));
2139

2240
print('Generating platform_test/ output...');
23-
final int exitCode = await generatePigeons(baseDir: baseDir);
24-
if (exitCode == 0) {
41+
final int generateExitCode = await generatePigeons(baseDir: baseDir);
42+
if (generateExitCode == 0) {
2543
print('Generation complete!');
2644
} else {
2745
print('Generation failed; see above for errors.');
46+
exit(generateExitCode);
47+
}
48+
49+
if (argResults.wasParsed(_formatFlag)) {
50+
print('Formatting generated output...');
51+
final int formatExitCode =
52+
await formatAllFiles(repositoryRoot: p.dirname(p.dirname(baseDir)));
53+
if (formatExitCode != 0) {
54+
print('Formatting failed; see above for errors.');
55+
exit(formatExitCode);
56+
}
2857
}
29-
exit(exitCode);
3058
}

‎packages/pigeon/tool/run_tests.dart‎

Lines changed: 83 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,11 @@
99
///
1010
/// For any use other than CI, use test.dart instead.
1111
////////////////////////////////////////////////////////////////////////////////
12-
import 'dart:io' show Platform, exit;
12+
import 'dart:io';
1313

14+
import 'package:path/path.dart' as p;
15+
16+
import 'shared/generation.dart';
1417
import 'shared/test_runner.dart';
1518
import 'shared/test_suites.dart';
1619

@@ -29,6 +32,77 @@ void _validateTestCoverage(List<List<String>> shards) {
2932
}
3033
}
3134

35+
Future<void> _validateGeneratedTestFiles() async {
36+
final String baseDir = p.dirname(p.dirname(Platform.script.toFilePath()));
37+
final String repositoryRoot = p.dirname(p.dirname(baseDir));
38+
final String relativePigeonPath = p.relative(baseDir, from: repositoryRoot);
39+
40+
print('Validating generated files:');
41+
print(' Generating output...');
42+
final int generateExitCode = await generatePigeons(baseDir: baseDir);
43+
if (generateExitCode != 0) {
44+
print('Generation failed; see above for errors.');
45+
exit(generateExitCode);
46+
}
47+
48+
print(' Formatting output...');
49+
final int formatExitCode =
50+
await formatAllFiles(repositoryRoot: repositoryRoot);
51+
if (formatExitCode != 0) {
52+
print('Formatting failed; see above for errors.');
53+
exit(formatExitCode);
54+
}
55+
56+
print(' Checking for changes...');
57+
final List<String> modifiedFiles = await _modifiedFiles(
58+
repositoryRoot: repositoryRoot, relativePigeonPath: relativePigeonPath);
59+
60+
if (modifiedFiles.isEmpty) {
61+
return;
62+
}
63+
64+
print('The following files are not updated, or not formatted correctly:');
65+
modifiedFiles.map((String line) => ' $line').forEach(print);
66+
67+
print('\nTo fix run "dart run tool/generate.dart --format" from the pigeon/ '
68+
'directory, or apply the diff with the command below.\n');
69+
70+
final ProcessResult diffResult = await Process.run(
71+
'git',
72+
<String>['diff', relativePigeonPath],
73+
workingDirectory: repositoryRoot,
74+
);
75+
if (diffResult.exitCode != 0) {
76+
print('Unable to determine diff.');
77+
exit(1);
78+
}
79+
print('patch -p1 <<DONE');
80+
print(diffResult.stdout);
81+
print('DONE');
82+
exit(1);
83+
}
84+
85+
Future<List<String>> _modifiedFiles(
86+
{required String repositoryRoot,
87+
required String relativePigeonPath}) async {
88+
final ProcessResult result = await Process.run(
89+
'git',
90+
<String>['ls-files', '--modified', relativePigeonPath],
91+
workingDirectory: repositoryRoot,
92+
);
93+
if (result.exitCode != 0) {
94+
print('Unable to determine changed files.');
95+
print(result.stdout);
96+
print(result.stderr);
97+
exit(1);
98+
}
99+
return (result.stdout as String)
100+
.split('\n')
101+
.map((String line) => line.trim())
102+
.where((String line) => line.isNotEmpty)
103+
.toList();
104+
}
105+
32106
Future<void> main(List<String> args) async {
33107
// Run most tests on Linux, since Linux tends to be the easiest and cheapest.
34108
const List<String> linuxHostTests = <String>[
@@ -85,6 +159,14 @@ Future<void> main(List<String> args) async {
85159
],
86160
]);
87161

162+
// Ensure that all generated files are up to date. This is run only on Linux
163+
// both to avoid duplication of work, and to avoid issues if different CI
164+
// configurations have different setups (e.g., different clang-format versions
165+
// or no clang-format at all).
166+
if (Platform.isLinux) {
167+
await _validateGeneratedTestFiles();
168+
}
169+
88170
final List<String> testsToRun;
89171
if (Platform.isMacOS) {
90172
if (Platform.environment['LUCI_CI'] != null) {

‎packages/pigeon/tool/shared/generation.dart‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,13 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import 'dart:io' show Platform;
6+
57
import 'package:path/path.dart' as p;
68
import 'package:pigeon/pigeon.dart';
79

10+
import 'process_utils.dart';
11+
812
enum GeneratorLanguages {
913
cpp,
1014
java,
@@ -166,3 +170,23 @@ Future<int> runPigeon({
166170
swiftOptions: const SwiftOptions(),
167171
));
168172
}
173+
174+
/// Runs the repository tooling's format command on this package.
175+
///
176+
/// This is intended for formatting generated autoput, but since there's no
177+
/// way to filter to specific files in with the repo tooling it runs over the
178+
/// entire package.
179+
Future<int> formatAllFiles({required String repositoryRoot}) {
180+
final String dartCommand = Platform.isWindows ? 'dart.exe' : 'dart';
181+
return runProcess(
182+
dartCommand,
183+
<String>[
184+
'run',
185+
'script/tool/bin/flutter_plugin_tools.dart',
186+
'format',
187+
'--packages=pigeon',
188+
],
189+
streamOutput: false,
190+
workingDirectory: repositoryRoot,
191+
logFailure: true);
192+
}

0 commit comments

Comments
 (0)