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: enhancements #341

Merged
merged 13 commits into from
Feb 20, 2023
Merged

test: enhancements #341

merged 13 commits into from
Feb 20, 2023

Conversation

PaulRBerg
Copy link
Member

@PaulRBerg PaulRBerg commented Feb 16, 2023

Closes #317, #342, #346, and hopefully also #338.

Besides the above, this PR also includes a few other enhancements, such as the addition of an expectEmit function in the Base_Test test contract, which makes it possible to avoid writing the full vm.expectEmit({ checkTopic1: true, ... syntax.

Copy link
Member

@andreivladbrg andreivladbrg left a comment

Choose a reason for hiding this comment

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

Given that we are going to keep the cancelMultiple function in the core we have to test when the arrays count is zero.

In rest looks good to me, nice dryfy with expectEmit(), it was indeed visually annoying before.

@PaulRBerg
Copy link
Member Author

Yes - I will add a test for when the array count is zero in cancelMultiple.

@PaulRBerg
Copy link
Member Author

@andreivladbrg I have just pushed a few new commits to:

  • Upgrade to Solhint v3.4.0 (see my comment)
  • Turn on the new Solhint rule named-parameters-mapping
  • Add a test for when the array count is zero in cancelMultiple

Waiting for your review!

@andreivladbrg
Copy link
Member

Looks like it can be merged.

@PaulRBerg
Copy link
Member Author

Yeah, but before we merge, we should check what's going on with the e2e tests - I see that they fail with FatalExternalError. Is this due to Alchemy's PRC allowance?

@andreivladbrg
Copy link
Member

Is this due to Alchemy's PRC allowance?

Looks like this, you probably had too many requests that day.

test: add "USDCLike" and "USDTLike" mockups
test: fix input bounding in "createWithRange" tests
test: fix input bounding in linear e2e tests
test: rename "protocolContract" param to "sablierContract"
test: rename "vars.amounts" to "vars.createAmounts"
test: disable invariant call summaries
test: lower invariant test depth and no. of runs
test: improve wording in test modifier
chore: delete Solhint rule "reason-string"
chore: enable Solhint rule "named-parameters-mapping"
ci: explicitly set number of fuzz runs for unit tests
test: delete fuzz runs and invariant depth from lite profile
test: lower number of fuzz runs in default profile
@PaulRBerg PaulRBerg merged commit 9a0d591 into main Feb 20, 2023
@PaulRBerg PaulRBerg deleted the prb/enhancements branch March 3, 2023 22:33
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.

Fix the e2e tests failure due to "Blacklistable: account is blacklisted"
2 participants