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

[CI] update coin paprika for CI #306

Merged
merged 2 commits into from
Jul 3, 2023
Merged

[CI] update coin paprika for CI #306

merged 2 commits into from
Jul 3, 2023

Conversation

bh2smith
Copy link
Contributor

We can only query coin paprika back one year -- had to update a test.

@bh2smith bh2smith added the easy label Jun 23, 2023
@bh2smith bh2smith requested a review from a team June 23, 2023 07:16
Copy link
Contributor

@gentrexha gentrexha left a comment

Choose a reason for hiding this comment

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

I know this fixes this issue and LGTM. But, we really shouldn't be relying on external APIs in our unit tests. Also, we have to update this test in 6 months, again.

I suggest we use the mock decorator from the unittest library. This is all that would be needed to fix this permanently:

from unittest import TestCase, mock

class TestSplitTransfers(TestCase):
    # Existing code...

    @mock.patch('src.models.split_transfers.token_in_eth')
    def test_process_with_overdraft_exceeding_eth_not_cow(self, mock_token_in_eth):
        eth_amount = 2 * ONE_ETH
        cow_reward = 100_000 * ONE_ETH
        slippage_amount = -3 * ONE_ETH
        accounting = self.construct_split_transfers_and_process(
            solvers=[self.solver],
            eth_amounts=[eth_amount],
            cow_rewards=[cow_reward],
            slippage_amounts=[slippage_amount],
            redirects=self.redirect_map,
        )
        
        # Configure the mock to return the desired value
        cow_deduction = cow_reward - 25369802491025623613440 or whatever value is correct here
        mock_token_in_eth.return_value = cow_deduction
        
        # Assertions and test logic...

@bh2smith
Copy link
Contributor Author

bh2smith commented Jul 3, 2023

Created an issue for real fix.

#308

@bh2smith bh2smith merged commit 0c85a9d into main Jul 3, 2023
@bh2smith bh2smith deleted the ci-update branch July 3, 2023 11:31
@github-actions github-actions bot locked and limited conversation to collaborators Jul 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants