Skip to content

content test: Compare parse trees via Diagnosticable #206

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 28, 2023
Merged

content test: Compare parse trees via Diagnosticable #206

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7d760b1
content [nfc]: Fix up docs formatting a bit
gnprice Jun 28, 2023
6f9df45
content test: Compare parse trees via Diagnosticable
gnprice Jun 28, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 18 additions & 17 deletions lib/model/content.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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});
Expand All @@ -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);
Expand Down Expand Up @@ -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
/// <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 {
Expand Down Expand Up @@ -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});
Expand Down
142 changes: 48 additions & 94 deletions test/model/content_checks.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
}