Skip to content

Commit

Permalink
fix a few match issues and prepare for test provider support (jest-co…
Browse files Browse the repository at this point in the history 
…mmunity#738)

* fix a few match issues and prepare for test provider support
  • Loading branch information
@connectdotz
connectdotz committed Aug 1, 2021
1 parent 4271e79 commit 47526da
Show file tree
Hide file tree
Showing 5 changed files with 182 additions and 57 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@ Bug-fixes within the same version aren't needed
## Master
* fix type warning in settings.json when using the `{"autoRun": "off"}` option - @tommy
* fix couple of test result matching issues - @connectdotz (#737)
* update match API/attributes to support up-coming vscode test provider API. - @connectdotz (#737)
-->

Expand Down
91 changes: 66 additions & 25 deletions src/TestResults/match-by-context.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ import {
ROOT_NODE_NAME,
OptionalAttributes,
MatchEvent,
NodeType,
MatchOptions,
} from './match-node';

export const buildAssertionContainer = (
Expand All @@ -46,7 +48,10 @@ export const buildAssertionContainer = (
new DataNode(a.title, zeroBasedLine, a, {
fullName: a.fullName,
isGroup: 'maybe',
range: { start: zeroBasedLine, end: zeroBasedLine },
range: {
start: { line: zeroBasedLine, column: 0 },
end: { line: zeroBasedLine, column: 0 },
},
})
);
});
Expand All @@ -57,6 +62,7 @@ export const buildAssertionContainer = (
return root;
};

const UnknownRange = { start: { line: -1, column: -1 }, end: { line: -1, column: -1 } };
export const buildSourceContainer = (sourceRoot: ParsedNode): ContainerNode<ItBlock> => {
const isDescribeBlock = (node: ParsedNode): node is DescribeBlock => node.type === 'describe';
const isItBlock = (node: ParsedNode): node is ItBlock => node.type === 'it';
Expand All @@ -65,7 +71,10 @@ export const buildSourceContainer = (sourceRoot: ParsedNode): ContainerNode<ItBl
const attrs = (namedNode: NamedBlock): OptionalAttributes => ({
isGroup: namedNode.lastProperty === 'each' ? 'yes' : 'no',
nonLiteralName: namedNode.nameType !== 'Literal',
range: { start: namedNode.start?.line - 1 ?? -1, end: namedNode.end?.line - 1 ?? -1 },
range: {
start: namedNode.start ? adjustLocation(namedNode.start) : UnknownRange.start,
end: namedNode.end ? adjustLocation(namedNode.end) : UnknownRange.end,
},
});
if (isDescribeBlock(node)) {
container = new ContainerNode(node.name, attrs(node));
Expand Down Expand Up @@ -95,7 +104,7 @@ const matchPos = (t: ItBlock, a: TestAssertionStatus, forError = false): boolean
};

// could not use "instanceof" check as it could fail tests that mocked jest-editor-support like in TestResultProvider.test.ts
const isDataNode = (arg: DataNode<ItBlock> | ItBlock): arg is DataNode<ItBlock> =>
const isSourceDataNode = (arg: DataNode<ItBlock> | ItBlock): arg is DataNode<ItBlock> =>
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(arg as any).data;

Expand All @@ -104,7 +113,7 @@ export const toMatchResult = (
assertionOrErr: DataNode<TestAssertionStatus> | string,
reason: MatchEvent
): TestResult => {
const [test, sourceHistory, sourceName] = isDataNode(source)
const [test, sourceHistory, sourceName] = isSourceDataNode(source)
? [source.data, source.history(reason), source.fullName]
: [source, [reason], source.name];
const [assertion, assertionHistory, err] =
Expand Down Expand Up @@ -178,9 +187,27 @@ interface FallbackMatchResult<C extends ContextType> {
type ClassicMatchType = 'by-name' | 'by-location';

const ContextMatch = (): ContextMatchAlgorithm => {
const onMatch = (
tNode: NodeType<ItBlock>,
aNode: NodeType<TestAssertionStatus>,
event: MatchEvent
): void => {
tNode.addEvent(event);
aNode.addEvent(event);
aNode.attrs.range = tNode.attrs.range;
};
const onMatchResult = (
tNode: DataNode<ItBlock>,
aNode: DataNode<TestAssertionStatus>,
event: MatchEvent
): TestResult => {
onMatch(tNode, aNode, event);
return toMatchResult(tNode, aNode, event);
};
const handleTestBlockMatch = (
result: MatchResultType<'data'>,
reportUnmatch: boolean
reportUnmatch = false,
matchGroup = true
): TestResult[] => {
const [t, matched, reason] = result;
if (matched.length === 0) {
Expand All @@ -190,31 +217,45 @@ const ContextMatch = (): ContextMatchAlgorithm => {
return [];
}

return matched.flatMap((a) => a.getAll().map((aa) => toMatchResult(t, aa, reason)));
return matched.flatMap((a) =>
matchGroup
? a.getAll().map((aa) => onMatchResult(t, aa, reason))
: onMatchResult(t, a, reason)
);
};

const handleDescribeBlockMatch = (result: MatchResultType<'container'>): TestResult[] => {
const handleDescribeBlockMatch = (
result: MatchResultType<'container'>,
matchGroup = true
): TestResult[] => {
const [t, matched, reason] = result;
if (matched.length === 0) {
return [];
}

t.addEvent(reason);
return matched.flatMap((a) => {
a.addEvent(reason);
const _matchContainers = (a: ContainerNode<TestAssertionStatus>) => {
onMatch(t, a, reason);
return matchContainers(t, a);
});
};
return matched.flatMap((a) =>
matchGroup ? a.getAll().flatMap(_matchContainers) : _matchContainers(a)
);
};

// match methods
const matchByNameOptions: MatchOptions = { ignoreGroupDiff: true };
const matchByLocationOptions: MatchOptions = {
checkIsWithin: true,
ignoreNonLiteralNameDiff: true,
ignoreGroupDiff: true,
};
const classicMatch = <C extends ContextType>(
type: ClassicMatchType,
tList: ChildNodeType<ItBlock, C>[],
aList: ChildNodeType<TestAssertionStatus, C>[]
): MatchMethodResult<C> => {
const reason = type === 'by-name' ? 'match-by-name' : 'match-by-location';
const options =
type === 'by-name' ? undefined : { checkIsWithin: true, ignoreNonLiteralNameDiff: true };
const options = type === 'by-name' ? matchByNameOptions : matchByLocationOptions;
const results: MatchResultType<C>[] = [];

const unmatchedT: ChildNodeType<ItBlock, C>[] = tList.filter((t) => {
Expand All @@ -236,15 +277,17 @@ const ContextMatch = (): ContextMatchAlgorithm => {
return { unmatchedT, results };
};

const matchByContextOptions: MatchOptions = {
acceptLocalNameMatch: true,
ignoreNonLiteralNameDiff: true,
};
const matchByContext = <C extends ContextType>(
tList: ChildNodeType<ItBlock, C>[],
aList: ChildNodeType<TestAssertionStatus, C>[]
): MatchMethodResult<C> => {
const results: MatchResultType<C>[] = [];
if (tList.length === aList.length) {
const hasMismatch = tList.find(
(t, idx) => !aList[idx].match(t, { ignoreNonLiteralNameDiff: true })
);
const hasMismatch = tList.find((t, idx) => !aList[idx].match(t, matchByContextOptions));
if (!hasMismatch) {
tList.forEach((t, idx) => results.push([t, [aList[idx]], 'match-by-context']));
return { unmatchedT: [], results };
Expand Down Expand Up @@ -304,14 +347,12 @@ const ContextMatch = (): ContextMatchAlgorithm => {
const matchResults: TestResult[] = [];

matchChildren('data', tContainer, aContainer, (result) =>
matchResults.push(...handleTestBlockMatch(result, false))
matchResults.push(...handleTestBlockMatch(result))
);
matchChildren('container', tContainer, aContainer, (result) =>
matchResults.push(...handleDescribeBlockMatch(result))
);

aContainer.group.forEach((c) => matchResults.push(...matchContainers(tContainer, c)));

return matchResults;
};

Expand All @@ -338,7 +379,7 @@ const ContextMatch = (): ContextMatchAlgorithm => {
return {};
}

let aList = aContainer.unmatchedNodes(type, { ungroup: true });
let aList = aContainer.unmatchedNodes(type);
const matched = (['by-name', 'by-location'] as ClassicMatchType[]).flatMap((matchType) => {
const matchResult = classicMatch(matchType, tList, aList);
tList = matchResult.unmatchedT;
Expand All @@ -357,10 +398,10 @@ const ContextMatch = (): ContextMatchAlgorithm => {
// handle unmatched container nodes
const cFallback = doMatch('container', (r) => {
const [t] = r;
return t.isMatched ? [] : handleDescribeBlockMatch(r);
return t.isMatched ? [] : handleDescribeBlockMatch(r, false);
});
// handle unmatched data nodes
const dFallback = doMatch('data', (r) => handleTestBlockMatch(r, false));
const dFallback = doMatch('data', (r) => handleTestBlockMatch(r, false, false));

return {
...dFallback,
Expand All @@ -369,7 +410,7 @@ const ContextMatch = (): ContextMatchAlgorithm => {
};

const toUnmatchedResults = (nodes: DataNode<ItBlock>[]): TestResult[] =>
nodes.flatMap((t) => handleTestBlockMatch([t, [], 'match-failed'], true));
nodes.flatMap((t) => handleTestBlockMatch([t, [], 'match-failed'], true, false));

const match = (
tContainer: ContainerNode<ItBlock>,
Expand Down Expand Up @@ -413,11 +454,11 @@ const { match } = ContextMatch();
export const matchTestAssertions = (
fileName: string,
sourceRoot: ParsedNode,
assertions: TestAssertionStatus[],
assertions: TestAssertionStatus[] | ContainerNode<TestAssertionStatus>,
verbose = false
): TestResult[] => {
const tContainer = buildSourceContainer(sourceRoot);
const aContainer = buildAssertionContainer(assertions);
const aContainer = Array.isArray(assertions) ? buildAssertionContainer(assertions) : assertions;

const messaging = createMessaging(fileName, verbose);
return match(tContainer, aContainer, messaging);
Expand Down
71 changes: 41 additions & 30 deletions src/TestResults/match-node.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
* internal classes used by `match-by-context`
*/

import { ParsedRange } from 'jest-editor-support';

type IsGroupType = 'yes' | 'no' | 'maybe';

const IsMatchedEvents = ['match-by-context', 'match-by-name', 'match-by-location'] as const;
Expand All @@ -19,6 +21,10 @@ export type MatchEvent =
export interface MatchOptions {
/** if true, will ignore name difference if both nodes have NonLiteral names */
ignoreNonLiteralNameDiff?: boolean;

// accept regular name, i.e. the name of the node, not the fullName, match.
acceptLocalNameMatch?: boolean;

/** if true, will ignore isGroupNode() difference */
ignoreGroupDiff?: boolean;
/** if true, will perform position check to see if "this" is enclosed within "other" node */
Expand Down Expand Up @@ -49,8 +55,8 @@ export interface OptionalAttributes {
fullName?: string;
isGroup?: IsGroupType;
nonLiteralName?: boolean;
// zero-based line range
range?: { start: number; end: number };
// zero-based location range
range?: ParsedRange;
}
export class BaseNode {
name: string;
Expand Down Expand Up @@ -123,8 +129,8 @@ export class BaseNode {
return false;
}
return (
this.attrs.range.start <= another.attrs.range.start &&
this.attrs.range.end >= another.attrs.range.end
this.attrs.range.start.line <= another.attrs.range.start.line &&
this.attrs.range.end.line >= another.attrs.range.end.line
);
}

Expand Down Expand Up @@ -181,21 +187,31 @@ export class BaseNode {
**/

match(other: BaseNode, options?: MatchOptions): boolean {
const posNotMatch = options?.checkIsWithin && !other.contains(this);
const matchedAsNonLiteralName =
// check position
if (options?.checkIsWithin && !other.contains(this)) {
return false;
}
// check name
const ignoreNameDiff =
options?.ignoreNonLiteralNameDiff &&
this.maybeNonLiteralName() &&
other.maybeNonLiteralName();
const nameNotMatch = this.fullName !== other.fullName && !matchedAsNonLiteralName;
const groupNotMatch =
!options?.ignoreGroupDiff &&
!(
this.isGroupNode() === 'maybe' ||
other.isGroupNode() === 'maybe' ||
this.isGroupNode() === other.isGroupNode()
);

return !(posNotMatch || nameNotMatch || groupNotMatch);
if (
this.fullName !== other.fullName &&
!ignoreNameDiff &&
!(options?.acceptLocalNameMatch && this.name === other.name)
) {
return false;
}

// check group
const ignoreGroupDiff =
options?.ignoreGroupDiff || this.isGroupNode() === 'maybe' || other.isGroupNode() === 'maybe';
if (this.isGroupNode() !== other.isGroupNode() && !ignoreGroupDiff) {
return false;
}

return true;
}

/**
Expand Down Expand Up @@ -230,9 +246,6 @@ export class DataNode<T> extends BaseNode {
}
}

export interface UnmatchedOptions {
ungroup?: boolean;
}
export type ContextType = 'container' | 'data';

export class ContainerNode<T> extends BaseNode {
Expand Down Expand Up @@ -307,6 +320,12 @@ export class ContainerNode<T> extends BaseNode {
.map((nodes) => nodes.find((n) => !n.hasEvent('invalid-location'))?.zeroBasedLine)
.filter((n) => n != null) as number[];
this.zeroBasedLine = topLines.length > 0 ? Math.min(...topLines) : -1;
if (!this.attrs.range) {
this.attrs.range = {
start: { line: this.zeroBasedLine, column: 0 },
end: { line: this.zeroBasedLine, column: 0 },
};
}
}
}

Expand Down Expand Up @@ -338,18 +357,10 @@ export class ContainerNode<T> extends BaseNode {
}
});
}

// extract all unmatched data node
public unmatchedNodes = <C extends ContextType>(
type: C,
options?: UnmatchedOptions
): ChildNodeType<T, C>[] => {
const unmatched = this.allChildNodes(type).filter((n) => !n.isMatched);

if (options?.ungroup) {
unmatched.forEach((u) => u.ungroup());
}
return unmatched;
};
public unmatchedNodes = <C extends ContextType>(type: C): ChildNodeType<T, C>[] =>
this.allChildNodes(type).filter((n) => !n.isMatched);
}

export type NodeType<T> = ContainerNode<T> | DataNode<T>;
Expand Down
Loading

0 comments on commit 47526da

Please sign in to comment.