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

feat(test): toHaveBeenNthCalledWith + improve some fail messages #7320

Merged
merged 2 commits into from
Nov 26, 2023

Conversation

james-elicx
Copy link
Contributor

What does this PR do?

  • Implement jest matcher toHaveBeenNthCalledWith.
  • Updates that it's supported in the docs.
  • Improve fail messages for toHaveBeenCalled + toHaveBeenCalledTimes so that they provide more info that jest failures provide.

.

  • Documentation or TypeScript types (it's okay to leave the rest blank in this case)
  • Code changes

How did you verify your code works?

I wrote automated tests

  • I included a test for the new code, or existing tests cover it
  • I ran my tests locally and they pass (bun-debug test test-file-name.test)

If Zig files changed:

  • I checked the lifetime of memory allocated to verify it's (1) freed and (2) only freed when it should be
  • I included a test for the new code, or an existing test covers it
  • JSValue used outside outside of the stack is either wrapped in a JSC.Strong or is JSValueProtect'ed
  • I wrote TypeScript/JavaScript tests and they pass locally (bun-debug test test-file-name.test)

If new methods, getters, or setters were added to a publicly exposed class:

  • I added TypeScript types for the new methods, getters, or setters

Copy link
Collaborator

@Jarred-Sumner Jarred-Sumner left a comment

Choose a reason for hiding this comment

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

Thank you

@@ -3389,8 +3379,8 @@ pub const Expect = struct {
return .zero;
}

if (arguments.len < 1 or !arguments[0].isAnyInt()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Does isAnyInt fail for 0?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No. The reason I changed it from isAnyInt to isUInt32AsAnyInt was so that it would fail for negative numbers since this matcher is only supposed to accept an argument that is >=0. Jest throws a matcher error for the arg type saying that it expects a non-negative argument, so this aligns the behaviour with how it works in Jest.

@Jarred-Sumner Jarred-Sumner merged commit 1840de3 into oven-sh:main Nov 26, 2023
18 of 29 checks passed
@james-elicx james-elicx deleted the feat/expect/nth-called-with branch November 27, 2023 06:03
cirospaciari pushed a commit that referenced this pull request Nov 27, 2023
…7320)

* feat(test): `toHaveBeenNthCalledWith` + improve some fail messages

* [autofix.ci] apply automated fixes

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
ryoppippi pushed a commit to ryoppippi/bun that referenced this pull request Feb 1, 2024
…ven-sh#7320)

* feat(test): `toHaveBeenNthCalledWith` + improve some fail messages

* [autofix.ci] apply automated fixes

---------

Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com>
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