Feat: in given out for /custom-direct-quote #423
Conversation
e2e tests for exact amount out init
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
deividaspetraitis
force-pushed
the
SQS-303-e2e
branch
from
0369547
to
a5986d3
Compare
deividaspetraitis
force-pushed
the
SQS-303-e2e
branch
from
ee53b6e
to
0acd6dc
Compare
e2e tests for /custom-direct-quote
| start_time = time.time() | ||
| response = sqs_service.get_exact_amount_out_custom_direct_quote(token_out, denom_in, pool_id) | ||
| elapsed_time_ms = (time.time() - start_time) * 1000 | ||
|
|
||
| assert response.status_code == expected_status_code, f"Error: {response.text}" | ||
| assert expected_latency_upper_bound_ms > elapsed_time_ms, f"Error: latency {elapsed_time_ms} exceeded {expected_latency_upper_bound_ms} ms, denom in {denom_in} and token out {token_out}" | ||
|
|
||
| response_json = response.json() | ||
|
|
||
| print(response.text) |
There was a problem hiding this comment.
nit: I keep seeing this and similar boilerplate repeatedly. Is there a chance we could abstract it with a helper?
There was a problem hiding this comment.
Sure! It was abstracted in the following method.
| # Compute expected base out quote spot price | ||
| # First, get the USD price of each denom, and then divide to get the expected spot price | ||
| in_base_usd_quote_price = conftest.get_usd_price_scaled(denom_in) | ||
| out_base_usd_quote_price = conftest.get_usd_price_scaled(coin.denom) | ||
| expected_in_base_out_quote_price = out_base_usd_quote_price / in_base_usd_quote_price | ||
|
|
||
| # Compute expected token out | ||
| expected_token_in = int(coin.amount) * expected_in_base_out_quote_price |
There was a problem hiding this comment.
Would it be possible to create a helper to abstract this to reduce code duplication?
I think we could reuse that helper here
Similar abstraction could be added for "out given in"
There was a problem hiding this comment.
Moved such logic to calculate_expected_base_out_quote_spot_price methods for given out and given in respectively. 🚀
| pool_id = ','.join(map(str, optimal_quote.get_pool_ids())) | ||
| denoms_in = ','.join(map(str, optimal_quote.get_token_in_denoms())) | ||
|
|
||
| quote = self.run_quote_test(environment_url, token_out, denoms_in, pool_id, EXPECTED_LATENCY_UPPER_BOUND_MS) |
There was a problem hiding this comment.
Great to see that these tests are dynamic.
However, could we add the hardcoded tests similar to "out given in" still?
https://vscode.dev/github/osmosis-labs/sqs/blob/SQS-303-e2e/tests/test_router_quote_out_given_in.py#L101-L146
Let's then wire all hardcoded test cases into our synthetic monitoring suite here 🙏 :
https://vscode.dev/github/osmosis-labs/sqs/blob/SQS-303-e2e/tests/test_synthetic_geo.py#L18
The synthetic monitoring suite runs every 20 minutes on schedule against every environment. As a result, we need more determinism via hardcoding so that those are not flaky.
There was a problem hiding this comment.
Synth tests from now on includes test cases for given in/out quotes as well as for in/out custom direct quotes. 👍 I am not sure I get idea of "all" right, if not, would creating a separate task would make more sense? Given that we quite improved tests state in overall with this PR and by including all the rest tests to synh would be out of the scope of original task and would not provide enough visibility.
Break run_quote_test into smaller individual parts for reusability
Introduce calculate_expected_base_out_quote_spot_price
Add synthetic tests for /quote and /custom-direct-quote
* Feat: in given out for /custom-direct-quote * Feat: in given out for /custom-direct-quote Add humanDenoms support * Feat: in given out for /custom-direct-quote Update swagger * Feat: in given out for /custom-direct-quote Update docs * Feat: in given out for /custom-direct-quote Add docs for supported route types * Feat: in given out for /custom-direct-quote (#423) * Feat: in given out for /custom-direct-quote e2e tests for exact amount out init * fix merge error * WIP * WIP * Feat: in given out for /custom-direct-quote e2e tests for /custom-direct-quote * Feat: in given out for /custom-direct-quote Break run_quote_test into smaller individual parts for reusability * Feat: in given out for /custom-direct-quote Introduce calculate_expected_base_out_quote_spot_price * Feat: in given out for /custom-direct-quote Add synthetic tests for /quote and /custom-direct-quote * Update tests/route.py --------- Co-authored-by: Deividas Petraitis <[email protected]> * code rabbit --------- Co-authored-by: Roman <[email protected]>
This is a separate pull request from actual implemetation of in given out for
/custom-direct-quoteintroducing e2e tests for exact amount out/in custom quotes.Test results for
TestExactAmountOutDirectCustomQuote:Test results for
TestExactAmountInDirectCustomQuote:PR includes work for refactoring of quotes testing logic in order to reuse existing code to avoid duplication. Failures are not related directly with refactoring and this task and should be dealt seperately.
Test results for
TestExactAmountInQuote:Test results for
TestExactAmountOutQuote: