content test: Compare parse trees via Diagnosticable

This makes for significantly cleaner and more informative output
when a test fails (resolving all the TODO-comments), by taking
more control of how the comparison is done.

At the same time, it makes for less boilerplate and fewer places
to update when adding more features in the ContentNode parse trees.
Effectively it deduplicates some of the boilerplate, by reusing
for comparing the trees the metadata that we're already prepared
to generate for printing them out when a comparison fails.

Author: gnprice

lib/model/content.dart

@@ -17,12 +17,16 @@ import 'package:html/parser.dart';
 ///  * Override [debugDescribeChildren] and/or [debugFillProperties]
 ///    to report all the data attached to the node, for debugging.
 ///    See docs: https://api.flutter.dev/flutter/foundation/Diagnosticable/debugFillProperties.html
+///    We also rely on these for comparing actual to expected in tests.
 ///
-/// When modifying subclasses:
-///  * Always check the following places to see if they need a matching update:
-///    * [==] and [hashCode], if overridden.
-///    * [debugFillProperties] and/or [debugDescribeChildren], if present.
-///    * `equalsNode` in test/model/content_checks.dart .
+/// When modifying subclasses, always check the following places
+/// to see if they need a matching update:
+///  * [==] and [hashCode], if overridden.
+///  * [debugFillProperties] and/or [debugDescribeChildren].
+///
+/// In particular, a newly-added field typically must be added in
+/// [debugFillProperties].  Otherwise tests will not examine the new field,
+/// and will not spot discrepancies there.
 @immutable
 sealed class ContentNode extends DiagnosticableTree {
   const ContentNode({this.debugHtmlNode});
@@ -42,9 +46,6 @@ sealed class ContentNode extends DiagnosticableTree {
 
   @override
   String toString({DiagnosticLevel minLevel = DiagnosticLevel.info}) {
-    // TODO(checks): Better integrate package:checks with Diagnosticable, for
-    //   better interaction in output indentation.
-    //   (See also comment at equalsNode, with related improvements.)
     String? result;
     assert(() {
       result = toStringDeep(minLevel: minLevel);

test/model/content_checks.dart

@@ -1,108 +1,62 @@
-import 'package:checks/checks.dart';
+import 'package:checks/context.dart';
+import 'package:flutter/foundation.dart';
 import 'package:zulip/model/content.dart';
 
 extension ContentNodeChecks on Subject<ContentNode> {
   void equalsNode(ContentNode expected) {
-    // TODO: Make equalsNode output clearer on failure, applying Diagnosticable.
-    //   In particular (a) show the top-level expected node in one piece
-    //   (as well as the actual); (a') ideally, suppress on the "expected" side
-    //   the various predicates below, which should be redundant with just
-    //   the expected node; (b) show expected for the specific `equals` leaf.
-    //   See also comment on [ContentNode.toString].
-    if (expected is ZulipContent) {
-      isA<ZulipContent>()
-        .nodes.equalsNodes(expected.nodes);
-    } else if (expected is UnimplementedBlockContentNode) {
-      isA<UnimplementedBlockContentNode>()
-        .debugHtmlText.equals(expected.debugHtmlText);
-    } else if (expected is ParagraphNode) {
-      isA<ParagraphNode>()
-        ..wasImplicit.equals(expected.wasImplicit)
-        ..nodes.equalsNodes(expected.nodes);
-    } else if (expected is HeadingNode) {
-      isA<HeadingNode>()
-        ..level.equals(expected.level)
-        ..nodes.equalsNodes(expected.nodes);
-    } else if (expected is ListNode) {
-      isA<ListNode>()
-        ..style.equals(expected.style)
-        ..items.deepEquals(expected.items.map(
-          (item) => it()..isA<List<BlockContentNode>>().equalsNodes(item)));
-    } else if (expected is QuotationNode) {
-      isA<QuotationNode>()
-        .nodes.equalsNodes(expected.nodes);
-    } else if (expected is UnimplementedInlineContentNode) {
-      isA<UnimplementedInlineContentNode>()
-        .debugHtmlText.equals(expected.debugHtmlText);
-    } else if (expected is StrongNode) {
-      isA<StrongNode>()
-        .nodes.equalsNodes(expected.nodes);
-    } else if (expected is EmphasisNode) {
-      isA<EmphasisNode>()
-        .nodes.equalsNodes(expected.nodes);
-    } else if (expected is InlineCodeNode) {
-      isA<InlineCodeNode>()
-        .nodes.equalsNodes(expected.nodes);
-    } else if (expected is LinkNode) {
-      isA<LinkNode>()
-        .nodes.equalsNodes(expected.nodes);
-    } else if (expected is UserMentionNode) {
-      isA<UserMentionNode>()
-        .nodes.equalsNodes(expected.nodes);
-    } else {
-      // The remaining node types have structural `==`.  Use that.
-      equals(expected);
-    }
+    return context.expect(() => prefixFirst('equals ', literal(expected)), (actual) {
+      final which = _compareDiagnosticsNodes(
+        actual.toDiagnosticsNode(), expected.toDiagnosticsNode());
+      return which == null ? null : Rejection(which: [
+        'differs in that it:',
+        ...indent(which),
+      ]);
+    });
   }
-
-  Subject<String> get debugHtmlText => has((n) => n.debugHtmlText, 'debugHtmlText');
 }
 
-extension ZulipContentChecks on Subject<ZulipContent> {
-  Subject<List<BlockContentNode>> get nodes => has((n) => n.nodes, 'nodes');
-}
+Iterable<String>? _compareDiagnosticsNodes(DiagnosticsNode actual, DiagnosticsNode expected) {
+  assert(actual is DiagnosticableTreeNode && expected is DiagnosticableTreeNode);
 
-extension BlockContentNodeListChecks on Subject<List<BlockContentNode>> {
-  void equalsNodes(List<BlockContentNode> expected) {
-    deepEquals(expected.map(
-      (e) => it()..isA<BlockContentNode>().equalsNode(e)));
-    // A shame we need the dynamic `isA` there.  This
-    // version hits a runtime type error:
-    //   .nodes.deepEquals(expected.nodes.map(
-    //     (e) => it<BlockContentNode>()..equalsNode(e)));
-    // and with `it()` with no type argument, it doesn't type-check.
-    // TODO(checks): report that as API feedback on deepEquals
+  if (actual.value.runtimeType != expected.value.runtimeType) {
+    return [
+      'has type ${actual.value.runtimeType}',
+      'expected: ${expected.value.runtimeType}',
+    ];
   }
-}
-
-extension BlockInlineContainerNodeChecks on Subject<BlockInlineContainerNode> {
-  Subject<List<InlineContentNode>> get nodes => has((n) => n.nodes, 'nodes');
-}
-
-extension ParagraphNodeChecks on Subject<ParagraphNode> {
-  Subject<bool> get wasImplicit => has((n) => n.wasImplicit, 'wasImplicit');
-}
-
-extension HeadingNodeChecks on Subject<HeadingNode> {
-  Subject<HeadingLevel> get level => has((n) => n.level, 'level');
-}
-
-extension ListNodeChecks on Subject<ListNode> {
-  Subject<ListStyle> get style => has((n) => n.style, 'style');
-  Subject<List<List<BlockContentNode>>> get items => has((n) => n.items, 'items');
-}
 
-extension QuotationNodeChecks on Subject<QuotationNode> {
-  Subject<List<BlockContentNode>> get nodes => has((n) => n.nodes, 'nodes');
-}
+  final actualProperties = actual.getProperties();
+  final expectedProperties = expected.getProperties();
+  assert(actualProperties.length == expectedProperties.length);
+  for (int i = 0; i < actualProperties.length; i++) {
+    assert(actualProperties[i].name == expectedProperties[i].name);
+    if (actualProperties[i].value != expectedProperties[i].value) {
+      return [
+        'has ${actualProperties[i].name} that:',
+        ...indent(prefixFirst('is ',        literal(actualProperties[i].value))),
+        ...indent(prefixFirst('expected: ', literal(expectedProperties[i].value)))
+      ];
+    }
+  }
 
-extension InlineContentNodeListChecks on Subject<List<InlineContentNode>> {
-  void equalsNodes(List<InlineContentNode> expected) {
-    deepEquals(expected.map(
-      (e) => it()..isA<InlineContentNode>().equalsNode(e)));
+  final actualChildren = actual.getChildren();
+  final expectedChildren = expected.getChildren();
+  if (actualChildren.length != expectedChildren.length) {
+    return [
+      'has ${actualChildren.length} children',
+      'expected: ${expectedChildren.length} children',
+    ];
+  }
+  for (int i = 0; i < actualChildren.length; i++) {
+    final failure = _compareDiagnosticsNodes(actualChildren[i], expectedChildren[i]);
+    if (failure != null) {
+      final diagnosticable = actualChildren[i].value as Diagnosticable;
+      return [
+        'has child $i (${diagnosticable.toStringShort()}) that:',
+        ...indent(failure),
+      ];
+    }
   }
-}
 
-extension InlineContainerNodeChecks on Subject<InlineContainerNode> {
-  Subject<List<InlineContentNode>> get nodes => has((n) => n.nodes, 'nodes');
+  return null;
 }