Skip to content

Conversation

@juanarbol
Copy link
Member

The assert.match method by default will throws if there is no match,
this change can add the function of instead of throwing or not, returns
the boolean result of match.

Fixes: #33869

Note: I'm going to leave the documentation as a TODO :)

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

The `assert.match` method by default will throws if there is no match,
this change can add the function of instead of throwing or not, returns
the boolean result of match.

Fixes: nodejs#33869
@nodejs-github-bot nodejs-github-bot added the assert Issues and PRs related to the assert subsystem. label Sep 9, 2020
@addaleax
Copy link
Member

addaleax commented Sep 9, 2020

I think this effectively just gives you the same result as regexp.test(string), as it is currently implemented, and in that case regexp.test(string) should definitely be preferred, I think.

I would suggest either closing #33869 as wontfix – I agree with the concern voiced by the author that this would be unexpected for an assertion method – or following @lundibundi’s suggestion there and making all the underlying assertion mechanisms available as separate methods, not just assert.match

@juanarbol
Copy link
Member Author

I think this effectively just gives you the same result as regexp.test(string), as it is currently implemented, and in that case regexp.test(string) should definitely be preferred, I think.

:O haha, this is true...

I would suggest either closing #33869 as wontfix – I agree with the concern voiced by the author that this would be unexpected for an assertion method – or following @lundibundi’s suggestion there and making all the underlying assertion mechanisms available as separate methods, not just assert.match

That really make sense to my.

@juanarbol
Copy link
Member Author

Closing this, this is almost like regexp.test(string).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

assert Issues and PRs related to the assert subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

assert.match should return match result

4 participants