Skip to content

Commit 2676386

Browse files
authored
feat(assertions): improve printing of match failures (#23453)
Debugging match failures with the `assertions` library used to be a bit frustrating. The library used to guess the single closest matching resource and print it fully, with a list of match failures underneath. This has two problems: - Matching the failures up to the data structure requires a lot of scanning back and forth between different screen lines. - The library could incorrectly guess at the closest match, giving you useless information. This change is a first (and by no means the last) stab at improving the situation: - Mismatch errors are printed in-line with the data structure, making it very clear what they are referencing (removing the need for scanning). - Fields in complex data structures that aren't involved in the mismatch at all are collapsed. For example, a complex subobject that matches be printed as `{ ... }` so it doesn't take up too much vertical space. - Because we now have a lot of vertical space left, we can print the N closest matches, lowering the chance that we guess wrong and leave you without information to correct the test. The last point can probably be improved more in the future, by coming up with a more accurate metric for "closest match" than "failure count", but this will do for now. ---- *By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license*
1 parent e13495d commit 2676386

File tree

21 files changed

+845
-249
lines changed

21 files changed

+845
-249
lines changed

packages/@aws-cdk/assertions/lib/match.ts

+100-31
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
import { Matcher, MatchResult } from './matcher';
22
import { AbsentMatch } from './private/matchers/absent';
3+
import { sortKeyComparator } from './private/sorting';
4+
import { SparseMatrix } from './private/sparse-matrix';
35
import { getType } from './private/type';
46

57
/**
@@ -196,18 +198,53 @@ class ArrayMatch extends Matcher {
196198
message: `Expected type array but received ${getType(actual)}`,
197199
});
198200
}
199-
if (!this.subsequence && this.pattern.length !== actual.length) {
200-
return new MatchResult(actual).recordFailure({
201+
202+
return this.subsequence ? this.testSubsequence(actual) : this.testFullArray(actual);
203+
}
204+
205+
private testFullArray(actual: Array<any>): MatchResult {
206+
const result = new MatchResult(actual);
207+
208+
let i = 0;
209+
for (; i < this.pattern.length && i < actual.length; i++) {
210+
const patternElement = this.pattern[i];
211+
const matcher = Matcher.isMatcher(patternElement)
212+
? patternElement
213+
: new LiteralMatch(this.name, patternElement, { partialObjects: this.partialObjects });
214+
215+
const innerResult = matcher.test(actual[i]);
216+
result.compose(`${i}`, innerResult);
217+
}
218+
219+
if (i < this.pattern.length) {
220+
result.recordFailure({
201221
matcher: this,
202-
path: [],
203-
message: `Expected array of length ${this.pattern.length} but received ${actual.length}`,
222+
message: `Not enough elements in array (expecting ${this.pattern.length}, got ${actual.length})`,
223+
path: [`${i}`],
204224
});
205225
}
226+
if (i < actual.length) {
227+
result.recordFailure({
228+
matcher: this,
229+
message: `Too many elements in array (expecting ${this.pattern.length}, got ${actual.length})`,
230+
path: [`${i}`],
231+
});
232+
}
233+
234+
return result;
235+
}
236+
237+
private testSubsequence(actual: Array<any>): MatchResult {
238+
const result = new MatchResult(actual);
239+
240+
// For subsequences, there is a lot of testing and backtracking that happens
241+
// here, keep track of it all so we can report in a sensible amount of
242+
// detail on what we did if the match happens to fail.
206243

207244
let patternIdx = 0;
208245
let actualIdx = 0;
246+
const matches = new SparseMatrix<MatchResult>();
209247

210-
const result = new MatchResult(actual);
211248
while (patternIdx < this.pattern.length && actualIdx < actual.length) {
212249
const patternElement = this.pattern[patternIdx];
213250

@@ -216,30 +253,59 @@ class ArrayMatch extends Matcher {
216253
: new LiteralMatch(this.name, patternElement, { partialObjects: this.partialObjects });
217254

218255
const matcherName = matcher.name;
219-
if (this.subsequence && (matcherName == 'absent' || matcherName == 'anyValue')) {
256+
if (matcherName == 'absent' || matcherName == 'anyValue') {
220257
// array subsequence matcher is not compatible with anyValue() or absent() matcher. They don't make sense to be used together.
221258
throw new Error(`The Matcher ${matcherName}() cannot be nested within arrayWith()`);
222259
}
223260

224261
const innerResult = matcher.test(actual[actualIdx]);
262+
matches.set(patternIdx, actualIdx, innerResult);
225263

226-
if (!this.subsequence || !innerResult.hasFailed()) {
227-
result.compose(`[${actualIdx}]`, innerResult);
264+
actualIdx++;
265+
if (innerResult.isSuccess) {
266+
result.compose(`${actualIdx}`, innerResult); // Record any captures
228267
patternIdx++;
229-
actualIdx++;
230-
} else {
231-
actualIdx++;
232268
}
233269
}
234270

235-
for (; patternIdx < this.pattern.length; patternIdx++) {
236-
const pattern = this.pattern[patternIdx];
237-
const element = (Matcher.isMatcher(pattern) || typeof pattern === 'object') ? ' ' : ` [${pattern}] `;
238-
result.recordFailure({
239-
matcher: this,
240-
path: [],
241-
message: `Missing element${element}at pattern index ${patternIdx}`,
242-
});
271+
// If we haven't matched all patterns:
272+
// - Report on each one that did match on where it matched (perhaps it was wrong)
273+
// - Report the closest match for the failing one
274+
if (patternIdx < this.pattern.length) {
275+
// Succeeded Pattern Index
276+
for (let spi = 0; spi < patternIdx; spi++) {
277+
const foundMatch = matches.row(spi).find(([, r]) => r.isSuccess);
278+
if (!foundMatch) { continue; } // Should never fail but let's be defensive
279+
280+
const [index] = foundMatch;
281+
282+
result.compose(`${index}`, new MatchResult(actual[index]).recordFailure({
283+
matcher: this,
284+
message: `arrayWith pattern ${spi} matched here`,
285+
path: [],
286+
cost: 0, // This is an informational message so it would be unfair to assign it cost
287+
}));
288+
}
289+
290+
const failedMatches = matches.row(patternIdx);
291+
failedMatches.sort(sortKeyComparator(([i, r]) => [r.failCost, i]));
292+
if (failedMatches.length > 0) {
293+
const [index, innerResult] = failedMatches[0];
294+
result.recordFailure({
295+
matcher: this,
296+
message: `Could not match arrayWith pattern ${patternIdx}. This is the closest match`,
297+
path: [`${index}`],
298+
cost: 0, // Informational message
299+
});
300+
result.compose(`${index}`, innerResult);
301+
} else {
302+
// The previous matcher matched at the end of the pattern and we didn't even get to try anything
303+
result.recordFailure({
304+
matcher: this,
305+
message: `Could not match arrayWith pattern ${patternIdx}. No more elements to try`,
306+
path: [`${actual.length}`],
307+
});
308+
}
243309
}
244310

245311
return result;
@@ -288,8 +354,8 @@ class ObjectMatch extends Matcher {
288354
if (!(a in this.pattern)) {
289355
result.recordFailure({
290356
matcher: this,
291-
path: [`/${a}`],
292-
message: 'Unexpected key',
357+
path: [a],
358+
message: `Unexpected key ${a}`,
293359
});
294360
}
295361
}
@@ -299,16 +365,16 @@ class ObjectMatch extends Matcher {
299365
if (!(patternKey in actual) && !(patternVal instanceof AbsentMatch)) {
300366
result.recordFailure({
301367
matcher: this,
302-
path: [`/${patternKey}`],
303-
message: `Missing key '${patternKey}' among {${Object.keys(actual).join(',')}}`,
368+
path: [patternKey],
369+
message: `Missing key '${patternKey}'`,
304370
});
305371
continue;
306372
}
307373
const matcher = Matcher.isMatcher(patternVal) ?
308374
patternVal :
309375
new LiteralMatch(this.name, patternVal, { partialObjects: this.partial });
310376
const inner = matcher.test(actual[patternKey]);
311-
result.compose(`/${patternKey}`, inner);
377+
result.compose(patternKey, inner);
312378
}
313379

314380
return result;
@@ -324,35 +390,38 @@ class SerializedJson extends Matcher {
324390
};
325391

326392
public test(actual: any): MatchResult {
327-
const result = new MatchResult(actual);
328393
if (getType(actual) !== 'string') {
329-
result.recordFailure({
394+
return new MatchResult(actual).recordFailure({
330395
matcher: this,
331396
path: [],
332397
message: `Expected JSON as a string but found ${getType(actual)}`,
333398
});
334-
return result;
335399
}
336400
let parsed;
337401
try {
338402
parsed = JSON.parse(actual);
339403
} catch (err) {
340404
if (err instanceof SyntaxError) {
341-
result.recordFailure({
405+
return new MatchResult(actual).recordFailure({
342406
matcher: this,
343407
path: [],
344408
message: `Invalid JSON string: ${actual}`,
345409
});
346-
return result;
347410
} else {
348411
throw err;
349412
}
350413
}
351414

352415
const matcher = Matcher.isMatcher(this.pattern) ? this.pattern : new LiteralMatch(this.name, this.pattern);
353416
const innerResult = matcher.test(parsed);
354-
result.compose(`(${this.name})`, innerResult);
355-
return result;
417+
if (innerResult.hasFailed()) {
418+
innerResult.recordFailure({
419+
matcher: this,
420+
path: [],
421+
message: 'Encoded JSON value does not match',
422+
});
423+
}
424+
return innerResult;
356425
}
357426
}
358427

0 commit comments

Comments
 (0)