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(bridge): test TokenVault sendERC20 function #473

Merged
merged 13 commits into from
Jan 4, 2023

Conversation

RogerLamTd
Copy link
Contributor

No description provided.

@RogerLamTd RogerLamTd self-assigned this Dec 22, 2022
@codecov
Copy link

codecov bot commented Dec 22, 2022

Codecov Report

Merging #473 (4158422) into claimTokens (c513734) will increase coverage by 1.32%.
The diff coverage is n/a.

@@               Coverage Diff               @@
##           claimTokens     #473      +/-   ##
===============================================
+ Coverage        65.91%   67.24%   +1.32%     
===============================================
  Files              109      109              
  Lines             2940     2940              
  Branches           355      355              
===============================================
+ Hits              1938     1977      +39     
+ Misses             926      886      -40     
- Partials            76       77       +1     
Flag Coverage Δ *Carryforward flag
bridge-ui 95.05% <ø> (ø) Carriedforward from a3ba5a8
protocol 60.28% <ø> (+2.60%) ⬆️
relayer 69.10% <ø> (ø) Carriedforward from a3ba5a8
ui 100.00% <ø> (ø) Carriedforward from a3ba5a8

*This pull request uses carry forward flags. Click here to find out more.

Impacted Files Coverage Δ
...es/protocol/contracts/thirdparty/LibMerkleTrie.sol 90.36% <0.00%> (+7.22%) ⬆️
...protocol/contracts/thirdparty/ERC20Upgradeable.sol 61.44% <0.00%> (+10.84%) ⬆️
packages/protocol/contracts/bridge/TokenVault.sol 60.86% <0.00%> (+34.78%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@RogerLamTd RogerLamTd marked this pull request as ready for review January 4, 2023 05:20
[bridgedToken.address]: weth,
});

await helpers.setStorageAt(bridgedToken.address, 203, 1000000);
Copy link
Member

@davidtaikocha davidtaikocha Jan 4, 2023

Choose a reason for hiding this comment

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

If smock.setVariable works, i think we should change all the getSlot and setStorageAt to smock.setVariable in all tests? Since these setStorageAt with a hardcode slot number is really non-trivial to maintain, eveytime anyone changes the contract's code, their tests may fail and u may need to update this 203 or other slots number to a new value manually?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can go through and change that, should I make a separate PR or do it for all tests using setStorageAt on this branch?

Copy link
Member

@davidtaikocha davidtaikocha Jan 4, 2023

Choose a reason for hiding this comment

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

Lets make a separate PR that targets to this PR's branch?

@RogerLamTd RogerLamTd changed the base branch from main to claimTokens January 4, 2023 09:30
@vercel
Copy link

vercel bot commented Jan 4, 2023

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

Name Status Preview Updated
bridge-ui 🔄 Building (Inspect) Jan 4, 2023 at 9:32AM (UTC)

@dantaik dantaik merged commit d5ca2a4 into claimTokens Jan 4, 2023
@dantaik dantaik deleted the tokenvaulttests branch January 4, 2023 11:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
3 participants