Add unordered matching to expectDiagnostics - #11710
Open
Vivek JM (vivekjm) wants to merge 2 commits into
Open
Conversation
Vivek JM (vivekjm)
requested review from
catalinaperalta,
iscai-msft,
Laurent Mazuel (lmazuel),
Mark Cowlishaw (markcowl) and
Timothee Guerin (timotheeguerin)
as code owners
August 18, 2026 06:59
|
Azure Pipelines: Successfully started running 1 pipeline(s). 1 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds an option to expectDiagnostics so tests can assert expected diagnostics without depending on emission order.
Changes:
- Introduces
fixedOrder(defaulttrue) to optionally match diagnostics regardless of order. - Adds an unordered matching implementation that supports overlapping expectations.
- Adds Vitest coverage and a changelog entry for the new option.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| packages/compiler/src/testing/expect.ts | Adds ExpectDiagnosticsOptions.fixedOrder and implements unordered matching logic. |
| packages/compiler/test/testing/expect.test.ts | Adds tests for ordered vs unordered matching behavior and strictness interactions. |
| .chronus/changes/unordered-diagnostic-expectations-2026-08-18.md | Documents the new fixedOrder option as a feature change. |
Suppressed comments (1)
packages/compiler/src/testing/expect.ts:254
strOrRegexMatchesintroduces a second (boolean-returning) implementation of string/regex matching alongside the existingmatchStrOrRegex. This duplication risks the two paths diverging over time (especially around regex edge cases) and makes it harder to reason about consistency between ordered and unordered matching. Consider refactoring somatchStrOrRegexdelegates tostrOrRegexMatches(using it as the single source of truth for matching semantics), and then only handles assertion/error-message concerns.
function strOrRegexMatches(value: string, expectation: string | RegExp): boolean {
if (typeof expectation === "string") {
return value === expectation;
}
const lastIndex = expectation.lastIndex;
const result = expectation.test(value);
expectation.lastIndex = lastIndex;
return result;
}
function matchStrOrRegex(value: string, expectation: string | RegExp, assertMessage: string) {
if (typeof expectation === "string") {
strictEqual(value, expectation, assertMessage);
} else {
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (3)
packages/compiler/src/testing/expect.ts:268
- Using fail(assertMessage) here reduces the actionable information in failures (no actual/expected values, and regex expectation formatting may be unclear). Consider including the actual value and the expectation in the thrown message, or re-introducing assert-based comparisons while preserving RegExp.lastIndex (save/restore around the check) so failures remain as informative as before.
function matchStrOrRegex(value: string, expectation: string | RegExp, assertMessage: string) {
if (!strOrRegexMatches(value, expectation)) {
fail(assertMessage);
}
}
packages/compiler/src/testing/expect.ts:87
- When strict is false, the failure case array.length > diagnostics.length is still enforced (which makes sense: you can’t satisfy more expectations than diagnostics), but the error text "Expected X diagnostics" can be misleading in non-strict mode. Consider adjusting the message in this branch to something like "Expected at least X diagnostics..." or explicitly mention that there are insufficient diagnostics to satisfy the expectations.
const strict = options.strict ?? true;
const fixedOrder = options.fixedOrder ?? true;
if ((strict && array.length !== diagnostics.length) || array.length > diagnostics.length) {
fail(
`Expected ${array.length} diagnostics but found ${diagnostics.length}:\n ${formatDiagnostics(
diagnostics,
)}`,
);
}
packages/compiler/src/testing/expect.ts:205
- The unordered matching implementation is a non-trivial bipartite-matching-style algorithm. Adding a brief comment describing the approach (augmenting paths), its intent (avoid greedy mismatch with overlapping expectations), and rough complexity would make future maintenance safer (especially if diagnostic counts grow in some tests).
function hasUnorderedMatch(
diagnostics: readonly Diagnostic[],
expectations: readonly DiagnosticMatch[],
): boolean {
const diagnosticMatches = new Array<number>(diagnostics.length).fill(-1);
function assignExpectation(expectationIndex: number, visited: boolean[]): boolean {
for (let diagnosticIndex = 0; diagnosticIndex < diagnostics.length; diagnosticIndex++) {
if (
visited[diagnosticIndex] ||
!diagnosticMatchesExpectation(diagnostics[diagnosticIndex], expectations[expectationIndex])
) {
continue;
}
visited[diagnosticIndex] = true;
const previousExpectation = diagnosticMatches[diagnosticIndex];
if (previousExpectation === -1 || assignExpectation(previousExpectation, visited)) {
diagnosticMatches[diagnosticIndex] = expectationIndex;
return true;
}
}
return false;
}
return expectations.every((_, index) =>
assignExpectation(index, new Array<boolean>(diagnostics.length).fill(false)),
);
}
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #5818.
expectDiagnosticscurrently compares each expected diagnostic with the item at the same array index. That makes multi-diagnostic tests fail when compiler changes only affect emission order.This adds an opt-in
fixedOrder: falsemode while retaining ordered, strict matching by default. Unordered matching uses one-to-one assignment rather than a greedy scan, so overlapping expectations such as a broad code match and a more specific message match still find a valid pairing. Existingstrict: falsebehavior continues to allow additional diagnostics.The change includes focused coverage for default ordering, reversed diagnostics, overlapping matches, non-strict matching, and strict count validation, plus the compiler change entry.
I used an AI coding assistant while preparing and validating this contribution.
Validation:
pnpm setup:minpnpm --filter @typespec/compiler buildpnpm --filter @typespec/compiler test(4,112 passed, 6 skipped)pnpm --filter @typespec/compiler lintpnpm exec prettier packages/compiler/src/testing/expect.ts packages/compiler/test/testing/expect.test.ts .chronus/changes/unordered-diagnostic-expectations-2026-08-18.md --checkpnpm gen-compiler-extern-signaturepnpm chronus verifygit diff --check