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

Draft: Refactor repository module's tests #255

Merged
merged 9 commits into from
Feb 9, 2025
Merged

Conversation

Kaaveh
Copy link
Owner

@Kaaveh Kaaveh commented Feb 7, 2025

Refactor repository module's tests.

fix #254

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.
@Kaaveh Kaaveh added the refactor label Feb 7, 2025
@Kaaveh Kaaveh requested a review from VahidGarousi February 7, 2025 14:16
@Kaaveh Kaaveh self-assigned this Feb 7, 2025
Copy link

github-actions bot commented Feb 7, 2025

1 Warning
⚠️ Please consider breaking up this pull request.
1 Message
📖 Thanks @Kaaveh!

Generated by 🚫 Danger

@Kaaveh Kaaveh changed the title Refactor repository module's tests Draft: Refactor repository module's tests Feb 7, 2025
@Kaaveh Kaaveh marked this pull request as draft February 7, 2025 15:11
- 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.
Copy link
Collaborator

@VahidGarousi VahidGarousi left a 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.
@Kaaveh Kaaveh marked this pull request as ready for review February 8, 2025 14:45
Copy link
Collaborator

@VahidGarousi VahidGarousi left a 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.

@VahidGarousi VahidGarousi merged commit f11414f into kmp Feb 9, 2025
3 checks passed
@VahidGarousi VahidGarousi deleted the test/repository branch February 9, 2025 04:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[refactor] tests in repository
2 participants