-
Notifications
You must be signed in to change notification settings - Fork 54
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
Draft: Refactor repository module's tests #255
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit relocates the Market DTO and Entity Mapper tests to improve the project structure. - Moves `MarketDtoMapperTest` and `MarketEntityMapperTest` from `ir.composenews.data.repository.mapper` to `ir.composenews.data.mapper`. - Deletes the now-empty `MarketDtoMapperTest` and `MarketEntityMapperTest` files in the `repository` package. - Creates `MarketDtoMapperTest` and `MarketEntityMapperTest` files in `mapper` package.
This commit adds comprehensive tests for the `MarketChartDtoMapper` which is responsible for converting `MarketChartResponse` to `Chart`. The tests cover various scenarios: - Conversion of valid market chart responses. - Handling of empty prices. - Handling of negative timestamps and prices. - Handling of zero values. - Handling of large double values. - Truncation of decimals in non-integer timestamps. - Handling of missing elements in price pairs, throwing `IndexOutOfBoundsException`.
This commit introduces a new test suite `MarketDetailDtoMapperKtTest` to specifically address scenarios where the market detail response might contain null or invalid data. **Key changes:** * Added test cases to handle `null` values for `id`, `name`, and `marketCapRank` in the `MarketDetailResponse`. * Added test cases to handle `null` `marketData` within `MarketDetailResponse`. * Added test cases to handle `null` fields within `marketData` like `high24h`, `low24h`, `marketCap`, and `marketCapRank`. * Added test cases for handling negative and zero values in market data. * Refactored test assertions to compare against expected default values when `null` is present. * Implemented custom `shouldBeEqual` functions for more readable assertions. * These test cases ensure the robustness of the mapping logic, especially when dealing with incomplete or faulty external data. * The new test cases verify that default values are assigned properly when data is null. * The new test cases verify that zero values and negative values are handled properly. * The test cases ensures the response will be associated with the request.
- Renamed `Chart` to `MarketChart` for better clarity. - Updated all references of `Chart` to `MarketChart` across the codebase. - Modified mapper to return `MarketChart`. - Updated use case to return `MarketChart`. - Updated repository to work with `MarketChart`. - Updated view model, test cases and UI test. - Updated contract to work with `MarketChart`.
…)` and improve assertions #254 The `MarketChartDtoMapperKtTest` has been refactored to use the `toMarketChart()` function instead of `toChart()`. Additionally, the assertions have been updated to be more concise and efficient, using `shouldContainExactly` for a more direct comparison of the `MarketChart` to the `MarketChartResponse`. Also a infix function `shouldContainExactly` added for easier checking.
VahidGarousi
reviewed
Feb 8, 2025
data/market-repository/src/test/java/ir/composenews/data/mapper/MarketChartDtoMapperKtTest.kt
Outdated
Show resolved
Hide resolved
VahidGarousi
requested changes
Feb 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let we talk if you have any questions
- Removed `FakeMarketsApi` class. - Updated `MarketRepositoryImpl`: - Refactored `toggleFavoriteMarket` to use `marketEntity` for clarity. - Refactored error handling in `fetchChart` and `fetchDetail` using more compact syntax.
#254 - Delete `NewsRepositoryImplTest` file - Create `MarketRepositoryImplTest` file with comprehensive tests for `MarketRepositoryImpl` - Add test cases for `getMarketList`, `getFavoriteMarketList`, `syncMarketList`, `toggleFavoriteMarket`, `fetchChart`, and `fetchDetail` functions - Implement test cases for successful and failed API calls - Add new constants for database `FALSE` and `TRUE` values - Add test cases for different scenarios including database, and api interactions.
- Refactored the assertion logic in `MarketChartDtoMapperKtTest` to use `shouldBe 2` instead of the previous `if` check. - Removed the previous `IndexOutOfBoundsException` for this change.
VahidGarousi
approved these changes
Feb 9, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me! The changes are well-structured and improve clarity. Approved.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Refactor repository module's tests.
fix #254