SQS-375 | Unit test createFormattedLimitOrder #492
Conversation
Introduces unit tests for Orderbook usecase createFormattedLimitOrder method.
WalkthroughThe changes introduce a mock implementation of the Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant OrderbookUseCase
participant OrderbookRepositoryMock
Client->>OrderbookUseCase: CreateFormattedLimitOrder(params)
OrderbookUseCase->>OrderbookRepositoryMock: StoreTicks(poolID, ticksMap)
OrderbookRepositoryMock-->>OrderbookUseCase: Ticks stored
OrderbookUseCase-->>Client: Return LimitOrder
Tip New featuresWalkthrough comment now includes:
Notes:
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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- domain/mocks/orderbook_repository_mock.go (1 hunks)
- orderbook/usecase/export_test.go (1 hunks)
- orderbook/usecase/orderbook_usecase.go (1 hunks)
- orderbook/usecase/orderbook_usecase_test.go (1 hunks)
Additional comments not posted (16)
orderbook/usecase/export_test.go (1)
7-15: LGTM!The function correctly forwards the call to
createFormattedLimitOrder.The code changes are approved.
domain/mocks/orderbook_repository_mock.go (4)
17-23: LGTM!The method is correctly implemented.
The code changes are approved.
26-31: LGTM!The method is correctly implemented.
The code changes are approved.
34-39: LGTM!The method is correctly implemented.
The code changes are approved.
42-47: LGTM!The method is correctly implemented.
The code changes are approved.
orderbook/usecase/orderbook_usecase_test.go (10)
37-52: LGTM!The test case is correctly implemented.
The code changes are approved.
53-67: LGTM!The test case is correctly implemented.
The code changes are approved.
69-84: LGTM!The test case is correctly implemented.
The code changes are approved.
86-104: LGTM!The test case is correctly implemented.
The code changes are approved.
106-123: LGTM!The test case is correctly implemented.
The code changes are approved.
126-146: LGTM!The test case is correctly implemented.
The code changes are approved.
149-171: LGTM!The test case is correctly implemented.
The code changes are approved.
173-196: LGTM!The test case is correctly implemented.
The code changes are approved.
199-225: LGTM!The test case is correctly implemented.
The code changes are approved.
227-279: LGTM!The test case is correctly implemented.
The code changes are approved.
orderbook/usecase/orderbook_usecase.go (1)
48-50: Verify the impact of changing the return type.The return type of the
Newfunction has been changed frommvc.OrderBookUsecaseto*orderbookUseCaseImpl. This change can improve performance but may affect other parts of the codebase that expect the interface type.Run the following script to verify the usage of the
Newfunction and ensure that all occurrences are compatible with the new return type:Verification successful
No direct issues found with the return type change.
The change in the return type of the
Newfunction frommvc.OrderBookUsecaseto*orderbookUseCaseImpldoes not appear to cause any direct issues in theapp/sidecar_query_server.gofile. However, ensure that any functions receivingorderBookUseCasedo not expect an interface type elsewhere in the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `New` function and ensure compatibility with the new return type. # Test: Search for the function usage. Expect: No incompatible occurrences. rg --type go -A 5 $'orderbookusecase.New'Length of output: 931
Script:
#!/bin/bash # Description: Check if `orderBookUseCase` is used as an `mvc.OrderBookUsecase` in `app/sidecar_query_server.go`. # Test: Search for type assertions or interface implementations related to `orderBookUseCase`. rg --type go 'orderBookUseCase' -A 10 app/sidecar_query_server.goLength of output: 1049
Implements requested improvements
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- orderbook/usecase/orderbook_usecase.go (2 hunks)
- orderbook/usecase/orderbook_usecase_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- orderbook/usecase/orderbook_usecase_test.go
Additional comments not posted (2)
orderbook/usecase/orderbook_usecase.go (2)
285-287: Added validation forplacedQuantity.The addition of a validation check to ensure that
placedQuantityis neither zero nor negative enhances the robustness of the order creation process.The code changes are approved.
48-48: Return type change inNewfunction.The change from returning an interface to a concrete type could improve performance by reducing interface overhead. However, ensure that all consumers of this function are updated to handle the new return type.
The code changes are approved.
Run the following script to verify the impact on consumers:
Verification successful
Return type change successfully integrated.
The change in the return type of the
Newfunction inorderbook/usecase/orderbook_usecase.gohas been successfully integrated into the codebase. The consumers of this function have been updated to handle the new return type without issues.
- The
Newfunction is used inapp/sidecar_query_server.goand aligns with the new return type.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all consumers of the `New` function are updated to handle the new return type. # Test: Search for the function usage. Expect: Only occurrences of the new return type. rg --type go -A 5 $'New'Length of output: 347556
Implements requested improvements: Define error messages as types
Implements requested improvements: Define error messages as types
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- orderbook/types/errors.go (1 hunks)
- orderbook/usecase/orderbook_usecase.go (6 hunks)
- orderbook/usecase/orderbook_usecase_test.go (1 hunks)
Files skipped from review due to trivial changes (1)
- orderbook/usecase/orderbook_usecase_test.go
Additional comments not posted (3)
orderbook/types/errors.go (1)
1-133: LGTM!The
errors.gofile is well-structured and follows the Go conventions for defining custom error types. It covers a comprehensive set of error scenarios related to the orderbook package, providing meaningful error messages with relevant details.The custom error types are defined consistently, each with its own struct and
Error()method implementation. The error messages include the problematic values and the underlying error (if any), which helps in debugging and understanding the error context.Overall, the file is clean, organized, and serves its purpose effectively.
orderbook/usecase/orderbook_usecase.go (2)
294-295: LGTM!The validation check for
placedQuantityis a good addition to prevent the submission of invalid orders.
266-269: LGTM!The improved error handling enhances the clarity and specificity of error reporting by replacing generic error messages with specific error types from the
typespackage.Also applies to: 277-280, 288-291, 300-303, 312-316, 324-327, 332-335, 340-343, 348-351, 358-361, 374-374, 380-380, 386-386, 403-406
* SQS-377 | Unit test ProcessPool Introduces unit tests for Orderbook usecase ProcessPool method.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- domain/mocks/orderbook_grpc_client_mock.go (2 hunks)
- domain/orderbook/grpcclient/orderbook_grpc_client.go (7 hunks)
- orderbook/types/errors.go (1 hunks)
- orderbook/usecase/export_test.go (1 hunks)
- orderbook/usecase/orderbook_usecase.go (14 hunks)
- orderbook/usecase/orderbook_usecase_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- orderbook/types/errors.go
Additional comments not posted (12)
orderbook/usecase/export_test.go (1)
8-16: LGTM!Exporting the unexported
createFormattedLimitOrderfunction for testing purposes is a good practice. The function signature matches the unexported function and the implementation simply calls the unexported function and returns the result, which is the expected behavior.domain/mocks/orderbook_grpc_client_mock.go (4)
18-18: LGTM!The code changes are approved.
20-20: LGTM!The code changes are approved.
47-53: LGTM!The code changes are approved.
63-69: LGTM!The code changes are approved.
domain/orderbook/grpcclient/orderbook_grpc_client.go (2)
25-30: LGTM!The new method
FetchTickUnrealizedCancelsis well-documented and implemented. It follows the documented behavior of fetching unrealized cancels in chunks, handling errors, and checking for mismatches in the number of unrealized cancels fetched.Also applies to: 88-113
35-40: LGTM!The new method
FetchTicksis well-documented and implemented. It follows the documented behavior of fetching ticks in chunks, handling errors, and checking for mismatches in the number of ticks fetched.Also applies to: 124-149
orderbook/usecase/orderbook_usecase.go (3)
298-299: LGTM: Added validation check forplacedQuantity.The added validation check ensures that the
placedQuantityis greater than zero, preventing the submission of invalid orders. This strengthens the order creation process and helps maintain data integrity.
270-273: LGTM: Improved error handling using specific error types.The error handling for various parsing operations has been improved by replacing generic error messages with specific error types from the
typespackage. This enhances the clarity and specificity of error reporting, making it easier to diagnose and handle different types of errors.Also applies to: 281-284, 292-295, 304-307, 316-320, 328-331, 336-339, 344-347, 352-355, 362-365, 378-378, 384-384, 390-390, 407-410
49-49: Verify the impact of the return type change.The return type of the
Newfunction has been modified from the interface typemvc.OrderBookUsecaseto a pointer to the concrete type*OrderbookUseCaseImpl. This change indicates a shift towards a more specific implementation and may improve performance by reducing interface overhead. However, it's important to ensure that all consumers of this function are adapted to handle the concrete type instead of the interface.Run the following script to verify the usage of the
Newfunction:Verification successful
Return type change successfully integrated.
The return type of the
Newfunction has been changed to a pointer to the concrete type*OrderbookUseCaseImpl. The usages inorderbook/usecase/orderbook_usecase_test.goandapp/sidecar_query_server.goindicate that the consumers have been adapted to handle this concrete type directly, with no type assertions or interface assignments observed. This confirms that the change has been successfully integrated into the codebase.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all usages of the `New` function are adapted to the concrete type. # Test: Search for the function usage. Expect: No type assertions or interface assignments. rg --type go -A 5 $'orderbookusecase\.New'Length of output: 1401
orderbook/usecase/orderbook_usecase_test.go (2)
35-222: LGTM!The
TestProcessPooltest suite covers a comprehensive set of scenarios, including error cases and successful processing. The use of table-driven tests and mocks enhances maintainability and readability. The test cases are well-structured and cover the important aspects of theProcessPoolmethod.
223-528: LGTM!The
TestCreateFormattedLimitOrdertest suite is comprehensive and covers various scenarios for theCreateFormattedLimitOrdermethod. The test cases handle different error conditions, such as tick not found, parsing errors, and overflow checks. They also verify the correctness of the formatted limit order in the successful case.The use of table-driven tests and mocks improves the maintainability and readability of the code. The test cases are well-structured and cover the important aspects of the
CreateFormattedLimitOrdermethod.
Implements requested improvements: Parse quantity as osmomath.Dec
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (4)
- domain/orderbook/order.go (2 hunks)
- orderbook/types/errors.go (1 hunks)
- orderbook/usecase/orderbook_usecase.go (13 hunks)
- orderbook/usecase/orderbook_usecase_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- orderbook/types/errors.go
- orderbook/usecase/orderbook_usecase_test.go
Additional comments not posted (16)
domain/orderbook/order.go (7)
80-80: LGTM!The change to
osmomath.Decfor theQuantityfield is approved as it enhances the precision of numerical representations in the order book.
83-83: LGTM!The change to
osmomath.Decfor thePlacedQuantityfield is approved as it enhances the precision of numerical representations in the order book.
85-85: LGTM!The change to
osmomath.Decfor thePricefield is approved as it enhances the precision of numerical representations in the order book.
86-86: LGTM!The change to
osmomath.Decfor thePercentClaimedfield is approved as it enhances the precision of numerical representations in the order book.
87-87: LGTM!The change to
osmomath.Decfor theTotalFilledfield is approved as it enhances the precision of numerical representations in the order book.
88-88: LGTM!The change to
osmomath.Decfor thePercentFilledfield is approved as it enhances the precision of numerical representations in the order book.
91-91: LGTM!The change to
osmomath.Decfor theOutputfield is approved as it enhances the precision of numerical representations in the order book.orderbook/usecase/orderbook_usecase.go (9)
17-17: LGTM!The code change is approved.
24-24: LGTM!The code change is approved.
32-32: LGTM!The code change is approved.
48-48: LGTM!The code change is approved.
59-59: LGTM!The code change is approved.
Line range hint
64-87: LGTM!The code changes are approved.
137-137: LGTM!The code changes are approved.
200-200: LGTM!The code changes are approved.
Line range hint
259-431: LGTM!The code changes are approved.
Implements requested improvements: Parse ETA`s as osmomath.Dec
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- orderbook/usecase/orderbook_usecase.go (13 hunks)
- orderbook/usecase/orderbook_usecase_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- orderbook/usecase/orderbook_usecase_test.go
Additional comments not posted (20)
orderbook/usecase/orderbook_usecase.go (20)
17-17: LGTM!The import statement for the
typespackage is valid and necessary.
24-24: LGTM!The struct name change to
OrderbookUseCaseImplfollows the Go naming convention.
32-32: LGTM!The interface assertion is correctly updated to match the struct name change.
48-48: LGTM!The return type change to
*OrderbookUseCaseImplis valid and may have performance benefits by reducing interface overhead.
49-49: LGTM!The struct initialization is correctly updated to match the struct name change.
58-58: LGTM!The method declaration is correctly updated to match the struct name change.
63-63: LGTM!The method declaration is correctly updated to match the struct name change.
65-67: LGTM!The error handling block for the nil
poolcase is valid and necessary to prevent potential nil pointer dereferences and provide a specific error type.
71-71: LGTM!The error handling block for the nil
CosmWasmPoolModelcase is valid and necessary to prevent potential nil pointer dereferences and provide a specific error type.
76-76: LGTM!The error handling block for the non-orderbook pool case is valid and necessary to ensure that the pool is an orderbook pool and provide a specific error type.
87-87: LGTM!The error handling block for the failed type assertion case is valid and necessary to ensure that the underlying pool is of the expected type and provide a specific error type.
99-99: LGTM!The error handling block for the error case while fetching ticks is valid and necessary to handle potential errors and provide a specific error type with relevant information.
105-105: LGTM!The error handling block for the error case while fetching unrealized cancels is valid and necessary to handle potential errors and provide a specific error type with relevant information.
114-114: LGTM!The error handling block for the tick ID mismatch case is valid and necessary to ensure the consistency of tick IDs and provide a specific error type with relevant information.
119-119: LGTM!The error handling block for the tick ID mismatch case is valid and necessary to ensure the consistency of tick IDs and provide a specific error type with relevant information.
137-137: LGTM!The method declaration is correctly updated to match the struct name change.
200-200: LGTM!The method declaration is correctly updated to match the struct name change.
258-260: LGTM!The variable declaration for
zeroDecis valid and the comment provides a clear explanation of its purpose. Defining the zero decimal value in a global space avoids creating a new instance every time, which can improve performance.
263-263: LGTM!The method declaration is correctly updated to match the struct name change.
273-276: LGTM!The error handling block for the case where the tick for the order is not found is valid and necessary to handle the scenario and provide a specific error type with relevant information.
There was a problem hiding this comment.
LGTM
Could we address this one dangling comment pre-merge?
#492 (comment)
* SQS-377 | Unit test ProcessPool Introduces unit tests for Orderbook usecase ProcessPool method. * SQS-378 | WIP * SQS-377 | Unit test ProcessPool Adds requested improvements * SQS-377 | Unit test ProcessPool Add cosmwasmpool import * SQS-378 | Test GetActiveOrders Formatting * SQS-378 | Test GetActiveOrders Adds several more test cases * SQS-378 | Test GetActiveOrders Implements tests for GetActiveOrders orderbook usecase method * SQS-378 | Test GetActiveOrders * SQS-378 | Test GetActiveOrders Implements requested changes * SQS-378 | Test GetActiveOrders Cleanup * SQS-379 | unit tests for processOrderBookActiveOrders (#499) * SQS-379 | processOrderBookActiveOrders Implements tests for processOrderBookActiveOrders orderbook usecase method. As a side effect introduces some helper methods and refactors some test cases.
There was a problem hiding this comment.
Actionable comments posted: 2
Outside diff range, codebase verification and nitpick comments (1)
Makefile (1)
38-38: Unrecognized--pdoption inswag initcommandThe
--pdoption used in theswag initcommand within the Makefile is not recognized as a standard option in Swaggo's documentation, and there is no custom implementation found in the codebase. Please verify if this option is intended or correct it if it is a typo. The.swaggofile for overrides is correctly specified and exists.Analysis chain
Verify the
--pdoption and the overrides file path.The
--pdoption in theswag initcommand is not recognized in standard Swaggo documentation. Please confirm if this is a custom option specific to your project. Additionally, ensure that the.swaggofile exists at the specified path and is correctly formatted to be used as an overrides file.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for the existence of the .swaggo file and its content. cat ./.swaggoLength of output: 207
Script:
#!/bin/bash # Search for any custom implementation or alias of the `swag` command that includes the `--pd` option. rg 'swag.*--pd'Length of output: 98
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (21)
- .swaggo (1 hunks)
- Makefile (1 hunks)
- docs/docs.go (6 hunks)
- docs/swagger.json (6 hunks)
- docs/swagger.yaml (5 hunks)
- domain/errors.go (1 hunks)
- domain/mocks/orderbook_grpc_client_mock.go (2 hunks)
- domain/mocks/orderbook_usecase_mock.go (1 hunks)
- domain/mocks/pools_usecase_mock.go (2 hunks)
- domain/pools.go (1 hunks)
- orderbook/types/errors.go (1 hunks)
- orderbook/types/get_orders_request.go (1 hunks)
- orderbook/usecase/export_test.go (1 hunks)
- orderbook/usecase/orderbook_usecase.go (16 hunks)
- orderbook/usecase/orderbook_usecase_test.go (1 hunks)
- orderbook/usecase/orderbooktesting/parsing/active_orders_response.json (1 hunks)
- orderbook/usecase/orderbooktesting/suite.go (1 hunks)
- passthrough/delivery/http/passthrough_handler.go (3 hunks)
- passthrough/delivery/http/passthrough_handler_test.go (1 hunks)
- pools/delivery/http/pools_handler.go (1 hunks)
- router/usecase/routertesting/suite.go (2 hunks)
Files skipped from review due to trivial changes (2)
- .swaggo
- pools/delivery/http/pools_handler.go
Files skipped from review as they are similar to previous changes (4)
- domain/mocks/orderbook_grpc_client_mock.go
- orderbook/usecase/export_test.go
- orderbook/usecase/orderbook_usecase.go
- orderbook/usecase/orderbook_usecase_test.go
Additional comments not posted (28)
orderbook/usecase/orderbooktesting/parsing/active_orders_response.json (1)
1-53: Well-formed JSON structure for testing.The JSON structure is comprehensive and well-formed, suitable for testing the active orders response. It includes detailed fields that are essential for thorough testing of the orderbook functionality.
orderbook/types/get_orders_request.go (2)
14-37: Clear and robust validation logic.The addition of specific error messages for different types of validation failures enhances the clarity and robustness of the API. The validation logic for the
UserOsmoAddressis concise and effectively uses thesdk.AccAddressFromBech32function to ensure the address's validity.
20-20: Verify impact of removing pagination parameters.The removal of
LimitandCursorfields simplifies the request structure. However, verify the impact on client-side implementations and ensure that this change is clearly communicated to API consumers.passthrough/delivery/http/passthrough_handler_test.go (2)
22-28: Well-structured test suite initialization.The test suite is properly initialized using the
suite.Runfunction, which is a standard practice for running tests in Go using thetestify/suitepackage. This ensures that all methods defined with theTestprefix are run as part of the test suite.
30-117: Comprehensive test cases forGetActiveOrders.The test cases cover a variety of scenarios including validation errors, successful retrieval of active orders, and handling of internal server errors. This comprehensive coverage is crucial for ensuring the robustness of the
GetActiveOrdersmethod.
- Validation Error Handling: The test correctly anticipates a
BadRequeststatus when no query parameters are provided, which aligns with the expected behavior for missing required fields.- Successful Data Retrieval: The test for successful retrieval of active orders mocks the expected behavior and uses a file to compare the expected JSON response, which is a good practice for ensuring the response structure is maintained.
- Error Handling: The test for internal server errors properly mocks a failure scenario in the use case, ensuring the handler correctly handles and responds with an
InternalServerError.Overall, the tests are well-structured and effectively use mocking to isolate the handler's functionality, which is key in unit testing.
passthrough/delivery/http/passthrough_handler.go (1)
Line range hint
64-101: Well-documented and robust implementation ofGetActiveOrders.The method
GetActiveOrdersis well-documented with clear annotations that describe the API endpoint, expected responses, and parameters. This documentation is crucial for API usability and clarity.
- Error Handling: The method uses
domain.ResponseErrorfor error responses, which centralizes error handling and ensures consistency across different parts of the application. This is a significant improvement in maintaining code quality and reducing redundancy.- Request Validation: The method includes request validation logic, which is essential for ensuring that the incoming requests meet the expected criteria before processing.
- Trace Handling: The use of OpenTelemetry for tracing and error recording is a good practice, especially for debugging and monitoring the application in production environments.
Overall, the method is implemented with a clear focus on error handling, request validation, and documentation, which are all best practices in API development.
domain/pools.go (1)
70-85: Robust validation logic inCanonicalOrderBooksResult.Validate.The addition of the
Validatemethod to theCanonicalOrderBooksResultstruct enhances the robustness of the data handling by ensuring that all critical fields are checked before the object is used further in the application. This method checks for empty strings and zero values, which are common sources of bugs in data processing.
- Field Validations: Each field is appropriately validated against specific criteria, such as non-empty strings and non-zero values. This is crucial for preventing invalid data from propagating through the system.
- Error Handling: The method returns custom errors for each type of validation failure, which aids in debugging and maintaining the system by providing clear error messages.
This method is a good example of defensive programming and helps ensure the integrity of the data within the application.
orderbook/types/errors.go (1)
9-220: Approved: Comprehensive and well-defined custom error types for orderbook operations.The new file
errors.gointroduces a variety of custom error types that enhance the error handling capabilities within the orderbook domain. Each error type is well-defined and implements the error interface consistently, providing clear and informative error messages that are specific to various failure scenarios encountered in orderbook operations.This approach not only improves the maintainability of the code by segregating error handling but also aids in debugging and error tracking during runtime.
orderbook/usecase/orderbooktesting/suite.go (1)
15-208: Approved: Well-structured test suite helper for the orderbook use case.The file
suite.gointroduces a comprehensive test suite helper for the orderbook use case, featuring various helper methods and default objects. These additions significantly simplify the setup and execution of unit tests by providing reusable components and clear, concise helper methods.This structure not only enhances the readability and maintainability of the test code but also promotes consistency and efficiency in test implementation.
domain/errors.go (4)
21-21: Approved: Clear error message for empty base denomination.The addition of
ErrBaseDenomNotValidwith the message "base denom is empty" is clear and effectively communicates the specific validation failure.
22-22: Approved: Clear error message for empty quote denomination.The addition of
ErrQuoteDenomNotValidwith the message "quote denom is empty" is appropriate and clearly indicates the validation failure when the quote denomination is not provided.
23-23: Approved: Clear error message for invalid pool ID.The addition of
ErrPoolIDNotValidwith the message "pool ID is zero" effectively communicates the validation failure when a pool ID is not properly specified.
24-24: Approved: Clear error message for empty contract address.The addition of
ErrContractAddressNotValidwith the message "contract address is empty" is clear and effectively communicates the specific validation failure when a contract address is not provided.router/usecase/routertesting/suite.go (1)
462-470: Approved: Robust error handling method added.The addition of the
ErrorIsAsmethod in theRouterTestHelperstruct enhances error assertions by effectively usingerrors.Isanderrors.Asto check for exact and compatible error conditions, respectively.docs/swagger.yaml (8)
13-17: Approved: Addition ofdomain.ResponseErrordefinition.This new structure standardizes error responses across the API, enhancing clarity and consistency in error handling.
38-44: Approved: Addition ofgithub_com_cosmos_cosmos-sdk_types.Coindefinition.This definition is essential for clearly representing monetary values within the API, ensuring structured data handling for amounts and denominations.
45-49: Approved: Addition ofgithub_com_osmosis-labs_sqs_domain_orderbook.Assetdefinition.This definition standardizes the representation of assets within the API, which is crucial for financial and trading applications.
50-90: Approved: Comprehensive addition ofgithub_com_osmosis-labs_sqs_domain_orderbook.LimitOrderdefinition.This definition is critical for the API's trading functionality, providing a clear and structured representation of limit orders with detailed properties.
91-104: Approved: Addition ofgithub_com_osmosis-labs_sqs_domain_orderbook.OrderStatusenum.This enum is essential for clearly tracking and managing the status of orders within the API.
105-135: Approved: Addition of variousgithub_com_osmosis-labs_sqs_domain_passthroughdefinitions.These definitions are crucial for representing detailed financial data within the API, enhancing the portfolio management features.
136-144: Approved: Addition ofgithub_com_osmosis-labs_sqs_orderbook_types.GetActiveOrdersResponsedefinition.This definition is important for structuring responses related to active orders, ensuring clarity and accessibility of the data.
181-207: Approved: Addition of new endpoints/passthrough/active-ordersand/passthrough/portfolio-assets/{address}.These endpoints are crucial for the API's functionality, allowing users to retrieve detailed financial data related to orders and assets.
Also applies to: 225-229
docs/swagger.json (3)
9-45: Comprehensive addition of the/passthrough/active-ordersendpoint.The addition of the
/passthrough/active-ordersendpoint is well-documented with a clear description, parameters, and response schemas. The use of$reffor schema references ensures consistency and reusability of the defined models. This is a good practice as it helps maintain the schema definitions in one place, making the API easier to update and maintain.
67-73: Ensure consistency in schema references.The schema references for
PortfolioAssetsResultandCanonicalOrderBooksResultare used correctly to ensure type safety and clarity in the API documentation. This approach helps in maintaining a clean and manageable codebase by avoiding redundancy and enhancing readability.Also applies to: 145-145
Line range hint
468-702: Review of new definitions added to the Swagger configuration.The new definitions such as
ResponseError,LimitOrder,OrderStatus, and others are well-structured and provide a clear and detailed schema for the API. Each definition is equipped with appropriate properties and types, enhancing the API's robustness and usability.
ResponseError: Properly defined with a message property.LimitOrderandOrderStatus: These definitions are crucial for the new/passthrough/active-ordersendpoint and are detailed with relevant properties.It's important to ensure that these new definitions are consistently used across the API wherever applicable to maintain uniformity.
docs/docs.go (3)
18-54: Well-documented addition of the/passthrough/active-ordersendpoint in Go documentation.The Go documentation for the new endpoint
/passthrough/active-ordersmirrors the Swagger JSON configuration accurately. The detailed description, parameters, and response schemas are consistent and correctly implemented in the Go template string. This ensures that the generated Swagger documentation will be accurate and informative.
76-82: Consistent implementation of schema references in Go documentation.The schema references for
PortfolioAssetsResultandCanonicalOrderBooksResultare implemented consistently in the Go documentation as seen in the Swagger JSON. This consistency is crucial for maintaining the integrity and accuracy of the API documentation across different implementations.Also applies to: 154-154
Line range hint
477-711: Review of new definitions added to the Go documentation.The new definitions such as
ResponseError,LimitOrder,OrderStatus, and others are correctly implemented in the Go documentation. The structure and properties match those defined in the Swagger JSON configuration, ensuring consistency across the documentation sources.
ResponseError: Defined with a message property, aligning with the Swagger JSON.LimitOrderandOrderStatus: Detailed with appropriate properties and types, crucial for the new endpoint documentation.These definitions are essential for ensuring that the generated Swagger documentation is robust and user-friendly.
| // OrderbookUsecaseMock is a mock implementation of the RouterUsecase interface | ||
| type OrderbookUsecaseMock struct { | ||
| ProcessPoolFunc func(ctx context.Context, pool sqsdomain.PoolI) error | ||
| GetAllTicksFunc func(poolID uint64) (map[int64]orderbookdomain.OrderbookTick, bool) | ||
| GetActiveOrdersFunc func(ctx context.Context, address string) ([]orderbookdomain.LimitOrder, bool, error) | ||
| } | ||
|
|
||
| func (m *OrderbookUsecaseMock) ProcessPool(ctx context.Context, pool sqsdomain.PoolI) error { | ||
| if m.ProcessPoolFunc != nil { | ||
| return m.ProcessPoolFunc(ctx, pool) | ||
| } | ||
| panic("unimplemented") | ||
| } | ||
|
|
||
| func (m *OrderbookUsecaseMock) GetAllTicks(poolID uint64) (map[int64]orderbookdomain.OrderbookTick, bool) { | ||
| if m.GetAllTicksFunc != nil { | ||
| return m.GetAllTicksFunc(poolID) | ||
| } | ||
| panic("unimplemented") | ||
| } | ||
|
|
||
| func (m *OrderbookUsecaseMock) GetActiveOrders(ctx context.Context, address string) ([]orderbookdomain.LimitOrder, bool, error) { | ||
| if m.GetActiveOrdersFunc != nil { | ||
| return m.GetActiveOrdersFunc(ctx, address) | ||
| } | ||
| panic("unimplemented") | ||
| } |
There was a problem hiding this comment.
Consider enhancing panic messages for clarity.
The mock implementation uses a common pattern of function pointers for flexible testing. However, consider enhancing the panic messages to include the name of the unimplemented method for easier debugging.
For example:
- panic("unimplemented")
+ panic("ProcessPool method is unimplemented")Committable suggestion was skipped due to low confidence.
| GetAllPoolsFunc func() ([]sqsdomain.PoolI, error) | ||
| GetPoolsFunc func(opts ...domain.PoolsOption) ([]sqsdomain.PoolI, error) | ||
| StorePoolsFunc func(pools []sqsdomain.PoolI) error | ||
| GetRoutesFromCandidatesFunc func(candidateRoutes sqsdomain.CandidateRoutes, tokenInDenom, tokenOutDenom string) ([]route.RouteImpl, error) | ||
| GetTickModelMapFunc func(poolIDs []uint64) (map[uint64]*sqsdomain.TickModel, error) | ||
| GetPoolFunc func(poolID uint64) (sqsdomain.PoolI, error) | ||
| GetPoolSpotPriceFunc func(ctx context.Context, poolID uint64, takerFee osmomath.Dec, quoteAsset, baseAsset string) (osmomath.BigDec, error) | ||
| GetCosmWasmPoolConfigFunc func() domain.CosmWasmPoolRouterConfig | ||
| CalcExitCFMMPoolFunc func(poolID uint64, exitingShares osmomath.Int) (sdk.Coins, error) | ||
| GetAllCanonicalOrderbookPoolIDsFunc func() ([]domain.CanonicalOrderBooksResult, error) |
There was a problem hiding this comment.
Approved: Addition of GetAllCanonicalOrderbookPoolIDsFunc enhances mock flexibility.
The addition of GetAllCanonicalOrderbookPoolIDsFunc to PoolsUsecaseMock is consistent with the existing structure of the mock and enhances its flexibility by allowing specific behavior to be injected for testing. This change is well-integrated and follows the established pattern of optional function implementations within the mock.
Consider adding documentation for GetAllCanonicalOrderbookPoolIDsFunc to explain its purpose and usage, which will aid future developers in understanding the mock's capabilities.
Would you like me to help draft the documentation for this new function?
Also applies to: 44-46
Requested changes: cover test cases with different addresses
|
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- orderbook/usecase/orderbook_usecase_test.go (1 hunks)
Files skipped from review as they are similar to previous changes (1)
- orderbook/usecase/orderbook_usecase_test.go
* SQS-375 | Unit test createFormattedLimitOrder Introduces unit tests for Orderbook usecase createFormattedLimitOrder method. * SQS-377 | Unit test ProcessPool Introduces unit tests for Orderbook usecase ProcessPool method. * SQS-378 | Test GetActiveOrders Implements tests for GetActiveOrders orderbook usecase method * SQS-379 | processOrderBookActiveOrders Implements tests for processOrderBookActiveOrders orderbook usecase method. As a side effect introduces some helper methods and refactors some test cases. (cherry picked from commit ec649ca)
* SQS-375 | Unit test createFormattedLimitOrder (#492) * SQS-375 | Unit test createFormattedLimitOrder Introduces unit tests for Orderbook usecase createFormattedLimitOrder method. * SQS-377 | Unit test ProcessPool Introduces unit tests for Orderbook usecase ProcessPool method. * SQS-378 | Test GetActiveOrders Implements tests for GetActiveOrders orderbook usecase method * SQS-379 | processOrderBookActiveOrders Implements tests for processOrderBookActiveOrders orderbook usecase method. As a side effect introduces some helper methods and refactors some test cases. (cherry picked from commit ec649ca) * SQS-375 | Unit test createFormattedLimitOrder Backport --------- Co-authored-by: Deividas Petraitis <[email protected]>
This PR introduces unit tests for Orderbook usecase
createFormattedLimitOrdermethod. The aim of the tests are to cover edge cases as well as successful scenario. As a side effect pull request introducesOrderbookRepositoryMock.Summary by CodeRabbit
New Features
Bug Fixes
placedQuantityin order creation to prevent invalid orders.