Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test(testFunctions): display stringified arguments in error message #2442

Merged

Conversation

pano9000
Copy link
Contributor

@pano9000 pano9000 commented Aug 25, 2024

Hello,

the error message that the testFunctions currently throws, when a test case fails is currently not correctly displaying options objects, even though the intention clearly is there.

This pull request properly implements this now, by stringifying the arguments, making sure they can get correctly printed as string.

Before
Error: validator.isDate("2024/05/01", [Object object]) passed but should have failed
After:
Error: validator.isDate("2024/05/01", {"delimiters":["/"," "],"format":"YYYY/MM/DD","strictMode":false}) passed but should have failed

This is achieved by a tiny helper function that maps through the args and JSON.stringifys them.

All of these are available since node v0.10, so we still are compatible with that (ancient :)) version:
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/map#browser_compatibility
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/stringify

I would've like to replace the "format" function with template literals as well, but these were not available in node 0.10.0 (which is the minimum engine mentioned in the package.json) yet, but only came with node 4.

Checklist

  • PR contains only changes related; no stray files, etc.
  • README updated (where applicable)
  • Tests written (where applicable)
  • References provided in PR (where applicable)

this now shows the actual passed arguments,
instead of [Object object] in case a test with a options object fails
Copy link
Member

@WikiRik WikiRik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. Maybe you can do an example commit (in this PR and reverting it afterwards or in a separate temporary PR) with a failing test so we can check that it works well on CI?

@pano9000
Copy link
Contributor Author

ok, will try to do this in a separate PR, that we can then just delete

@pano9000
Copy link
Contributor Author

@WikiRik kindly check #2444 - works as expected in the CI as well :-)

@rubiin rubiin merged commit 96ff3b2 into validatorjs:master Aug 25, 2024
10 checks passed
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.

3 participants