Skip to content

Conversation

@Han5991
Copy link

@Han5991 Han5991 commented Jan 29, 2026

This PR adds a proposal for enhancing expectFailure to support both custom messages (for reasoning) and validation (matching errors via regex/objects), addressing the needs discussed in nodejs/node#61563.

@vassudanagunta
Copy link

vassudanagunta commented Jan 29, 2026

@Han5991 this should reference nodejs/node#61570 which goes beyond nodejs/node#61563. It may end up being the case that a fix for the latter lands before the former, since the latter is relatively trivial and simply brings expectFailure in line with todo/skip.

--EDIT--

I'm a little confused. Your first proposal, #9, included a way to supply both a reason and an expected error as I proposed in nodejs/node#61570, but this one does not.

@Han5991
Copy link
Author

Han5991 commented Jan 29, 2026

@vassudanagunta

Sorry for the confusion. PR #9 was accidentally opened from my other account, so I closed it and opened this one instead.
I have updated this proposal to include the Object support (both reason and validation) as you suggested in #61570. Thanks for pointing that out!

@vassudanagunta
Copy link

Alternative syntaxes to consider for supplying both reason and expected error:

test('fails with reason and specific error', {
  expectFailure: 'Bug #123: Edge case behavior',
  expectFailureMessage: /err msg pattern/
}, () => ...);

or for consistency with assert.throws:

test('fails with reason and specific error', {
  expectFailure: 'Bug #123: Edge case behavior',
  expectFailureError: <RegExp> | <Function> | <Object> | <Error>
}, () => ...);

The user can supply either one of the two new options or both.

@vassudanagunta
Copy link

Consistency with assert.throws could be trivial to implement either by sharing code or by calling assert.throws in a try-catch to invert the effect.

@Han5991
Copy link
Author

Han5991 commented Jan 29, 2026

Thanks for the feedback!

Implementation:
Great suggestion! I plan to update the implementation to use assert.throws internally for validation. This will allow us to leverage all the existing validation capabilities (RegExp, Object, Class, custom function) without reinventing the wheel and keep the code cleaner.
API Syntax:
Regarding the flat options (expectFailureError), I lean towards keeping the nested object structure { expectFailure: { with: ..., message: ... } }. It keeps all failure expectations grouped under a single property and seems more extensible if we need to add more configuration in the future, without polluting the top-level options namespace.

@vassudanagunta
Copy link

the terseness and legibility of with+message is very nice. as i said on your PR, it gets my top vote.

Update the `expectFailure` enhancements proposal based on feedback and implementation alignment:
     - Consolidate validation logic under the `with` property within an object.
     - Remove direct RegExp support in favor of the object syntax for consistency.
     - Specify usage of `assert.throws` for robust error validation.
     - Document alternatives considered (flat options).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants