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 swap() tests for Pool 9 #197

Merged
merged 10 commits into from
Dec 11, 2023
Merged

add swap() tests for Pool 9 #197

merged 10 commits into from
Dec 11, 2023

Conversation

uri-99
Copy link
Collaborator

@uri-99 uri-99 commented Dec 5, 2023

This PR solves #170
This PR shouldn't be merged before #194 , as it has its implementation. This PR won't pass CI tests without this.

pool_case,
array![*expected_cases[PANIC_CASE]],
array![*panic_swap_cases[PANIC_CASE]],
PRECISION
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
PRECISION
Zeroable::zero()

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done, i will push the same comment on test/swap for the already merged pools

Comment on lines +1823 to +1824
amount_to_swap =
IntegerTrait::<i256>::new(*swap_case.amount_specified.mag, true);
Copy link
Contributor

Choose a reason for hiding this comment

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

why this change is here? what does mean?

Copy link
Collaborator

Choose a reason for hiding this comment

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

This answer is really important because this function affects the whole test suite

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is because of PR #194 , this is the poolCase that triggered that fix, and it had to be tested somewhere relevant. This branch won't be merged until #194 is merged first

crates/yas_core/src/tests/utils/pool_9.cairo Outdated Show resolved Hide resolved
crates/yas_core/src/tests/utils/pool_9.cairo Outdated Show resolved Hide resolved
crates/yas_core/src/tests/utils/pool_9.cairo Outdated Show resolved Hide resolved
crates/yas_core/src/tests/utils/pool_9.cairo Outdated Show resolved Hide resolved
crates/yas_core/src/tests/utils/pool_9.cairo Outdated Show resolved Hide resolved
crates/yas_core/src/tests/utils/pool_9.cairo Outdated Show resolved Hide resolved
crates/yas_core/src/tests/utils/pool_9.cairo Outdated Show resolved Hide resolved
Comment on lines +1823 to +1824
amount_to_swap =
IntegerTrait::<i256>::new(*swap_case.amount_specified.mag, true);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This answer is really important because this function affects the whole test suite

@uri-99 uri-99 requested a review from dubzn December 6, 2023 20:05
@dubzn dubzn merged commit b47faaf into test/swap Dec 11, 2023
3 checks passed
@dubzn dubzn deleted the pool-9 branch December 11, 2023 13:39
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

4 participants