[flutter_plugin_tools] Fix pubspec-check on Windows (#4428)

The repository check always failed when run on Windows, because the relative path generated to check the end of the URL was using local filesystem style for separators, but URLs always use POSIX separators.
2025-08-06 08:53:11 +08:00 · 2021-10-25 11:29:07 -04:00
parent 5bd3ae6a0a
commit 78395e5adf
3 changed files with 258 additions and 204 deletions
--- a/script/tool/CHANGELOG.md
+++ b/script/tool/CHANGELOG.md
@ -14,6 +14,7 @@
 - `pubspec-check` now checks that the description is of the pub-recommended
  length.
 - Fix `license-check` when run on Windows with line ending conversion enabled.
+- Fixed `pubspec-check` on Windows.

 ## 0.7.1

--- a/script/tool/lib/src/pubspec_check_command.dart
+++ b/script/tool/lib/src/pubspec_check_command.dart
@ -177,7 +177,7 @@ class PubspecCheckCommand extends PackageLoopingCommand {
      errorMessages.add('Missing "repository"');
    } else {
      final String relativePackagePath =
-          path.relative(package.path, from: packagesDir.parent.path);
+          getRelativePosixPath(package.directory, from: packagesDir.parent);
      if (!pubspec.repository!.path.endsWith(relativePackagePath)) {
        errorMessages
            .add('The "repository" link should end with the package path.');
--- a/script/tool/test/pubspec_check_command_test.dart
+++ b/script/tool/test/pubspec_check_command_test.dart
@ -12,6 +12,99 @@ import 'package:test/test.dart';
 import 'mocks.dart';
 import 'util.dart';

+/// Returns the top section of a pubspec.yaml for a package named [name],
+/// for either a flutter/packages or flutter/plugins package depending on
+/// the values of [isPlugin].
+///
+/// By default it will create a header that includes all of the expected
+/// values, elements can be changed via arguments to create incorrect
+/// entries.
+///
+/// If [includeRepository] is true, by default the path in the link will
+/// be "packages/[name]"; a different "packages"-relative path can be
+/// provided with [repositoryPackagesDirRelativePath].
+String _headerSection(
+  String name, {
+  bool isPlugin = false,
+  bool includeRepository = true,
+  String? repositoryPackagesDirRelativePath,
+  bool includeHomepage = false,
+  bool includeIssueTracker = true,
+  bool publishable = true,
+  String? description,
+}) {
+  final String repositoryPath = repositoryPackagesDirRelativePath ?? name;
+  final List<String> repoLinkPathComponents = <String>[
+    'flutter',
+    if (isPlugin) 'plugins' else 'packages',
+    'tree',
+    'master',
+    'packages',
+    repositoryPath,
+  ];
+  final String repoLink =
+      'https://github.com/' + repoLinkPathComponents.join('/');
+  final String issueTrackerLink = 'https://github.com/flutter/flutter/issues?'
+      'q=is%3Aissue+is%3Aopen+label%3A%22p%3A+$name%22';
+  description ??= 'A test package for validating that the pubspec.yaml '
+      'follows repo best practices.';
+  return '''
+name: $name
+description: $description
+${includeRepository ? 'repository: $repoLink' : ''}
+${includeHomepage ? 'homepage: $repoLink' : ''}
+${includeIssueTracker ? 'issue_tracker: $issueTrackerLink' : ''}
+version: 1.0.0
+${publishable ? '' : 'publish_to: \'none\''}
+''';
+}
+
+String _environmentSection() {
+  return '''
+environment:
+  sdk: ">=2.12.0 <3.0.0"
+  flutter: ">=2.0.0"
+''';
+}
+
+String _flutterSection({
+  bool isPlugin = false,
+  String? implementedPackage,
+}) {
+  final String pluginEntry = '''
+  plugin:
+${implementedPackage == null ? '' : '    implements: $implementedPackage'}
+    platforms:
+''';
+  return '''
+flutter:
+${isPlugin ? pluginEntry : ''}
+''';
+}
+
+String _dependenciesSection() {
+  return '''
+dependencies:
+  flutter:
+    sdk: flutter
+''';
+}
+
+String _devDependenciesSection() {
+  return '''
+dev_dependencies:
+  flutter_test:
+    sdk: flutter
+''';
+}
+
+String _falseSecretsSection() {
+  return '''
+false_secrets:
+  - /lib/main.dart
+''';
+}
+
 void main() {
  group('test pubspec_check_command', () {
    late CommandRunner<void> runner;
@ -37,103 +130,16 @@ void main() {
      runner.addCommand(command);
    });

-    /// Returns the top section of a pubspec.yaml for a package named [name],
-    /// for either a flutter/packages or flutter/plugins package depending on
-    /// the values of [isPlugin].
-    ///
-    /// By default it will create a header that includes all of the expected
-    /// values, elements can be changed via arguments to create incorrect
-    /// entries.
-    ///
-    /// If [includeRepository] is true, by default the path in the link will
-    /// be "packages/[name]"; a different "packages"-relative path can be
-    /// provided with [repositoryPackagesDirRelativePath].
-    String headerSection(
-      String name, {
-      bool isPlugin = false,
-      bool includeRepository = true,
-      String? repositoryPackagesDirRelativePath,
-      bool includeHomepage = false,
-      bool includeIssueTracker = true,
-      bool publishable = true,
-      String? description,
-    }) {
-      final String repositoryPath = repositoryPackagesDirRelativePath ?? name;
-      final String repoLink = 'https://github.com/flutter/'
-          '${isPlugin ? 'plugins' : 'packages'}/tree/master/'
-          'packages/$repositoryPath';
-      final String issueTrackerLink =
-          'https://github.com/flutter/flutter/issues?'
-          'q=is%3Aissue+is%3Aopen+label%3A%22p%3A+$name%22';
-      description ??= 'A test package for validating that the pubspec.yaml '
-          'follows repo best practices.';
-      return '''
-name: $name
-description: $description
-${includeRepository ? 'repository: $repoLink' : ''}
-${includeHomepage ? 'homepage: $repoLink' : ''}
-${includeIssueTracker ? 'issue_tracker: $issueTrackerLink' : ''}
-version: 1.0.0
-${publishable ? '' : 'publish_to: \'none\''}
-''';
-    }
-
-    String environmentSection() {
-      return '''
-environment:
-  sdk: ">=2.12.0 <3.0.0"
-  flutter: ">=2.0.0"
-''';
-    }
-
-    String flutterSection({
-      bool isPlugin = false,
-      String? implementedPackage,
-    }) {
-      final String pluginEntry = '''
-  plugin:
-${implementedPackage == null ? '' : '    implements: $implementedPackage'}
-    platforms:
-''';
-      return '''
-flutter:
-${isPlugin ? pluginEntry : ''}
-''';
-    }
-
-    String dependenciesSection() {
-      return '''
-dependencies:
-  flutter:
-    sdk: flutter
-''';
-    }
-
-    String devDependenciesSection() {
-      return '''
-dev_dependencies:
-  flutter_test:
-    sdk: flutter
-''';
-    }
-
-    String falseSecretsSection() {
-      return '''
-false_secrets:
-  - /lib/main.dart
-''';
-    }
-
    test('passes for a plugin following conventions', () async {
      final Directory pluginDirectory = createFakePlugin('plugin', packagesDir);

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin', isPlugin: true)}
-${environmentSection()}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
-${devDependenciesSection()}
-${falseSecretsSection()}
+${_headerSection('plugin', isPlugin: true)}
+${_environmentSection()}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
+${_devDependenciesSection()}
+${_falseSecretsSection()}
 ''');

      final List<String> output = await runCapturingPrint(runner, <String>[
@ -154,12 +160,12 @@ ${falseSecretsSection()}
      final Directory pluginDirectory = createFakePlugin('plugin', packagesDir);

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin')}
-${environmentSection()}
-${dependenciesSection()}
-${devDependenciesSection()}
-${flutterSection()}
-${falseSecretsSection()}
+${_headerSection('plugin')}
+${_environmentSection()}
+${_dependenciesSection()}
+${_devDependenciesSection()}
+${_flutterSection()}
+${_falseSecretsSection()}
 ''');

      final List<String> output = await runCapturingPrint(runner, <String>[
@ -181,9 +187,9 @@ ${falseSecretsSection()}
      packageDirectory.createSync(recursive: true);

      packageDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('package')}
-${environmentSection()}
-${dependenciesSection()}
+${_headerSection('package')}
+${_environmentSection()}
+${_dependenciesSection()}
 ''');

      final List<String> output = await runCapturingPrint(runner, <String>[
@ -203,11 +209,11 @@ ${dependenciesSection()}
      final Directory pluginDirectory = createFakePlugin('plugin', packagesDir);

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin', isPlugin: true, includeHomepage: true)}
-${environmentSection()}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
-${devDependenciesSection()}
+${_headerSection('plugin', isPlugin: true, includeHomepage: true)}
+${_environmentSection()}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
+${_devDependenciesSection()}
 ''');

      Error? commandError;
@ -230,11 +236,11 @@ ${devDependenciesSection()}
      final Directory pluginDirectory = createFakePlugin('plugin', packagesDir);

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin', isPlugin: true, includeRepository: false)}
-${environmentSection()}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
-${devDependenciesSection()}
+${_headerSection('plugin', isPlugin: true, includeRepository: false)}
+${_environmentSection()}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
+${_devDependenciesSection()}
 ''');

      Error? commandError;
@ -256,11 +262,11 @@ ${devDependenciesSection()}
      final Directory pluginDirectory = createFakePlugin('plugin', packagesDir);

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin', isPlugin: true, includeHomepage: true, includeRepository: false)}
-${environmentSection()}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
-${devDependenciesSection()}
+${_headerSection('plugin', isPlugin: true, includeHomepage: true, includeRepository: false)}
+${_environmentSection()}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
+${_devDependenciesSection()}
 ''');

      Error? commandError;
@ -283,11 +289,11 @@ ${devDependenciesSection()}
      final Directory pluginDirectory = createFakePlugin('plugin', packagesDir);

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin', isPlugin: true, repositoryPackagesDirRelativePath: 'different_plugin')}
-${environmentSection()}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
-${devDependenciesSection()}
+${_headerSection('plugin', isPlugin: true, repositoryPackagesDirRelativePath: 'different_plugin')}
+${_environmentSection()}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
+${_devDependenciesSection()}
 ''');

      Error? commandError;
@ -309,11 +315,11 @@ ${devDependenciesSection()}
      final Directory pluginDirectory = createFakePlugin('plugin', packagesDir);

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin', isPlugin: true, includeIssueTracker: false)}
-${environmentSection()}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
-${devDependenciesSection()}
+${_headerSection('plugin', isPlugin: true, includeIssueTracker: false)}
+${_environmentSection()}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
+${_devDependenciesSection()}
 ''');

      Error? commandError;
@ -336,11 +342,11 @@ ${devDependenciesSection()}
          createFakePlugin('a_plugin', packagesDir.childDirectory('a_plugin'));

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin', isPlugin: true, description: 'Too short')}
-${environmentSection()}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
-${devDependenciesSection()}
+${_headerSection('plugin', isPlugin: true, description: 'Too short')}
+${_environmentSection()}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
+${_devDependenciesSection()}
 ''');

      Error? commandError;
@ -365,11 +371,11 @@ ${devDependenciesSection()}
      final Directory pluginDirectory = createFakePlugin('plugin', packagesDir);

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin', isPlugin: true, description: 'Too short')}
-${environmentSection()}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
-${devDependenciesSection()}
+${_headerSection('plugin', isPlugin: true, description: 'Too short')}
+${_environmentSection()}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
+${_devDependenciesSection()}
 ''');

      Error? commandError;
@ -397,11 +403,11 @@ ${devDependenciesSection()}
          'the core description so that search results are more useful and the '
          'package does not lose pub points.';
      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin', isPlugin: true, description: description)}
-${environmentSection()}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
-${devDependenciesSection()}
+${_headerSection('plugin', isPlugin: true, description: description)}
+${_environmentSection()}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
+${_devDependenciesSection()}
 ''');

      Error? commandError;
@ -424,11 +430,11 @@ ${devDependenciesSection()}
      final Directory pluginDirectory = createFakePlugin('plugin', packagesDir);

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin', isPlugin: true)}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
-${devDependenciesSection()}
-${environmentSection()}
+${_headerSection('plugin', isPlugin: true)}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
+${_devDependenciesSection()}
+${_environmentSection()}
 ''');

      Error? commandError;
@ -451,11 +457,11 @@ ${environmentSection()}
      final Directory pluginDirectory = createFakePlugin('plugin', packagesDir);

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin', isPlugin: true)}
-${flutterSection(isPlugin: true)}
-${environmentSection()}
-${dependenciesSection()}
-${devDependenciesSection()}
+${_headerSection('plugin', isPlugin: true)}
+${_flutterSection(isPlugin: true)}
+${_environmentSection()}
+${_dependenciesSection()}
+${_devDependenciesSection()}
 ''');

      Error? commandError;
@ -478,11 +484,11 @@ ${devDependenciesSection()}
      final Directory pluginDirectory = createFakePlugin('plugin', packagesDir);

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin', isPlugin: true)}
-${environmentSection()}
-${flutterSection(isPlugin: true)}
-${devDependenciesSection()}
-${dependenciesSection()}
+${_headerSection('plugin', isPlugin: true)}
+${_environmentSection()}
+${_flutterSection(isPlugin: true)}
+${_devDependenciesSection()}
+${_dependenciesSection()}
 ''');

      Error? commandError;
@ -505,11 +511,11 @@ ${dependenciesSection()}
      final Directory pluginDirectory = createFakePlugin('plugin', packagesDir);

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin', isPlugin: true)}
-${environmentSection()}
-${devDependenciesSection()}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
+${_headerSection('plugin', isPlugin: true)}
+${_environmentSection()}
+${_devDependenciesSection()}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
 ''');

      Error? commandError;
@ -532,12 +538,12 @@ ${dependenciesSection()}
      final Directory pluginDirectory = createFakePlugin('plugin', packagesDir);

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin', isPlugin: true)}
-${environmentSection()}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
-${falseSecretsSection()}
-${devDependenciesSection()}
+${_headerSection('plugin', isPlugin: true)}
+${_environmentSection()}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
+${_falseSecretsSection()}
+${_devDependenciesSection()}
 ''');

      Error? commandError;
@ -562,11 +568,11 @@ ${devDependenciesSection()}
          'plugin_a_foo', packagesDir.childDirectory('plugin_a'));

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin_a_foo', isPlugin: true)}
-${environmentSection()}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
-${devDependenciesSection()}
+${_headerSection('plugin_a_foo', isPlugin: true)}
+${_environmentSection()}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
+${_devDependenciesSection()}
 ''');

      Error? commandError;
@ -590,11 +596,11 @@ ${devDependenciesSection()}
          'plugin_a_foo', packagesDir.childDirectory('plugin_a'));

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin_a_foo', isPlugin: true)}
-${environmentSection()}
-${flutterSection(isPlugin: true, implementedPackage: 'plugin_a_foo')}
-${dependenciesSection()}
-${devDependenciesSection()}
+${_headerSection('plugin_a_foo', isPlugin: true)}
+${_environmentSection()}
+${_flutterSection(isPlugin: true, implementedPackage: 'plugin_a_foo')}
+${_dependenciesSection()}
+${_devDependenciesSection()}
 ''');

      Error? commandError;
@ -618,15 +624,15 @@ ${devDependenciesSection()}
          'plugin_a_foo', packagesDir.childDirectory('plugin_a'));

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection(
+${_headerSection(
        'plugin_a_foo',
        isPlugin: true,
        repositoryPackagesDirRelativePath: 'plugin_a/plugin_a_foo',
      )}
-${environmentSection()}
-${flutterSection(isPlugin: true, implementedPackage: 'plugin_a')}
-${dependenciesSection()}
-${devDependenciesSection()}
+${_environmentSection()}
+${_flutterSection(isPlugin: true, implementedPackage: 'plugin_a')}
+${_dependenciesSection()}
+${_devDependenciesSection()}
 ''');

      final List<String> output =
@ -646,15 +652,15 @@ ${devDependenciesSection()}
          createFakePlugin('plugin_a', packagesDir.childDirectory('plugin_a'));

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection(
+${_headerSection(
        'plugin_a',
        isPlugin: true,
        repositoryPackagesDirRelativePath: 'plugin_a/plugin_a',
      )}
-${environmentSection()}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
-${devDependenciesSection()}
+${_environmentSection()}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
+${_devDependenciesSection()}
 ''');

      final List<String> output =
@ -676,16 +682,16 @@ ${devDependenciesSection()}
          packagesDir.childDirectory('plugin_a'));

      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection(
+${_headerSection(
        'plugin_a_platform_interface',
        isPlugin: true,
        repositoryPackagesDirRelativePath:
            'plugin_a/plugin_a_platform_interface',
      )}
-${environmentSection()}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
-${devDependenciesSection()}
+${_environmentSection()}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
+${_devDependenciesSection()}
 ''');

      final List<String> output =
@ -707,11 +713,11 @@ ${devDependenciesSection()}
      // Environment section is in the wrong location.
      // Missing 'implements'.
      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection('plugin_a_foo', isPlugin: true, publishable: false)}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
-${devDependenciesSection()}
-${environmentSection()}
+${_headerSection('plugin_a_foo', isPlugin: true, publishable: false)}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
+${_devDependenciesSection()}
+${_environmentSection()}
 ''');

      Error? commandError;
@ -737,17 +743,17 @@ ${environmentSection()}
      // Missing metadata that is only useful for published packages, such as
      // repository and issue tracker.
      pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
-${headerSection(
+${_headerSection(
        'plugin',
        isPlugin: true,
        publishable: false,
        includeRepository: false,
        includeIssueTracker: false,
      )}
-${environmentSection()}
-${flutterSection(isPlugin: true)}
-${dependenciesSection()}
-${devDependenciesSection()}
+${_environmentSection()}
+${_flutterSection(isPlugin: true)}
+${_dependenciesSection()}
+${_devDependenciesSection()}
 ''');

      final List<String> output =
@ -762,4 +768,51 @@ ${devDependenciesSection()}
      );
    });
  });
+
+  group('test pubspec_check_command on Windows', () {
+    late CommandRunner<void> runner;
+    late RecordingProcessRunner processRunner;
+    late FileSystem fileSystem;
+    late MockPlatform mockPlatform;
+    late Directory packagesDir;
+
+    setUp(() {
+      fileSystem = MemoryFileSystem(style: FileSystemStyle.windows);
+      mockPlatform = MockPlatform(isWindows: true);
+      packagesDir = fileSystem.currentDirectory.childDirectory('packages');
+      createPackagesDirectory(parentDir: packagesDir.parent);
+      processRunner = RecordingProcessRunner();
+      final PubspecCheckCommand command = PubspecCheckCommand(
+        packagesDir,
+        processRunner: processRunner,
+        platform: mockPlatform,
+      );
+
+      runner = CommandRunner<void>(
+          'pubspec_check_command', 'Test for pubspec_check_command');
+      runner.addCommand(command);
+    });
+
+    test('repository check works', () async {
+      final Directory packageDirectory =
+          createFakePackage('package', packagesDir);
+
+      packageDirectory.childFile('pubspec.yaml').writeAsStringSync('''
+${_headerSection('package')}
+${_environmentSection()}
+${_dependenciesSection()}
+''');
+
+      final List<String> output =
+          await runCapturingPrint(runner, <String>['pubspec-check']);
+
+      expect(
+        output,
+        containsAllInOrder(<Matcher>[
+          contains('Running for package...'),
+          contains('No issues found!'),
+        ]),
+      );
+    });
+  });
 }