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

add coverage for registrar and registry #12988

Merged
merged 2 commits into from May 2, 2024
Merged

add coverage for registrar and registry #12988

merged 2 commits into from May 2, 2024

Conversation

shileiwill
Copy link
Contributor

@shileiwill shileiwill commented Apr 26, 2024

More in AUTO-10143

registerUpkeep in registrar

Existing:

testLink_autoApproveOff_happy
testLink_autoApproveOn_happy

testUSDToken_autoApproveOff_happy
testUSDToken_autoApproveOn_happy

testNative_autoApproveOn_happy
testNative_autoApproveOff_msgValue0
testNative_autoApproveOff_msgValueNot0

testLink_autoApproveOff_revertOnDuplicateEntry

New:
testLink_revertOnInsufficientPayment
testLink_revertOnInvalidAdminAddress
testLink_revertOnInvalidBillingToken

approve in registrar
New:
testUSDToken_happy

cancel in registrar
New:
testUSDToken_happy

AddFunds in registry is very well convered, just updated the name:
testNative_msgValue0: this covers Native
testNative_msgValueNot0: this covers Native
test_RevertsWhen_NativePaymentDoesntMatchBillingToken
test_RevertsWhen_UpkeepDoesNotExist
test_RevertsWhen_UpkeepIsCanceled
test_anyoneCanAddFunds
test_movesFundFromCorrectToken: this covers LINK and USD
test_emitsAnEvent

@shileiwill shileiwill marked this pull request as ready for review April 26, 2024 04:19
@shileiwill shileiwill requested a review from a team as a code owner April 26, 2024 04:19
@shileiwill shileiwill changed the title add coverage for registerUpkeep in registrar add coverage for registrar and addFunds in registry Apr 29, 2024
@shileiwill shileiwill changed the title add coverage for registrar and addFunds in registry add coverage for registrar and registry Apr 29, 2024
Copy link
Contributor

@RyanRHall RyanRHall left a comment

Choose a reason for hiding this comment

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

Tests all look good! Are there any corresponding hardhat tests we can remove or are these all new codepaths?


uint256 startRegistrarBalance = usdToken18.balanceOf(address(registrar));

// approve the upkeep
Copy link
Contributor

Choose a reason for hiding this comment

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

typo - "cancel the upkeep"

Comment on lines 59 to 63
uint256 endRegistrarBalance = usdToken18.balanceOf(address(registrar));
assertEq(startRegistrarBalance - amount, endRegistrarBalance);
Copy link
Contributor

Choose a reason for hiding this comment

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

can we also test that the $$ is sent back to the admin?

Copy link
Contributor

@RyanRHall RyanRHall left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@shileiwill shileiwill enabled auto-merge May 2, 2024 17:13
@shileiwill shileiwill added this pull request to the merge queue May 2, 2024
@cl-sonarqube-production
Copy link

Quality Gate passed Quality Gate passed

Issues
0 New issues
0 Fixed issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
No data about Duplication

See analysis details on SonarQube

@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 2, 2024
@shileiwill shileiwill added this pull request to the merge queue May 2, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 2, 2024
@shileiwill shileiwill added this pull request to the merge queue May 2, 2024
github-merge-queue bot pushed a commit that referenced this pull request May 2, 2024
* add coverage for registry and registrar

* address comments
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 2, 2024
@shileiwill shileiwill added this pull request to the merge queue May 2, 2024
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks May 2, 2024
@shileiwill shileiwill added this pull request to the merge queue May 2, 2024
Merged via the queue into develop with commit 5633b51 May 2, 2024
110 checks passed
@shileiwill shileiwill deleted the unittests branch May 2, 2024 21:03
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.

None yet

2 participants