diff --git a/lib/model/content.dart b/lib/model/content.dart index 1dd237b3f7..88afbf9114 100644 --- a/lib/model/content.dart +++ b/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); @@ -92,7 +93,7 @@ class ZulipContent extends ContentNode { /// /// Generally these correspond to HTML elements which in the Zulip web client /// are laid out as block-level boxes, in a block formatting context: -/// https://developer.mozilla.org/en-US/docs/Web/CSS/CSS_flow_layout/Block_and_inline_layout_in_normal_flow +/// /// /// Almost all nodes are either a [BlockContentNode] or an [InlineContentNode]. abstract class BlockContentNode extends ContentNode { @@ -143,14 +144,14 @@ class LineBreakNode extends BlockContentNode { int get hashCode => 'LineBreakNode'.hashCode; } -// A `p` element, or a place where the DOM tree logically wanted one. -// -// We synthesize these in the absence of an actual `p` element in cases where -// there's inline content (like [dom.Text] nodes, links, or spans) in a context -// where block content can also appear (like inside a `li`.) These are marked -// with [wasImplicit]. -// -// See also [parseImplicitParagraphBlockContentList]. +/// A `p` element, or a place where the DOM tree logically wanted one. +/// +/// We synthesize these in the absence of an actual `p` element in cases where +/// there's inline content (like [dom.Text] nodes, links, or spans) in a context +/// where block content can also appear (like inside a `li`.) These are marked +/// with [wasImplicit]. +/// +/// See also [parseImplicitParagraphBlockContentList]. class ParagraphNode extends BlockInlineContainerNode { const ParagraphNode( {super.debugHtmlNode, required super.nodes, this.wasImplicit = false}); diff --git a/test/model/content_checks.dart b/test/model/content_checks.dart index d0412d515e..c9f38af1a7 100644 --- a/test/model/content_checks.dart +++ b/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 { 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() - .nodes.equalsNodes(expected.nodes); - } else if (expected is UnimplementedBlockContentNode) { - isA() - .debugHtmlText.equals(expected.debugHtmlText); - } else if (expected is ParagraphNode) { - isA() - ..wasImplicit.equals(expected.wasImplicit) - ..nodes.equalsNodes(expected.nodes); - } else if (expected is HeadingNode) { - isA() - ..level.equals(expected.level) - ..nodes.equalsNodes(expected.nodes); - } else if (expected is ListNode) { - isA() - ..style.equals(expected.style) - ..items.deepEquals(expected.items.map( - (item) => it()..isA>().equalsNodes(item))); - } else if (expected is QuotationNode) { - isA() - .nodes.equalsNodes(expected.nodes); - } else if (expected is UnimplementedInlineContentNode) { - isA() - .debugHtmlText.equals(expected.debugHtmlText); - } else if (expected is StrongNode) { - isA() - .nodes.equalsNodes(expected.nodes); - } else if (expected is EmphasisNode) { - isA() - .nodes.equalsNodes(expected.nodes); - } else if (expected is InlineCodeNode) { - isA() - .nodes.equalsNodes(expected.nodes); - } else if (expected is LinkNode) { - isA() - .nodes.equalsNodes(expected.nodes); - } else if (expected is UserMentionNode) { - isA() - .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 get debugHtmlText => has((n) => n.debugHtmlText, 'debugHtmlText'); } -extension ZulipContentChecks on Subject { - Subject> get nodes => has((n) => n.nodes, 'nodes'); -} +Iterable? _compareDiagnosticsNodes(DiagnosticsNode actual, DiagnosticsNode expected) { + assert(actual is DiagnosticableTreeNode && expected is DiagnosticableTreeNode); -extension BlockContentNodeListChecks on Subject> { - void equalsNodes(List expected) { - deepEquals(expected.map( - (e) => it()..isA().equalsNode(e))); - // A shame we need the dynamic `isA` there. This - // version hits a runtime type error: - // .nodes.deepEquals(expected.nodes.map( - // (e) => it()..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 { - Subject> get nodes => has((n) => n.nodes, 'nodes'); -} - -extension ParagraphNodeChecks on Subject { - Subject get wasImplicit => has((n) => n.wasImplicit, 'wasImplicit'); -} - -extension HeadingNodeChecks on Subject { - Subject get level => has((n) => n.level, 'level'); -} - -extension ListNodeChecks on Subject { - Subject get style => has((n) => n.style, 'style'); - Subject>> get items => has((n) => n.items, 'items'); -} -extension QuotationNodeChecks on Subject { - Subject> 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> { - void equalsNodes(List expected) { - deepEquals(expected.map( - (e) => it()..isA().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 { - Subject> get nodes => has((n) => n.nodes, 'nodes'); + return null; }