feat(compiler): add ignoreOrder option to expectDiagnostics (closes #5818)#10484
feat(compiler): add ignoreOrder option to expectDiagnostics (closes #5818)#10484Jah-yee wants to merge 3 commits intomicrosoft:mainfrom
Conversation
Jah-yee
requested review from
AlitzelMendez,
bterlson,
chrisradek,
markcowl,
timotheeguerin and
witemple-msft
as code owners
microsoft-github-policy-service
Bot
added
the
compiler:core
| options = { | ||
| options: { | ||
| /** | ||
| * When true, diagnostics are sorted before comparison so that order differences |
There was a problem hiding this comment.
that doc doesn't seem correct, would be good to use a named type also instead of inline
| * When true, diagnostics are sorted by code+message before comparison so that | ||
| * order does not matter. | ||
| */ | ||
| fixedOrder?: boolean; |
There was a problem hiding this comment.
i think the name here is confusing as it would from a user of this function make you think it does the opposite. The fact that it sort is an implementation details, maybe just ignoreOrder?
|
❌ There is undocummented changes. Run The following packages have changes but are not documented.
Show changes |
|
@timotheeguerin - addressed both feedback points:
Latest commit: c95b4f4 ✅ Let me know if you'd like anything else adjusted! Warmly, |
Jah-yee
force-pushed
the
feat/expectDiagnostics-fixedOrder
branch
from
b0454a0 to
e0a93f5
Compare
Jah-yee
changed the title
| diagnostics: readonly Diagnostic[], | ||
| match: DiagnosticMatch | DiagnosticMatch[], | ||
| options = { | ||
| options: { |
There was a problem hiding this comment.
Thanks for the feedback! Both issues are now fixed in the latest push:\n\n1. Naming: Renamed fixedOrder → ignoreOrder — you're right that this is clearer and avoids the "fixed" = "must be fixed in order" confusion.\n\n2. Doc fix: The strict option doc was indeed wrong — it checks exact count match, not sorting. Fixed to: "When true, the number of diagnostics must match exactly."\n\nThe updated code is at: https://github.com/microsoft/TypeSpec/pull/10484/commits/e0a93f52f9c410189318dca57ef009b8b89a9f8e\n\nThanks again for the review!
…crosoft#5818) Adds a new `fixedOrder` option to the `expectDiagnostics` function that sorts diagnostics by code+message before comparison. This allows tests to remain stable when diagnostic order changes due to code churn, without requiring library test authors to manually sort or wrap results. Example usage: ```typescript expectDiagnostics(diags, [{ code: "bar" }, { code: "foo" }], { fixedOrder: true, }); ```
Jah-yee
force-pushed
the
feat/expectDiagnostics-fixedOrder
branch
from
e0a93f5 to
8a8a337
Compare
Respond to review feedback from timotheeguerin: - Extract inline options type into named ExpectDiagnosticsOptions interface - Improve JSDoc: clarify strict (default true) and ignoreOrder semantics - Add @param JSDoc for match and options parameters - Export ExpectDiagnosticsOptions from testing index Ref: microsoft#10484
Adds a new `ignoreOrder` option to the `expectDiagnostics` testing helper that sorts diagnostics by code+message before comparison. This allows tests to remain stable when diagnostic order changes due to code churn, without requiring library test authors to manually sort or wrap results. Changes: - Extract inline options type into named ExpectDiagnosticsOptions interface - Improve JSDoc: clarify strict (default true) and ignoreOrder semantics - Rename fixedOrder to ignoreOrder based on review feedback - Export ExpectDiagnosticsOptions from testing index Ref: microsoft#10484, closes microsoft#5818
Respond to review feedback from timotheeguerin: - Extract inline options type into named ExpectDiagnosticsOptions interface - Improve JSDoc: clarify strict (default true) and ignoreOrder semantics - Add @param JSDoc for match and options parameters - Export ExpectDiagnosticsOptions from testing index Ref: microsoft#10484
Respond to review feedback from timotheeguerin: - Extract inline options type into named ExpectDiagnosticsOptions interface - Improve JSDoc: clarify strict (default true) and ignoreOrder semantics - Add @param JSDoc for match and options parameters - Export ExpectDiagnosticsOptions from testing index Ref: microsoft#10484
2 participants
Good day
Summary
Implements the feature requested in issue #5818: adds an
ignoreOrderoption to theexpectDiagnosticstesting helper.Problem
Code churn can easily cause diagnostics to appear in changing orders over time. When this happens, tests using
expectDiagnosticsfail even though the actual diagnostic content is unchanged. Library test authors had to manually wrap or pre-sort diagnostics.Solution
Added an
ignoreOrder?: booleanoption toexpectDiagnostics:ignoreOrder: true, both the actual and expected arrays are sorted bycode|severity|messagebefore comparison, so order no longer matters.ignoreOrder: falsepreserves existing behavior.Changes
ignoreOrder?: booleanto theoptionsparameterignoreOrderis true, bothdiagnosticsandarrayare sorted before comparisonTesting
The implementation is backward-compatible (default
ignoreOrder: falsepreserves existing behavior) and follows the existing code patterns in the file.Thank you for maintaining TypeSpec!
Best,
Jah-yee