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

Accept Typed object in matcher #4715

Merged
merged 18 commits into from
Jan 31, 2024

Conversation

RenanSouza2
Copy link
Contributor

@RenanSouza2 RenanSouza2 commented Dec 26, 2023

Resolves #4716

  • Because this PR includes a bug fix, relevant tests have been included.

This PR expands the reccently added feature of accepting multiple forms of address in the macthers

It now accpets objects created by ethers.Typed.address()

Copy link

changeset-bot bot commented Dec 26, 2023

🦋 Changeset detected

Latest commit: 72bf9f6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@nomicfoundation/hardhat-chai-matchers Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Dec 26, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
hardhat ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jan 31, 2024 11:12am

@fvictorio fvictorio merged commit e661947 into NomicFoundation:main Jan 31, 2024
15 checks passed
@fvictorio
Copy link
Member

Thanks @RenanSouza2, and sorry for taking so long to get this merged. I found a problem with one of our custom linting rules when I reviewed it (#4786), so we wanted to get that fixed first.

@fvictorio
Copy link
Member

Ah, also: I removed one of the tests, because I think it was just encoding a limitation (that 2 and typed 2 cannot be compared). Since that's more of a lack of a feature than a decision, I think it shouldn't be represented in a test. But please let me know if you disagree!

@RenanSouza2
Copy link
Contributor Author

Thanks @RenanSouza2, and sorry for taking so long to get this merged. I found a problem with one of our custom linting rules when I reviewed it (#4786), so we wanted to get that fixed first.

No problems about that :)

Ah, also: I removed one of the tests, because I think it was just encoding a limitation (that 2 and typed 2 cannot be compared). Since that's more of a lack of a feature than a decision, I think it shouldn't be represented in a test. But please let me know if you disagree!

That test was to assure it was not rejecting all other Typed objects.

At first I used the ethers dereference function dereference directly and that function throws an error when you pass a different type other than the one stored at the typed object

that test had a use but I don't see that much of a deal removing it

@RenanSouza2 RenanSouza2 deleted the accept-Typed-matcher branch January 31, 2024 11:44
@fvictorio
Copy link
Member

Hmm, I'm not sure I understand. This was the test:

  it("should accept other typed objects", async function () {
    expect(() => {
      expect(2).to.equal(ethers.Typed.uint256(2));
    }).to.throw("expected 2 to equal");
  });

But I would actually expect that assertion to pass, not to throw. I mean: asserting that expect(2).to.equal(Typed.uint256(2)) fails seems to be testing that something that ideally would work doesn't actually work. Does that make sense?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Hardhat matchers don't accept all kinds of AddressLike
2 participants