Fix bug in warnings where not specifying --show-warnings caused dartdoc to succeed even if there were errors. (#1805)

* Fix bug in warnings where not specifying --show-warnings caused dartdoc to succeed even if there were errors.

* Moving tests that are supposed to create errors into a separate package.

Author: gspencergoog

CHANGELOG.md

@@ -1,5 +1,7 @@
 ## 0.24.1-dev
 * Added more metadata (element name, project name, etc.) to external tool invocations.
+* Fixed a bug where not specifying --show-warnings caused dartdoc to report success
+  even when errors were present.
 
 ## 0.24.0
 * Add 'override' to feature list for members which override a superclass (#981)

lib/src/model.dart

@@ -6491,18 +6491,16 @@ class PackageBuilder {
     PackageWarningOptions warningOptions =
         new PackageWarningOptions(config.verboseWarnings);
     // TODO(jcollins-g): explode this into detailed command line options.
-    if (config.showWarnings) {
-      for (PackageWarning kind in PackageWarning.values) {
-        switch (kind) {
-          case PackageWarning.toolError:
-          case PackageWarning.invalidParameter:
-          case PackageWarning.unresolvedExport:
-            warningOptions.error(kind);
-            break;
-          default:
-            warningOptions.warn(kind);
-            break;
-        }
+    for (PackageWarning kind in PackageWarning.values) {
+      switch (kind) {
+        case PackageWarning.toolError:
+        case PackageWarning.invalidParameter:
+        case PackageWarning.unresolvedExport:
+          warningOptions.error(kind);
+          break;
+        default:
+          if (config.showWarnings) warningOptions.warn(kind);
+          break;
       }
     }
     return warningOptions;

lib/src/warnings.dart

@@ -101,7 +101,7 @@ final Map<PackageWarning, PackageWarningHelpText> packageWarningText = const {
   PackageWarning.unresolvedExport: const PackageWarningHelpText(
       PackageWarning.unresolvedExport,
       "unresolvedExport",
-      "An export refers to a URI that can not be resolved."),
+      "An export refers to a URI that cannot be resolved."),
 };
 
 /// Something that package warnings can be called on.  Optionally associated

test/dartdoc_test.dart

@@ -73,6 +73,45 @@ void main() {
         expect(Something.isPublic, isTrue);
         expect(Something.displayedCategories, isNotEmpty);
       });
+
+      test('errors generate errors even when warnings are off', () async {
+        Dartdoc dartdoc = await buildDartdoc([], testPackageToolError);
+        DartdocResults results = await dartdoc.generateDocsBase();
+        PackageGraph p = results.packageGraph;
+        Iterable<String> unresolvedToolErrors = p
+            .packageWarningCounter.countedWarnings.values
+            .expand<String>((Set<Tuple2<PackageWarning, String>> s) => s
+                .where((Tuple2<PackageWarning, String> t) =>
+                    t.item1 == PackageWarning.toolError)
+                .map<String>((Tuple2<PackageWarning, String> t) => t.item2));
+
+        expect(p.packageWarningCounter.errorCount, equals(1));
+        expect(unresolvedToolErrors.length, equals(1));
+        expect(unresolvedToolErrors.first,
+            contains('Tool "drill" returned non-zero exit code'));
+      });
+    });
+
+    group('Invoking command-line dartdoc', () {
+      String dartdocPath = pathLib.join('bin', 'dartdoc.dart');
+
+      test('errors cause non-zero exit when warnings are off', () async {
+        ProcessResult result = Process.runSync(Platform.resolvedExecutable, [
+          dartdocPath,
+          '--input=$testPackageToolError',
+          '--output=${pathLib.join(tempDir.absolute.path, 'test_package_tool_error')}',
+        ]);
+        expect(result.exitCode, isNonZero);
+      });
+      test('errors cause non-zero exit when warnings are on', () async {
+        ProcessResult result = Process.runSync(Platform.resolvedExecutable, [
+          dartdocPath,
+          '--input=$testPackageToolError',
+          '--output=${pathLib.join(tempDir.absolute.path, 'test_package_tool_error')}',
+          '--show-warnings',
+        ]);
+        expect(result.exitCode, isNonZero);
+      });
     });
 
     group('Option handling with cross-linking', () {