Skip to content

Comments

chore: HubPool tests migration#1237

Merged
tbwebb22 merged 44 commits intomasterfrom
taylor/tests-migration
Jan 22, 2026
Merged

chore: HubPool tests migration#1237
tbwebb22 merged 44 commits intomasterfrom
taylor/tests-migration

Conversation

@tbwebb22
Copy link
Contributor

@tbwebb22 tbwebb22 commented Jan 12, 2026

Migrates HubPool tests from Hardhat to Foundry
Closes ACP-20

grasphoper and others added 23 commits December 11, 2025 19:15
Signed-off-by: Ihor Farion <ihor@umaproject.org>
Signed-off-by: Ihor Farion <ihor@umaproject.org>
Signed-off-by: Ihor Farion <ihor@umaproject.org>
Signed-off-by: Ihor Farion <ihor@umaproject.org>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
@linear
Copy link

linear bot commented Jan 15, 2026

Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
@socket-security
Copy link

socket-security bot commented Jan 15, 2026

No dependency changes detected. Learn more about Socket for GitHub.

👍 No dependency changes detected in pull request

tbwebb22 and others added 2 commits January 19, 2026 19:01
tbwebb22 and others added 3 commits January 20, 2026 14:35
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
…dToken test

Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This test had a dependency on constructSimpleTree from the deleted HubPool.ExecuteRootBundle test, so moved that function into here for now

Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
@tbwebb22 tbwebb22 marked this pull request as ready for review January 20, 2026 23:49
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a bit lost RE whether this is needed or if a master merge will fix this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah this change certainly isn't intentional from my end

I think if we get this one merged #1262
and then merge master into here, should fix this right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yea I think so

Copy link
Contributor

Choose a reason for hiding this comment

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

merged

grasphoper
grasphoper previously approved these changes Jan 21, 2026
Copy link
Contributor

@grasphoper grasphoper left a comment

Choose a reason for hiding this comment

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

just 1 nit

// Verify LP token was created (using mock factory, so just verify it's a valid ERC20)
MintableERC20 lpTokenContract = MintableERC20(lpToken);
assertTrue(bytes(lpTokenContract.symbol()).length > 0, "LP token should have a symbol");
assertTrue(bytes(lpTokenContract.name()).length > 0, "LP token should have a name");
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we also verify the exact name here like the hardhat test was doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
fusmanii
fusmanii previously approved these changes Jan 22, 2026
Copy link
Contributor

@fusmanii fusmanii left a comment

Choose a reason for hiding this comment

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

Nice work! One minor feedback from claude

1. CLAUDE.md Documentation Mismatch - The doc says FOUNDRY_PROFILE=local but the actual profile name is local-test. This will cause all      
  revert-message tests to fail if someone follows the docs literally.

Signed-off-by: Taylor Webb <tbwebb22@gmail.com>
@tbwebb22
Copy link
Contributor Author

Nice work! One minor feedback from claude

1. CLAUDE.md Documentation Mismatch - The doc says FOUNDRY_PROFILE=local but the actual profile name is local-test. This will cause all      
  revert-message tests to fail if someone follows the docs literally.

Good catch! Fixed bd1bda0

@tbwebb22 tbwebb22 merged commit 1b2bfd6 into master Jan 22, 2026
11 checks passed
@tbwebb22 tbwebb22 deleted the taylor/tests-migration branch January 22, 2026 16:04
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