Preserve newline following {@endtemplate}. (#2289)

Preserve newline following `{@endtemplate}`.

Additionally introduce (some of?) the first unit tests. These live
in test/unit/ for now. This includes a new dev dependency on mockito.

Fixes #1848

Author: srawlins

lib/src/model/comment_processable.dart

@@ -11,7 +11,7 @@ import 'package:dartdoc/src/warnings.dart';
 import 'package:path/path.dart' as path;
 
 final _templatePattern = RegExp(
-    r'[ ]*{@template\s+(.+?)}([\s\S]+?){@endtemplate}[ ]*\n?',
+    r'[ ]*{@template\s+(.+?)}([\s\S]+?){@endtemplate}[ ]*(\n?)',
     multiLine: true);
 final _htmlPattern = RegExp(
     r'[ ]*{@inject-html\s*}([\s\S]+?){@end-inject-html}[ ]*\n?',
@@ -366,9 +366,6 @@ mixin CommentProcessable on Documentable, Warnable, Locatable, SourceCodeMixin {
   /// The width and height must be integers specifying the dimensions of the
   /// video file in pixels.
   String _injectAnimations(String rawDocs) {
-    // Make sure we have a set to keep track of used IDs for this href.
-    package.usedAnimationIdsByHref[href] ??= {};
-
     String getUniqueId(String base) {
       var animationIdCount = 1;
       var id = '$base$animationIdCount';
@@ -382,6 +379,9 @@ mixin CommentProcessable on Documentable, Warnable, Locatable, SourceCodeMixin {
     }
 
     return rawDocs.replaceAllMapped(_basicAnimationPattern, (basicMatch) {
+      // Make sure we have a set to keep track of used IDs for this href.
+      package.usedAnimationIdsByHref[href] ??= {};
+
       var parser = ArgParser();
       parser.addOption('id');
       var args = _parseArgs(basicMatch[1], parser, 'animation');
@@ -481,8 +481,9 @@ mixin CommentProcessable on Documentable, Warnable, Locatable, SourceCodeMixin {
     return rawDocs.replaceAllMapped(_templatePattern, (match) {
       var name = match[1].trim();
       var content = match[2].trim();
+      var trailingNewline = match[3];
       packageGraph.addMacro(name, content);
-      return '{@macro $name}';
+      return '{@macro $name}$trailingNewline';
     });
   }
 

lib/src/model/model.dart

@@ -7,6 +7,7 @@ export 'canonicalization.dart';
 export 'categorization.dart';
 export 'category.dart';
 export 'class.dart';
+export 'comment_processable.dart';
 export 'constructor.dart';
 export 'container.dart';
 export 'container_member.dart';

pubspec.yaml

@@ -40,6 +40,7 @@ dev_dependencies:
   grinder: ^0.8.2
   io: ^0.3.0
   http: ^0.12.0
+  mockito: ^4.1.1
   pedantic: ^1.9.0
   test: ^1.3.0
 

test/README.md

@@ -0,0 +1,10 @@
+# Tests
+
+Most of dartdoc's tests are large end-to-end tests which read real files in
+real packages, in the `testing/` directory. Unit tests exist in `test/unit/`.
+
+Many of the end-to-end test cases should be rewritten as unit tests.
+
+At some point, the distinction should flip, such that unit tests are generally
+located in `test/`, and end-to-end tests are found in a specific directory, or
+in files whose names signify the distinction.

test/model_test.dart

@@ -719,7 +719,7 @@ void main() {
 
     test("renders a macro within the same comment where it's defined", () {
       expect(withMacro.documentation,
-          equals('Macro method\n\nFoo macro content\nMore docs'));
+          equals('Macro method\n\nFoo macro content\n\nMore docs'));
     });
 
     test("renders a macro in another method, not the same where it's defined",

test/src/utils.dart

@@ -384,8 +384,12 @@ class SubprocessLauncher {
 
     var exitCode = await process.exitCode;
     if (exitCode != 0) {
-      throw ProcessException(executable, arguments,
-          'SubprocessLauncher got non-zero exitCode: $exitCode', exitCode);
+      throw ProcessException(
+          executable,
+          arguments,
+          'SubprocessLauncher got non-zero exitCode: $exitCode\n\n'
+          'stdout: ${process.stdout}\n\nstderr: ${process.stderr}',
+          exitCode);
     }
     return jsonObjects;
   }

test/unit/comment_processable_test.dart

@@ -0,0 +1,195 @@
+// Copyright (c) 2020, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+import 'dart:io' show Directory;
+
+import 'package:dartdoc/src/dartdoc_options.dart';
+import 'package:dartdoc/src/model/model.dart';
+import 'package:dartdoc/src/package_meta.dart';
+import 'package:dartdoc/src/warnings.dart';
+import 'package:mockito/mockito.dart';
+import 'package:test/test.dart';
+
+void main() {
+  _Processor processor;
+  setUp(() {
+    processor = _Processor(_FakeDartdocOptionContext());
+    processor.href = '/project/a.dart';
+  });
+
+  test('removes triple slashes', () async {
+    var doc = await processor.processComment('''
+/// Text.
+/// More text.
+''');
+    expect(doc, equals('''
+Text.
+More text.'''));
+  });
+
+  test('removes space after triple slashes', () async {
+    var doc = await processor.processComment('''
+///  Text.
+///    More text.
+''');
+    // TODO(srawlins): Actually, the three spaces before 'More' is perhaps not
+    // the best fit. Should it only be two, to match the indent from the first
+    // line's "Text"?
+    expect(doc, equals('''
+Text.
+   More text.'''));
+  });
+
+  test('leaves blank lines', () async {
+    var doc = await processor.processComment('''
+/// Text.
+///
+/// More text.
+''');
+    expect(doc, equals('''
+Text.
+
+More text.'''));
+  });
+
+  test('processes @template', () async {
+    var doc = await processor.processComment('''
+/// Text.
+///
+/// {@template abc}
+/// Template text.
+/// {@endtemplate}
+///
+/// End text.
+''');
+    expect(doc, equals('''
+Text.
+
+{@macro abc}
+
+End text.'''));
+    verify(processor.packageGraph.addMacro('abc', 'Template text.')).called(1);
+  });
+
+  test('processes leading @template', () async {
+    var doc = await processor.processComment('''
+/// {@template abc}
+/// Template text.
+/// {@endtemplate}
+///
+/// End text.
+''');
+    expect(doc, equals('''
+{@macro abc}
+
+End text.'''));
+    verify(processor.packageGraph.addMacro('abc', 'Template text.')).called(1);
+  });
+
+  test('processes trailing @template', () async {
+    var doc = await processor.processComment('''
+/// Text.
+///
+/// {@template abc}
+/// Template text.
+/// {@endtemplate}
+''');
+    expect(doc, equals('''
+Text.
+
+{@macro abc}'''));
+    verify(processor.packageGraph.addMacro('abc', 'Template text.')).called(1);
+  });
+
+  test('processes @template w/o blank line following', () async {
+    var doc = await processor.processComment('''
+/// Text.
+///
+/// {@template abc}
+/// Template text.
+/// {@endtemplate}
+/// End text.
+''');
+    expect(doc, equals('''
+Text.
+
+{@macro abc}
+End text.'''));
+    verify(processor.packageGraph.addMacro('abc', 'Template text.')).called(1);
+  });
+
+  test('allows whitespace around @template name', () async {
+    var doc = await processor.processComment('''
+/// {@template    abc    }
+/// Template text.
+/// {@endtemplate}
+''');
+    expect(doc, equals('''
+{@macro abc}'''));
+    verify(processor.packageGraph.addMacro('abc', 'Template text.')).called(1);
+  });
+
+  // TODO(srawlins): More unit tests: @example, @youtube, @animation,
+  // @inject-html, @tool.
+}
+
+/// In order to mix in [CommentProcessable], we must first implement
+/// the super-class constraints.
+abstract class __Processor extends Fake
+    implements Documentable, Warnable, Locatable, SourceCodeMixin {}
+
+/// A simple comment processor for testing [CommentProcessable].
+class _Processor extends __Processor with CommentProcessable {
+  @override
+  final _FakeDartdocOptionContext config;
+
+  @override
+  final _FakePackage package;
+
+  @override
+  final _MockPackageGraph packageGraph;
+
+  @override
+  String href;
+
+  _Processor(this.config)
+      : package = _FakePackage(),
+        packageGraph = _MockPackageGraph() {
+    throwOnMissingStub(packageGraph);
+    when(packageGraph.addMacro(any, any)).thenReturn(null);
+  }
+}
+
+class _FakeDirectory extends Fake implements Directory {
+  @override
+  final String path;
+
+  _FakeDirectory() : path = '/project';
+}
+
+class _FakePackage extends Fake implements Package {
+  @override
+  final PackageMeta packageMeta;
+
+  _FakePackage() : packageMeta = _FakePackageMeta();
+}
+
+class _FakePackageMeta extends Fake implements PackageMeta {
+  @override
+  final Directory dir;
+
+  _FakePackageMeta() : dir = _FakeDirectory();
+}
+
+class _FakeDartdocOptionContext extends Fake implements DartdocOptionContext {
+  @override
+  final bool allowTools;
+
+  @override
+  final bool injectHtml;
+
+  _FakeDartdocOptionContext({this.allowTools = false, this.injectHtml = false});
+}
+
+class _MockPackageGraph extends Mock implements PackageGraph {}