Fix flutter root exception (#1714)

* Move SDK library expectation to all subcategories * Retype has been removed * Improve error message when FLUTTER_ROOT is needed but not set * Add a test for the new exception * Make the test work even when FLUTTER_ROOT is already in the environemnt * Allow the pub warning as well, since if pub get hasn't been run yet that's the error users will see
dart-lang · Jun 14, 2018 · 6241b55 · 6241b55
1 parent adbd2bb
commit 6241b55
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 9 deletions.
diff --git a/lib/src/dartdoc_options.dart b/lib/src/dartdoc_options.dart
@@ -1022,13 +1022,12 @@ Future<List<DartdocOption>> createDartdocOptions() async {
     // This could be a ArgOnly, but trying to not provide too many ways
     // to set the flutter root.
     new DartdocOptionSyntheticOnly<String>(
-      'flutterRoot',
-      (DartdocSyntheticOption<String> option, Directory dir) =>
-          resolveTildePath(Platform.environment['FLUTTER_ROOT']),
-      isDir: true,
-      help: 'Root of the Flutter SDK, specified from environment.',
-      mustExist: true,
-    ),
+        'flutterRoot',
+        (DartdocSyntheticOption<String> option, Directory dir) =>
+            resolveTildePath(Platform.environment['FLUTTER_ROOT']),
+        isDir: true,
+        help: 'Root of the Flutter SDK, specified from environment.',
+        mustExist: true),
     new DartdocOptionArgOnly<bool>('hideSdkText', false,
         hide: true,
         help:
@@ -1120,8 +1119,12 @@ Future<List<DartdocOption>> createDartdocOptions() async {
       if (!option.parent['sdkDocs'].valueAt(dir) &&
           (option.root['topLevelPackageMeta'].valueAt(dir) as PackageMeta)
               .requiresFlutter) {
-        return pathLib.join(option.root['flutterRoot'].valueAt(dir), 'bin',
-            'cache', 'dart-sdk');
+        String flutterRoot = option.root['flutterRoot'].valueAt(dir);
+        if (flutterRoot == null) {
+          throw new DartdocOptionError(
+              'Top level package requires Flutter but FLUTTER_ROOT environment variable not set');
+        }
+        return pathLib.join(flutterRoot, 'bin', 'cache', 'dart-sdk');
       }
       return defaultSdkDir.absolute.path;
     }, help: 'Path to the SDK directory.', isDir: true, mustExist: true),

diff --git a/test/compare_output_test.dart b/test/compare_output_test.dart
@@ -27,6 +27,9 @@ String get _testPackageDocsPath =>
 String get _testPackagePath =>
     pathLib.fromUri(_currentFileUri.resolve('../testing/test_package'));
 
+String get _testPackageFlutterPluginPath => pathLib
+    .fromUri(_currentFileUri.resolve('../testing/test_package_flutter_plugin'));
+
 void main() {
   group('compare outputs', () {
     Directory tempDir;
@@ -44,6 +47,21 @@ void main() {
       }
     });
 
+    test('Validate missing FLUTTER_ROOT exception is clean', () async {
+      var args = <String>[dartdocBin];
+      var result = Process.runSync(Platform.resolvedExecutable, args,
+          environment: new Map.from(Platform.environment)
+            ..remove('FLUTTER_ROOT'),
+          includeParentEnvironment: false,
+          workingDirectory: _testPackageFlutterPluginPath);
+      expect(
+          result.stderr,
+          contains(new RegExp(
+              'Top level package requires Flutter but FLUTTER_ROOT environment variable not set|test_package_flutter_plugin requires the Flutter SDK, version solving failed')));
+      expect(result.stderr, isNot(contains('asynchronous gap')));
+      expect(result.exitCode, isNot(0));
+    });
+
     test("Validate --version works", () async {
       var args = <String>[dartdocBin, '--version'];
       var result = Process.runSync(Platform.resolvedExecutable, args,