From 3e88d03a166069b0a2687d131b15c72dde126105 Mon Sep 17 00:00:00 2001 From: stuartmorgan Date: Tue, 25 Apr 2023 19:11:12 -0700 Subject: [PATCH] [tool] Move Android lint checks (#3816) Moves the checks for Android warning configuration from `lint-android`, where it made sense to put them at the time, to the new `check-gradle`, which is a newer command specifically for validating that our Gradle files are following best practices. Makes minor changes to the Pigeon platform test projects to make them conform to the checks, since they are now included in the run. The changes shouldn't actually change the behavior of the Pigeon tests. --- .../android/build.gradle | 4 - .../example/android/build.gradle | 8 + .../test_plugin/android/build.gradle | 6 + .../test_plugin/example/android/build.gradle | 8 + .../lib/src/common/repository_package.dart | 15 + script/tool/lib/src/gradle_check_command.dart | 96 +++++- script/tool/lib/src/lint_android_command.dart | 72 +---- .../tool/test/gradle_check_command_test.dart | 273 ++++++++++++++++-- .../tool/test/lint_android_command_test.dart | 141 +-------- 9 files changed, 370 insertions(+), 253 deletions(-) diff --git a/packages/pigeon/platform_tests/alternate_language_test_plugin/android/build.gradle b/packages/pigeon/platform_tests/alternate_language_test_plugin/android/build.gradle index fd7668d01b..8f4a155fd6 100644 --- a/packages/pigeon/platform_tests/alternate_language_test_plugin/android/build.gradle +++ b/packages/pigeon/platform_tests/alternate_language_test_plugin/android/build.gradle @@ -57,7 +57,3 @@ android { testImplementation "org.mockito:mockito-core:5.1.1" } } - -project.getTasks().withType(JavaCompile){ - options.compilerArgs.addAll(["-Xlint:all", "-Werror"]) -} diff --git a/packages/pigeon/platform_tests/alternate_language_test_plugin/example/android/build.gradle b/packages/pigeon/platform_tests/alternate_language_test_plugin/example/android/build.gradle index b68b7af5f8..8dfc50fd5f 100644 --- a/packages/pigeon/platform_tests/alternate_language_test_plugin/example/android/build.gradle +++ b/packages/pigeon/platform_tests/alternate_language_test_plugin/example/android/build.gradle @@ -29,3 +29,11 @@ subprojects { task clean(type: Delete) { delete rootProject.buildDir } + +gradle.projectsEvaluated { + project(":alternate_language_test_plugin") { + tasks.withType(JavaCompile) { + options.compilerArgs << "-Xlint:all" << "-Werror" + } + } +} diff --git a/packages/pigeon/platform_tests/test_plugin/android/build.gradle b/packages/pigeon/platform_tests/test_plugin/android/build.gradle index cb1f555d38..6ee629bf6c 100644 --- a/packages/pigeon/platform_tests/test_plugin/android/build.gradle +++ b/packages/pigeon/platform_tests/test_plugin/android/build.gradle @@ -58,6 +58,12 @@ android { } } + lintOptions { + checkAllWarnings true + warningsAsErrors true + disable 'AndroidGradlePluginVersion', 'InvalidPackage', 'GradleDependency' + } + dependencies { testImplementation 'junit:junit:4.+' testImplementation "io.mockk:mockk:1.13.5" diff --git a/packages/pigeon/platform_tests/test_plugin/example/android/build.gradle b/packages/pigeon/platform_tests/test_plugin/example/android/build.gradle index b68b7af5f8..7f97810454 100644 --- a/packages/pigeon/platform_tests/test_plugin/example/android/build.gradle +++ b/packages/pigeon/platform_tests/test_plugin/example/android/build.gradle @@ -29,3 +29,11 @@ subprojects { task clean(type: Delete) { delete rootProject.buildDir } + +gradle.projectsEvaluated { + project(":test_plugin") { + tasks.withType(JavaCompile) { + options.compilerArgs << "-Xlint:all" << "-Werror" + } + } +} diff --git a/script/tool/lib/src/common/repository_package.dart b/script/tool/lib/src/common/repository_package.dart index 5f448d36d7..2a824f54a2 100644 --- a/script/tool/lib/src/common/repository_package.dart +++ b/script/tool/lib/src/common/repository_package.dart @@ -151,4 +151,19 @@ class RepositoryPackage { .map((FileSystemEntity entity) => RepositoryPackage(entity as Directory)); } + + /// Returns the package that this package is a part of, if any. + /// + /// Currently this is limited to checking up two directories, since that + /// covers all the example structures currently used. + RepositoryPackage? getEnclosingPackage() { + final Directory parent = directory.parent; + if (isPackage(parent)) { + return RepositoryPackage(parent); + } + if (isPackage(parent.parent)) { + return RepositoryPackage(parent.parent); + } + return null; + } } diff --git a/script/tool/lib/src/gradle_check_command.dart b/script/tool/lib/src/gradle_check_command.dart index 75263b2023..6bce2927c8 100644 --- a/script/tool/lib/src/gradle_check_command.dart +++ b/script/tool/lib/src/gradle_check_command.dart @@ -6,6 +6,7 @@ import 'package:file/file.dart'; import 'common/core.dart'; import 'common/package_looping_command.dart'; +import 'common/plugin_utils.dart'; import 'common/repository_package.dart'; /// A command to enforce gradle file conventions and best practices. @@ -84,6 +85,8 @@ class GradleCheckCommand extends PackageLoopingCommand { .childFile('AndroidManifest.xml'); } + bool _isCommented(String line) => line.trim().startsWith('//'); + /// Validates the build.gradle file for a plugin /// (some_plugin/android/build.gradle). bool _validatePluginBuildGradle(RepositoryPackage package, File gradleFile) { @@ -101,6 +104,9 @@ class GradleCheckCommand extends PackageLoopingCommand { if (!_validateSourceCompatibilityVersion(lines)) { succeeded = false; } + if (!_validateGradleDrivenLintConfig(package, lines)) { + succeeded = false; + } return succeeded; } @@ -110,9 +116,16 @@ class GradleCheckCommand extends PackageLoopingCommand { RepositoryPackage package, File gradleFile) { print('${indentation}Validating ' '${getRelativePosixPath(gradleFile, from: package.directory)}.'); - // TODO(stuartmorgan): Move the -Xlint validation from lint_android_command - // to here. - return true; + final String contents = gradleFile.readAsStringSync(); + final List lines = contents.split('\n'); + + // This is tracked as a variable rather than a sequence of &&s so that all + // failures are reported at once, not just the first one. + bool succeeded = true; + if (!_validateJavacLintConfig(package, lines)) { + succeeded = false; + } + return succeeded; } /// Validates the app-level build.gradle for an example app (e.g., @@ -193,11 +206,9 @@ build.gradle "namespace" must match the "package" attribute in AndroidManifest.x /// lead to compile errors that show up for clients, but not in CI). bool _validateSourceCompatibilityVersion(List gradleLines) { if (!gradleLines.any((String line) => - line.contains('languageVersion') && - !line.trim().startsWith('//')) && + line.contains('languageVersion') && !_isCommented(line)) && !gradleLines.any((String line) => - line.contains('sourceCompatibility') && - !line.trim().startsWith('//'))) { + line.contains('sourceCompatibility') && !_isCommented(line))) { const String errorMessage = ''' build.gradle must set an explicit Java compatibility version. @@ -225,4 +236,75 @@ for more details.'''; } return true; } + + /// Returns whether the given gradle content is configured to enable all + /// Gradle-driven lints (those checked by ./gradlew lint) and treat them as + /// errors. + bool _validateGradleDrivenLintConfig( + RepositoryPackage package, List gradleLines) { + final List gradleBuildContents = package + .platformDirectory(FlutterPlatform.android) + .childFile('build.gradle') + .readAsLinesSync(); + if (!gradleBuildContents.any((String line) => + line.contains('checkAllWarnings true') && !_isCommented(line)) || + !gradleBuildContents.any((String line) => + line.contains('warningsAsErrors true') && !_isCommented(line))) { + printError('${indentation}This package is not configured to enable all ' + 'Gradle-driven lint warnings and treat them as errors. ' + 'Please add the following to the lintOptions section of ' + 'android/build.gradle:'); + print(''' + checkAllWarnings true + warningsAsErrors true +'''); + return false; + } + return true; + } + + /// Validates whether the given [example]'s gradle content is configured to + /// build its plugin target with javac lints enabled and treated as errors, + /// if the enclosing package is a plugin. + /// + /// This can only be called on example packages. (Plugin packages should not + /// be configured this way, since it would affect clients.) + /// + /// If [example]'s enclosing package is not a plugin package, this just + /// returns true. + bool _validateJavacLintConfig( + RepositoryPackage example, List gradleLines) { + final RepositoryPackage enclosingPackage = example.getEnclosingPackage()!; + if (!pluginSupportsPlatform(platformAndroid, enclosingPackage, + requiredMode: PlatformSupport.inline)) { + return true; + } + final String enclosingPackageName = enclosingPackage.directory.basename; + + // The check here is intentionally somewhat loose, to allow for the + // possibility of variations (e.g., not using Xlint:all in some cases, or + // passing other arguments). + if (!(gradleLines.any((String line) => + line.contains('project(":$enclosingPackageName")')) && + gradleLines.any((String line) => + line.contains('options.compilerArgs') && + line.contains('-Xlint') && + line.contains('-Werror')))) { + printError('The example ' + '"${getRelativePosixPath(example.directory, from: enclosingPackage.directory)}" ' + 'is not configured to treat javac lints and warnings as errors. ' + 'Please add the following to its build.gradle:'); + print(''' +gradle.projectsEvaluated { + project(":$enclosingPackageName") { + tasks.withType(JavaCompile) { + options.compilerArgs << "-Xlint:all" << "-Werror" + } + } +} +'''); + return false; + } + return true; + } } diff --git a/script/tool/lib/src/lint_android_command.dart b/script/tool/lib/src/lint_android_command.dart index 3fe25eec28..f229fd7469 100644 --- a/script/tool/lib/src/lint_android_command.dart +++ b/script/tool/lib/src/lint_android_command.dart @@ -38,22 +38,6 @@ class LintAndroidCommand extends PackageLoopingCommand { 'Plugin does not have an Android implementation.'); } - bool failed = false; - - // Ensure that the plugin has a strict Gradle-driven lint configuration, so - // that this test actually catches most issues. - if (!_mainGradleHasLintConfig(package)) { - failed = true; - printError('This plugin is not configured to enable all Gradle-driven ' - 'lint warnings and treat them as errors. ' - 'Please add the following to the lintOptions section of ' - 'android/build.gradle:'); - print(''' - checkAllWarnings true - warningsAsErrors true -'''); - } - for (final RepositoryPackage example in package.getExamples()) { final GradleProject project = GradleProject(example, processRunner: processRunner, platform: platform); @@ -75,62 +59,10 @@ class LintAndroidCommand extends PackageLoopingCommand { // inline, and the rest have to be checked via the CI-uploaded artifact. final int exitCode = await project.runCommand('$packageName:lintDebug'); if (exitCode != 0) { - failed = true; - } - - // In addition to running the Gradle-driven lint step, also ensure that - // the example project is configured to build with javac lints enabled and - // treated as errors. - if (!_exampleGradleHasJavacLintConfig(example, packageName)) { - failed = true; - printError('The example ' - '"${getRelativePosixPath(example.directory, from: package.directory)}" ' - 'is not configured to treat javac lints and warnings as errors. ' - 'Please add the following to its build.gradle:'); - print(''' -gradle.projectsEvaluated { - project(":${package.directory.basename}") { - tasks.withType(JavaCompile) { - options.compilerArgs << "-Xlint:all" << "-Werror" - } - } -} -'''); + return PackageResult.fail(); } } - return failed ? PackageResult.fail() : PackageResult.success(); - } - - /// Returns whether the plugin project is configured to enable all Gradle - /// lints and treat them as errors. - bool _mainGradleHasLintConfig(RepositoryPackage package) { - final List gradleBuildContents = package - .platformDirectory(FlutterPlatform.android) - .childFile('build.gradle') - .readAsLinesSync(); - return gradleBuildContents - .any((String line) => line.contains('checkAllWarnings true')) && - gradleBuildContents - .any((String line) => line.contains('warningsAsErrors true')); - } - - /// Returns whether the example project is configured to build with javac - /// lints enabled and treated as errors. - bool _exampleGradleHasJavacLintConfig( - RepositoryPackage example, String pluginPackageName) { - final List gradleBuildContents = example - .platformDirectory(FlutterPlatform.android) - .childFile('build.gradle') - .readAsLinesSync(); - // The check here is intentionally somewhat loose, to allow for the - // possibility of variations (e.g., not using Xlint:all in some cases, or - // passing other arguments). - return gradleBuildContents.any( - (String line) => line.contains('project(":$pluginPackageName")')) && - gradleBuildContents.any((String line) => - line.contains('options.compilerArgs') && - line.contains('-Xlint') && - line.contains('-Werror')); + return PackageResult.success(); } } diff --git a/script/tool/test/gradle_check_command_test.dart b/script/tool/test/gradle_check_command_test.dart index a3a482d762..ca373093db 100644 --- a/script/tool/test/gradle_check_command_test.dart +++ b/script/tool/test/gradle_check_command_test.dart @@ -6,6 +6,7 @@ import 'package:args/command_runner.dart'; import 'package:file/file.dart'; import 'package:file/memory.dart'; import 'package:flutter_plugin_tools/src/common/core.dart'; +import 'package:flutter_plugin_tools/src/common/plugin_utils.dart'; import 'package:flutter_plugin_tools/src/gradle_check_command.dart'; import 'package:test/test.dart'; @@ -31,25 +32,28 @@ void main() { runner.addCommand(command); }); - void writeFakeBuildGradle( + /// Writes a fake android/build.gradle file for plugin [package] with the + /// given options. + void writeFakePluginBuildGradle( RepositoryPackage package, { - bool isApp = false, bool includeLanguageVersion = false, bool includeSourceCompat = false, bool commentSourceLanguage = false, bool includeNamespace = true, bool commentNamespace = false, + bool warningsConfigured = true, }) { - final Directory androidDir = - package.platformDirectory(FlutterPlatform.android); - final Directory parentDir = - isApp ? androidDir.childDirectory('app') : androidDir; - final File buildGradle = parentDir.childFile('build.gradle'); + final File buildGradle = package + .platformDirectory(FlutterPlatform.android) + .childFile('build.gradle'); buildGradle.createSync(recursive: true); - final String compileOptionsSection = ''' - compileOptions { - ${commentSourceLanguage ? '// ' : ''}sourceCompatibility JavaVersion.VERSION_1_8 + const String warningConfig = ''' + lintOptions { + checkAllWarnings true + warningsAsErrors true + disable 'AndroidGradlePluginVersion', 'InvalidPackage', 'GradleDependency' + baseline file("lint-baseline.xml") } '''; final String javaSection = ''' @@ -59,6 +63,11 @@ java { } } +'''; + final String compileOptionsSection = ''' + compileOptions { + ${commentSourceLanguage ? '// ' : ''}sourceCompatibility JavaVersion.VERSION_1_8 + } '''; final String namespace = "${commentNamespace ? '// ' : ''}namespace '$_defaultFakeNamespace'"; @@ -84,13 +93,11 @@ android { defaultConfig { minSdkVersion 30 } - lintOptions { - checkAllWarnings true - } +${warningsConfigured ? warningConfig : ''} +${includeSourceCompat ? compileOptionsSection : ''} testOptions { unitTests.includeAndroidResources = true } -${includeSourceCompat ? compileOptionsSection : ''} } dependencies { @@ -99,6 +106,124 @@ dependencies { '''); } + /// Writes a fake android/build.gradle file for an example [package] with the + /// given options. + void writeFakeExampleTopLevelBuildGradle( + RepositoryPackage package, { + required String pluginName, + required bool warningsConfigured, + }) { + final File buildGradle = package + .platformDirectory(FlutterPlatform.android) + .childFile('build.gradle'); + buildGradle.createSync(recursive: true); + + final String warningConfig = ''' +gradle.projectsEvaluated { + project(":$pluginName") { + tasks.withType(JavaCompile) { + options.compilerArgs << "-Xlint:all" << "-Werror" + } + } +} +'''; + buildGradle.writeAsStringSync(''' +buildscript { + repositories { + google() + mavenCentral() + } + + dependencies { + classpath 'fake.package:fake:1.0.0' + } +} + +allprojects { + repositories { + google() + mavenCentral() + } +} + +rootProject.buildDir = '../build' +subprojects { + project.buildDir = "\${rootProject.buildDir}/\${project.name}" +} +subprojects { + project.evaluationDependsOn(':app') +} + +task clean(type: Delete) { + delete rootProject.buildDir +} + +${warningsConfigured ? warningConfig : ''} +'''); + } + + /// Writes a fake android/app/build.gradle file for an example [package] with + /// the given options. + void writeFakeExampleAppBuildGradle( + RepositoryPackage package, { + required bool includeNamespace, + required bool commentNamespace, + }) { + final File buildGradle = package + .platformDirectory(FlutterPlatform.android) + .childDirectory('app') + .childFile('build.gradle'); + buildGradle.createSync(recursive: true); + + final String namespace = + "${commentNamespace ? '// ' : ''}namespace '$_defaultFakeNamespace'"; + buildGradle.writeAsStringSync(''' +def flutterRoot = localProperties.getProperty('flutter.sdk') +if (flutterRoot == null) { + throw new GradleException("Flutter SDK not found. Define location with flutter.sdk in the local.properties file.") +} + +apply plugin: 'com.android.application' +apply from: "\$flutterRoot/packages/flutter_tools/gradle/flutter.gradle" + +android { + ${includeNamespace ? namespace : ''} + compileSdkVersion flutter.compileSdkVersion + + lintOptions { + disable 'InvalidPackage' + } + + defaultConfig { + applicationId "io.flutter.plugins.cameraexample" + minSdkVersion 21 + targetSdkVersion 28 + } +} + +flutter { + source '../..' +} + +dependencies { + testImplementation 'fake.package:fake:1.0.0' +} +'''); + } + + void writeFakeExampleBuildGradles( + RepositoryPackage package, { + required String pluginName, + bool includeNamespace = true, + bool commentNamespace = false, + bool warningsConfigured = true, + }) { + writeFakeExampleTopLevelBuildGradle(package, + pluginName: pluginName, warningsConfigured: warningsConfigured); + writeFakeExampleAppBuildGradle(package, + includeNamespace: includeNamespace, commentNamespace: commentNamespace); + } + void writeFakeManifest( RepositoryPackage package, { bool isApp = false, @@ -136,7 +261,7 @@ dependencies { test('fails when build.gradle has no java compatibility version', () async { final RepositoryPackage package = createFakePlugin('a_plugin', packagesDir, examples: []); - writeFakeBuildGradle(package); + writeFakePluginBuildGradle(package); writeFakeManifest(package); Error? commandError; @@ -158,7 +283,7 @@ dependencies { test('passes when sourceCompatibility is specified', () async { final RepositoryPackage package = createFakePlugin('a_plugin', packagesDir, examples: []); - writeFakeBuildGradle(package, includeSourceCompat: true); + writeFakePluginBuildGradle(package, includeSourceCompat: true); writeFakeManifest(package); final List output = @@ -175,7 +300,7 @@ dependencies { test('passes when toolchain languageVersion is specified', () async { final RepositoryPackage package = createFakePlugin('a_plugin', packagesDir, examples: []); - writeFakeBuildGradle(package, includeLanguageVersion: true); + writeFakePluginBuildGradle(package, includeLanguageVersion: true); writeFakeManifest(package); final List output = @@ -190,11 +315,12 @@ dependencies { }); test('does not require java version in examples', () async { - final RepositoryPackage package = createFakePlugin('a_plugin', packagesDir); - writeFakeBuildGradle(package, includeLanguageVersion: true); + const String pluginName = 'a_plugin'; + final RepositoryPackage package = createFakePlugin(pluginName, packagesDir); + writeFakePluginBuildGradle(package, includeLanguageVersion: true); writeFakeManifest(package); final RepositoryPackage example = package.getExamples().first; - writeFakeBuildGradle(example, isApp: true); + writeFakeExampleBuildGradles(example, pluginName: pluginName); writeFakeManifest(example, isApp: true); final List output = @@ -212,7 +338,7 @@ dependencies { test('fails when java compatibility version is commented out', () async { final RepositoryPackage package = createFakePlugin('a_plugin', packagesDir, examples: []); - writeFakeBuildGradle(package, + writeFakePluginBuildGradle(package, includeSourceCompat: true, commentSourceLanguage: true); writeFakeManifest(package); @@ -235,7 +361,7 @@ dependencies { test('fails when languageVersion is commented out', () async { final RepositoryPackage package = createFakePlugin('a_plugin', packagesDir, examples: []); - writeFakeBuildGradle(package, + writeFakePluginBuildGradle(package, includeLanguageVersion: true, commentSourceLanguage: true); writeFakeManifest(package); @@ -259,7 +385,7 @@ dependencies { () async { final RepositoryPackage package = createFakePlugin('a_plugin', packagesDir, examples: []); - writeFakeBuildGradle(package, includeLanguageVersion: true); + writeFakePluginBuildGradle(package, includeLanguageVersion: true); writeFakeManifest(package, packageName: 'wrong.package.name'); Error? commandError; @@ -281,7 +407,7 @@ dependencies { test('fails when namespace is missing', () async { final RepositoryPackage package = createFakePlugin('a_plugin', packagesDir, examples: []); - writeFakeBuildGradle(package, + writeFakePluginBuildGradle(package, includeLanguageVersion: true, includeNamespace: false); writeFakeManifest(package); @@ -301,12 +427,13 @@ dependencies { }); test('fails when namespace is missing from example', () async { - final RepositoryPackage package = createFakePlugin('a_plugin', packagesDir); - writeFakeBuildGradle(package, includeLanguageVersion: true); + const String pluginName = 'a_plugin'; + final RepositoryPackage package = createFakePlugin(pluginName, packagesDir); + writeFakePluginBuildGradle(package, includeLanguageVersion: true); writeFakeManifest(package); final RepositoryPackage example = package.getExamples().first; - writeFakeBuildGradle(example, - isApp: true, includeLanguageVersion: true, includeNamespace: false); + writeFakeExampleBuildGradles(example, + pluginName: pluginName, includeNamespace: false); writeFakeManifest(example, isApp: true); Error? commandError; @@ -330,11 +457,12 @@ dependencies { // don't just accidentally diverge. test('fails when namespace in example does not match AndroidManifest.xml', () async { - final RepositoryPackage package = createFakePlugin('a_plugin', packagesDir); - writeFakeBuildGradle(package, includeLanguageVersion: true); + const String pluginName = 'a_plugin'; + final RepositoryPackage package = createFakePlugin(pluginName, packagesDir); + writeFakePluginBuildGradle(package, includeLanguageVersion: true); writeFakeManifest(package); final RepositoryPackage example = package.getExamples().first; - writeFakeBuildGradle(example, isApp: true, includeLanguageVersion: true); + writeFakeExampleBuildGradles(example, pluginName: pluginName); writeFakeManifest(example, isApp: true, packageName: 'wrong.package.name'); Error? commandError; @@ -356,7 +484,7 @@ dependencies { test('fails when namespace is commented out', () async { final RepositoryPackage package = createFakePlugin('a_plugin', packagesDir, examples: []); - writeFakeBuildGradle(package, + writeFakePluginBuildGradle(package, includeLanguageVersion: true, commentNamespace: true); writeFakeManifest(package); @@ -374,4 +502,85 @@ dependencies { ]), ); }); + + test('fails if gradle-driven lint-warnings-as-errors is missing', () async { + const String pluginName = 'a_plugin'; + final RepositoryPackage plugin = + createFakePlugin(pluginName, packagesDir, examples: []); + writeFakePluginBuildGradle(plugin, + includeLanguageVersion: true, warningsConfigured: false); + writeFakeManifest(plugin); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['gradle-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder( + [ + contains('This package is not configured to enable all ' + 'Gradle-driven lint warnings and treat them as errors.'), + contains('The following packages had errors:'), + ], + )); + }); + + test('fails if plugin example javac lint-warnings-as-errors is missing', + () async { + const String pluginName = 'a_plugin'; + final RepositoryPackage plugin = createFakePlugin(pluginName, packagesDir, + platformSupport: { + platformAndroid: const PlatformDetails(PlatformSupport.inline), + }); + writeFakePluginBuildGradle(plugin, includeLanguageVersion: true); + writeFakeManifest(plugin); + final RepositoryPackage example = plugin.getExamples().first; + writeFakeExampleBuildGradles(example, + pluginName: pluginName, warningsConfigured: false); + writeFakeManifest(example, isApp: true); + + Error? commandError; + final List output = await runCapturingPrint( + runner, ['gradle-check'], errorHandler: (Error e) { + commandError = e; + }); + + expect(commandError, isA()); + expect( + output, + containsAllInOrder( + [ + contains('The example "example" is not configured to treat javac ' + 'lints and warnings as errors.'), + contains('The following packages had errors:'), + ], + )); + }); + + test( + 'passes if non-plugin package example javac lint-warnings-as-errors is missing', + () async { + const String packageName = 'a_package'; + final RepositoryPackage plugin = + createFakePackage(packageName, packagesDir); + final RepositoryPackage example = plugin.getExamples().first; + writeFakeExampleBuildGradles(example, + pluginName: packageName, warningsConfigured: false); + writeFakeManifest(example, isApp: true); + + final List output = + await runCapturingPrint(runner, ['gradle-check']); + + expect( + output, + containsAllInOrder( + [ + contains('Validating android/build.gradle'), + ], + )); + }); } diff --git a/script/tool/test/lint_android_command_test.dart b/script/tool/test/lint_android_command_test.dart index 614e60d105..fda3ee41ce 100644 --- a/script/tool/test/lint_android_command_test.dart +++ b/script/tool/test/lint_android_command_test.dart @@ -37,79 +37,6 @@ void main() { runner.addCommand(command); }); - void writeFakePluginBuildGradle(RepositoryPackage plugin, - {bool warningsConfigured = true}) { - const String warningConfig = ''' - lintOptions { - checkAllWarnings true - warningsAsErrors true - disable 'AndroidGradlePluginVersion', 'InvalidPackage', 'GradleDependency' - baseline file("lint-baseline.xml") - } -'''; - final File gradleFile = plugin - .platformDirectory(FlutterPlatform.android) - .childFile('build.gradle'); - gradleFile.createSync(recursive: true); - gradleFile.writeAsStringSync(''' -android { - compileSdkVersion 33 - - defaultConfig { - minSdkVersion 16 - testInstrumentationRunner "androidx.test.runner.AndroidJUnitRunner" - } -${warningsConfigured ? warningConfig : ''} - testOptions { - unitTests.returnDefaultValues = true - unitTests.all { - testLogging { - events "passed", "skipped", "failed", "standardOut", "standardError" - outputs.upToDateWhen {false} - showStandardStreams = true - } - } - } -} - -'''); - } - - void writeFakeExampleBuildGradle( - RepositoryPackage example, String pluginName, - {bool warningsConfigured = true}) { - final String warningConfig = ''' -gradle.projectsEvaluated { - project(":$pluginName") { - tasks.withType(JavaCompile) { - options.compilerArgs << "-Xlint:all" << "-Werror" - } - } -} -'''; - example - .platformDirectory(FlutterPlatform.android) - .childFile('build.gradle') - .writeAsStringSync(''' -buildscript { - repositories { - google() - mavenCentral() - } - dependencies { - classpath 'com.android.tools.build:gradle:8.0.1' - } -} -allprojects { - repositories { - google() - mavenCentral() - } -} -${warningsConfigured ? warningConfig : ''} -'''); - } - test('runs gradle lint', () async { final RepositoryPackage plugin = createFakePlugin('plugin1', packagesDir, extraFiles: [ @@ -117,8 +44,6 @@ ${warningsConfigured ? warningConfig : ''} ], platformSupport: { platformAndroid: const PlatformDetails(PlatformSupport.inline) }); - writeFakePluginBuildGradle(plugin); - writeFakeExampleBuildGradle(plugin.getExamples().first, 'plugin1'); final Directory androidDir = plugin.getExamples().first.platformDirectory(FlutterPlatform.android); @@ -156,10 +81,6 @@ ${warningsConfigured ? warningConfig : ''} platformSupport: { platformAndroid: const PlatformDetails(PlatformSupport.inline) }); - writeFakePluginBuildGradle(plugin); - for (final RepositoryPackage example in plugin.getExamples()) { - writeFakeExampleBuildGradle(example, 'plugin1'); - } final Iterable exampleAndroidDirs = plugin.getExamples().map( (RepositoryPackage example) => @@ -189,11 +110,10 @@ ${warningsConfigured ? warningConfig : ''} }); test('fails if gradlew is missing', () async { - final RepositoryPackage plugin = createFakePlugin('plugin1', packagesDir, + createFakePlugin('plugin1', packagesDir, platformSupport: { platformAndroid: const PlatformDetails(PlatformSupport.inline) }); - writeFakePluginBuildGradle(plugin); Error? commandError; final List output = await runCapturingPrint( @@ -218,8 +138,6 @@ ${warningsConfigured ? warningConfig : ''} ], platformSupport: { platformAndroid: const PlatformDetails(PlatformSupport.inline) }); - writeFakePluginBuildGradle(plugin); - writeFakeExampleBuildGradle(plugin.getExamples().first, 'plugin1'); final String gradlewPath = plugin .getExamples() @@ -247,63 +165,6 @@ ${warningsConfigured ? warningConfig : ''} )); }); - test('fails if gradle-driven lint-warnings-as-errors is missing', () async { - final RepositoryPackage plugin = - createFakePlugin('plugin1', packagesDir, extraFiles: [ - 'example/android/gradlew', - ], platformSupport: { - platformAndroid: const PlatformDetails(PlatformSupport.inline) - }); - writeFakePluginBuildGradle(plugin, warningsConfigured: false); - writeFakeExampleBuildGradle(plugin.getExamples().first, 'plugin1'); - - Error? commandError; - final List output = await runCapturingPrint( - runner, ['lint-android'], errorHandler: (Error e) { - commandError = e; - }); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder( - [ - contains('This plugin is not configured to enable all ' - 'Gradle-driven lint warnings and treat them as errors.'), - contains('The following packages had errors:'), - ], - )); - }); - - test('fails if javac lint-warnings-as-errors is missing', () async { - final RepositoryPackage plugin = - createFakePlugin('plugin1', packagesDir, extraFiles: [ - 'example/android/gradlew', - ], platformSupport: { - platformAndroid: const PlatformDetails(PlatformSupport.inline) - }); - writeFakePluginBuildGradle(plugin); - writeFakeExampleBuildGradle(plugin.getExamples().first, 'plugin1', - warningsConfigured: false); - - Error? commandError; - final List output = await runCapturingPrint( - runner, ['lint-android'], errorHandler: (Error e) { - commandError = e; - }); - - expect(commandError, isA()); - expect( - output, - containsAllInOrder( - [ - contains('The example "example" is not configured to treat javac ' - 'lints and warnings as errors.'), - contains('The following packages had errors:'), - ], - )); - }); - test('skips non-Android plugins', () async { createFakePlugin('plugin1', packagesDir);