[pigeon] Validate generated files in CI (flutter#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 7c47d56a6d6b5892b41977df64dab377c5cdd701.
ditman · Feb 17, 2023 · 925bea8 · 925bea8
1 parent 3094867
commit 925bea8
Show file tree

Hide file tree

Showing 3 changed files with 138 additions and 4 deletions.
diff --git a/packages/pigeon/tool/generate.dart b/packages/pigeon/tool/generate.dart
@@ -12,19 +12,47 @@
 
 import 'dart:io' show Platform, exit;
 
+import 'package:args/args.dart';
 import 'package:path/path.dart' as p;
 
 import 'shared/generation.dart';
 
+const String _helpFlag = 'help';
+const String _formatFlag = 'format';
+
 Future<void> main(List<String> args) async {
+  final ArgParser parser = ArgParser()
+    ..addFlag(_formatFlag, abbr: 'f', help: 'Autoformats after generation.')
+    ..addFlag(_helpFlag,
+        negatable: false, abbr: 'h', help: 'Print this reference.');
+
+  final ArgResults argResults = parser.parse(args);
+  if (argResults.wasParsed(_helpFlag)) {
+    print('''
+usage: dart run tool/generate.dart [options]
+
+${parser.usage}''');
+    exit(0);
+  }
+
   final String baseDir = p.dirname(p.dirname(Platform.script.toFilePath()));
 
   print('Generating platform_test/ output...');
-  final int exitCode = await generatePigeons(baseDir: baseDir);
-  if (exitCode == 0) {
+  final int generateExitCode = await generatePigeons(baseDir: baseDir);
+  if (generateExitCode == 0) {
     print('Generation complete!');
   } else {
     print('Generation failed; see above for errors.');
+    exit(generateExitCode);
+  }
+
+  if (argResults.wasParsed(_formatFlag)) {
+    print('Formatting generated output...');
+    final int formatExitCode =
+        await formatAllFiles(repositoryRoot: p.dirname(p.dirname(baseDir)));
+    if (formatExitCode != 0) {
+      print('Formatting failed; see above for errors.');
+      exit(formatExitCode);
+    }
   }
-  exit(exitCode);
 }
diff --git a/packages/pigeon/tool/run_tests.dart b/packages/pigeon/tool/run_tests.dart
@@ -9,8 +9,11 @@
 ///
 /// For any use other than CI, use test.dart instead.
 ////////////////////////////////////////////////////////////////////////////////
-import 'dart:io' show Platform, exit;
+import 'dart:io';
 
+import 'package:path/path.dart' as p;
+
+import 'shared/generation.dart';
 import 'shared/test_runner.dart';
 import 'shared/test_suites.dart';
 
@@ -29,6 +32,77 @@ void _validateTestCoverage(List<List<String>> shards) {
   }
 }
 
+Future<void> _validateGeneratedTestFiles() async {
+  final String baseDir = p.dirname(p.dirname(Platform.script.toFilePath()));
+  final String repositoryRoot = p.dirname(p.dirname(baseDir));
+  final String relativePigeonPath = p.relative(baseDir, from: repositoryRoot);
+
+  print('Validating generated files:');
+  print('  Generating output...');
+  final int generateExitCode = await generatePigeons(baseDir: baseDir);
+  if (generateExitCode != 0) {
+    print('Generation failed; see above for errors.');
+    exit(generateExitCode);
+  }
+
+  print('  Formatting output...');
+  final int formatExitCode =
+      await formatAllFiles(repositoryRoot: repositoryRoot);
+  if (formatExitCode != 0) {
+    print('Formatting failed; see above for errors.');
+    exit(formatExitCode);
+  }
+
+  print('  Checking for changes...');
+  final List<String> modifiedFiles = await _modifiedFiles(
+      repositoryRoot: repositoryRoot, relativePigeonPath: relativePigeonPath);
+
+  if (modifiedFiles.isEmpty) {
+    return;
+  }
+
+  print('The following files are not updated, or not formatted correctly:');
+  modifiedFiles.map((String line) => '  $line').forEach(print);
+
+  print('\nTo fix run "dart run tool/generate.dart --format" from the pigeon/ '
+      'directory, or apply the diff with the command below.\n');
+
+  final ProcessResult diffResult = await Process.run(
+    'git',
+    <String>['diff', relativePigeonPath],
+    workingDirectory: repositoryRoot,
+  );
+  if (diffResult.exitCode != 0) {
+    print('Unable to determine diff.');
+    exit(1);
+  }
+  print('patch -p1 <<DONE');
+  print(diffResult.stdout);
+  print('DONE');
+  exit(1);
+}
+
+Future<List<String>> _modifiedFiles(
+    {required String repositoryRoot,
+    required String relativePigeonPath}) async {
+  final ProcessResult result = await Process.run(
+    'git',
+    <String>['ls-files', '--modified', relativePigeonPath],
+    workingDirectory: repositoryRoot,
+  );
+  if (result.exitCode != 0) {
+    print('Unable to determine changed files.');
+    print(result.stdout);
+    print(result.stderr);
+    exit(1);
+  }
+  return (result.stdout as String)
+      .split('\n')
+      .map((String line) => line.trim())
+      .where((String line) => line.isNotEmpty)
+      .toList();
+}
+
 Future<void> main(List<String> args) async {
   // Run most tests on Linux, since Linux tends to be the easiest and cheapest.
   const List<String> linuxHostTests = <String>[
@@ -85,6 +159,14 @@ Future<void> main(List<String> args) async {
     ],
   ]);
 
+  // Ensure that all generated files are up to date. This is run only on Linux
+  // both to avoid duplication of work, and to avoid issues if different CI
+  // configurations have different setups (e.g., different clang-format versions
+  // or no clang-format at all).
+  if (Platform.isLinux) {
+    await _validateGeneratedTestFiles();
+  }
+
   final List<String> testsToRun;
   if (Platform.isMacOS) {
     if (Platform.environment['LUCI_CI'] != null) {

diff --git a/packages/pigeon/tool/shared/generation.dart b/packages/pigeon/tool/shared/generation.dart
@@ -2,9 +2,13 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import 'dart:io' show Platform;
+
 import 'package:path/path.dart' as p;
 import 'package:pigeon/pigeon.dart';
 
+import 'process_utils.dart';
+
 enum GeneratorLanguages {
   cpp,
   java,
@@ -166,3 +170,23 @@ Future<int> runPigeon({
     swiftOptions: const SwiftOptions(),
   ));
 }
+
+/// Runs the repository tooling's format command on this package.
+///
+/// This is intended for formatting generated autoput, but since there's no
+/// way to filter to specific files in with the repo tooling it runs over the
+/// entire package.
+Future<int> formatAllFiles({required String repositoryRoot}) {
+  final String dartCommand = Platform.isWindows ? 'dart.exe' : 'dart';
+  return runProcess(
+      dartCommand,
+      <String>[
+        'run',
+        'script/tool/bin/flutter_plugin_tools.dart',
+        'format',
+        '--packages=pigeon',
+      ],
+      streamOutput: false,
+      workingDirectory: repositoryRoot,
+      logFailure: true);
+}