[BLCKCHN-280] Refactor for Unit Testing for Developer Platform SDK (PY) #17 #18

leejw51crypto · 2025-07-21T15:26:21Z

tidy up

Pull Request Template

Description

Please include a summary of the change and which issue is fixed. Include the context and motivations behind your changes.

Type of change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
This change requires a documentation update

Checklist:

Before you submit the pull request, please review the following checklist:

I have performed a self-review of my own code.
I have commented my code, particularly in hard-to-understand areas.
I have made corresponding changes to the documentation.
My changes generate no new warnings.
I have added tests that prove my fix is effective or that my feature works.
New and existing unit tests pass locally with my changes.
Any dependent changes have been merged and published in downstream modules.

Linked Issues

Mention the issues that are addressed by this PR. You can use the # symbol to link an issue.

leejw51crypto · 2025-07-21T15:28:39Z

testing github action

leejw51crypto · 2025-07-22T05:15:22Z

polishing code

Copilot

Pull Request Overview

This PR implements comprehensive unit testing for the Developer Platform SDK (Python), adding test coverage for all major SDK components including client authentication, API modules (wallet, token, network, etc.), and integration APIs. It includes pytest configuration, test fixtures, mocking infrastructure, and coverage reporting setup.

Adds 100% unit test coverage for all SDK modules including Client, Wallet, Token, Network, Transaction, Event, Exchange, Contract, DeFi, CronosId, and Block classes
Implements comprehensive integration API testing for all backend services (wallet_api, token_api, network_api, transaction_api, event_api, exchange_api, contract_api, defi_api, cronosid_api, block_api)
Establishes testing infrastructure with pytest configuration, fixtures, mocking, and coverage reporting

Reviewed Changes

Copilot reviewed 39 out of 40 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
pyproject.toml	Adds pytest configuration, test dependencies, and coverage settings
tests/conftest.py	Provides shared test fixtures and utilities for API mocking
tests/test_client.py	Tests Client class authentication, initialization, and API key handling
tests/test_authentication.py	Tests authentication flows and API key security across modules
tests/test_wallet.py	Tests Wallet class functionality including creation and balance retrieval
tests/test_token.py	Tests Token class for native/ERC20/ERC721 operations
tests/test_network.py	Tests Network class for chain info and network details
tests/test_transaction.py	Tests Transaction class for transaction operations and gas estimation
tests/test_event.py	Tests Event class for contract event log retrieval
tests/test_exchange.py	Tests Exchange class for ticker and market data
tests/test_contract.py	Tests Contract class for ABI and bytecode retrieval
tests/test_integrations/	Tests all integration API modules with comprehensive error handling
crypto_com_developer_platform_client/	Minor import ordering fixes and error handling improvements

Comments suppressed due to low confidence (2)

tests/test_integrations/test_transaction_api.py:546

This test expects JSONDecodeError but the actual transaction_api functions don't handle JSON decode errors properly. The test should either mock the JSON decode error handling or the actual API functions should be updated to handle these errors gracefully.

            with pytest.raises(JSONDecodeError):

tests/test_integrations/test_token_api.py:591

This test verifies non-JSON error handling for token_api functions. However, the actual implementation may not properly handle non-JSON responses and could raise JSONDecodeError instead of the expected HTTP error message.

            with pytest.raises(Exception, match="HTTP error! status: 500"):

Copilot · 2025-07-22T05:32:51Z

pyproject.toml

+python_files = ["test_*.py", "*_test.py"]
+python_classes = ["Test*"]
+python_functions = ["test_*"]
+addopts = "--cov=crypto_com_developer_platform_client --cov-report=term-missing --cov-report=html --cov-report=xml --cov-fail-under=62 -v"


[nitpick] The coverage threshold is set to 62% which seems low for a comprehensive test suite. Consider raising this threshold as test coverage improves to maintain quality standards.

Suggested change

addopts = "--cov=crypto_com_developer_platform_client --cov-report=term-missing --cov-report=html --cov-report=xml --cov-fail-under=62 -v"

addopts = "--cov=crypto_com_developer_platform_client --cov-report=term-missing --cov-report=html --cov-report=xml --cov-fail-under=80 -v"

yeehan-crypto-com

Great job - 92% test coverage, maybe can consider increasing test coverage threshold from 62?

rarcifa · 2025-07-22T07:08:11Z

crypto_com_developer_platform_client/client.py

    _api_key: str
    _provider: str

+    def __init__(self):


why do we need init with pass?

it's for instance, restoring back to keep compatible

rarcifa · 2025-07-22T07:08:48Z

crypto_com_developer_platform_client/client.py


    @classmethod
-    def get_api_key(cls) -> str:
+    def get_api_key_class(cls) -> str:


Not sure why change the name? Is it consistent with the TS naming convention?

changing back

rarcifa · 2025-07-22T07:09:09Z

crypto_com_developer_platform_client/client.py


    @classmethod
-    def get_provider(cls) -> str:
+    def get_provider_class(cls) -> str:


Not sure why we change the name here

changing back

rarcifa · 2025-07-22T07:09:40Z

crypto_com_developer_platform_client/client.py

+        """
+        return self.__class__.get_provider_class()
+
+    def get_api_key_instance(self) -> str:


same as get_api_key

get_api_key is class method, this one is instance method.

changing back

rarcifa · 2025-07-22T07:11:28Z

crypto_com_developer_platform_client/exchange.py

        :return: A list of all available tickers and their information.
        """
-        return get_all_tickers()
+        if cls._client is None:


this is not needed if if we check the condition directly in the client file

before calling get_api_instance(), it check validity of _client

get_all_tickers need apikey, so this change ensures, apikey is given correctly

You are right this is needed after our latest change and is consistent with the other methods.

rarcifa · 2025-07-22T07:22:19Z

crypto_com_developer_platform_client/integrations/cronosid_api.py

-        server_error_message = (
-            error_body.get("error") or f"HTTP error! status: {response.status_code}"
-        )
+        try:


The endpoint returns status: success, data or status: failed, message. If we want to add Try Catch it would only be for scenarios where the response does not return a json response.

I am not opposing Try Catch, but if we decide to make this change it should be in a separate ticket and in parallel we should update the TS library so they stay insync and we avoid technical dept down the road.

try catch is necessary to write efficient unit test

error_body = response.json() server_error_message = ( error_body.get("error") or f"HTTP error! status: {response.status_code}" )

for example, by some reason, json format is broken, it throw exception
new code handle it, process it to return error message

and unit test process , in edge cases easily

rarcifa

Thanks @leejw51crypto — this is a really in-depth and well-structured unit test setup. My only concern is that this library is public and client-facing. Since we're already doing both unit and integration testing on the backend to fully validate all endpoints, I think duplicating tests on the client side has some drawbacks:

It increases the code size and technical debt we have to maintain.

It adds additional dependencies that external developers may not want or need.

Because we already handle all the testing on the server side, the client just acts as a lightweight interface. My opinion is that we should keep the public client as minimal and dependency-free as possible — and trust our backend test coverage to ensure correctness.

Let me know your thoughts, but I’d prefer to keep testing scoped to the backend for now.

leejw51crypto · 2025-07-22T08:13:59Z

will move testing code to developer platform service.
this pr will only contain other changes

leejw51crypto · 2025-07-22T12:07:22Z

unit test result

Required test coverage of 80% reached. Total coverage: 91.38%

leejw51crypto · 2025-07-22T14:52:50Z

porting test code to https://github.com/crypto-com/developer-platform-sdk-service

eric-cw-crypto · 2025-07-23T02:15:12Z

crypto_com_developer_platform_client/integrations/wallet_api.py

    if response.status_code not in (200, 201):
-        error_body = response.json()
-        server_error_message = (
-            error_body.get("error") or f"HTTP error! status: {response.status_code}"
-        )
+        try:
+            error_body = response.json()
+            server_error_message = (
+                error_body.get("error") or f"HTTP error! status: {response.status_code}"
+            )
+        except ValueError:
+            # Handle non-JSON error responses
+            server_error_message = f"HTTP error! status: {response.status_code}"
        raise Exception(server_error_message)

    return response.json()


looks like these code are duplicating. do you think is better to put into a shared function instead?

removed redundant code
also catch mal-formed json error

rarcifa · 2025-07-23T09:05:32Z

crypto_com_developer_platform_client/integrations/api_interfaces.py

    data: Dict[str, Any]
+
+
+def handle_api_error(response: requests.Response) -> None:


I do agree with reducing duplication as much as possible, but there are cases where it is ok to repeat logic. Maybe we can move this into a helper function instead of interface.

leejw51crypto · 2025-07-23T10:29:29Z

modifying for feedback

tidy up Update GitHub Actions workflow to use Python 3.12 Update GitHub Actions workflow to use actions/cache@v4 and actions/upload-artifact@v4 for improved caching and artifact management Update GitHub Actions workflow to use Python 3.12 and Codecov action v4; refactor client and exchange classes for consistency in string formatting and whitespace Refactor project structure by updating dependency definitions in pyproject.toml and reorganizing import statements across multiple modules for improved clarity and consistency. Add comprehensive unit tests for Client and API integrations - Introduced new unit tests for the Client class to cover various scenarios including constructor validation, API key handling, and provider management. - Enhanced error handling tests for the Defi API, including malformed JSON responses and various HTTP status codes. - Added tests for token API functions to ensure proper handling of non-JSON error responses and comprehensive error handling for GET and POST methods. - Implemented tests for transaction API functions, including transaction count retrieval and fee data, with extensive error handling and URL encoding validation. Refactor and enhance unit tests for Client and API integrations - Cleaned up whitespace and improved formatting in test cases for better readability. - Updated test functions to ensure consistent handling of parameters and error responses across various API endpoints. - Enhanced error handling tests for transaction and token APIs, including non-JSON responses and edge cases. - Improved URL encoding tests for special characters in parameters to ensure proper functionality. Update authentication tests in GitHub Actions workflow to disable coverage reporting Refactor Client class to use class-level API key and provider methods - Updated methods across multiple modules to replace instance-level API key retrieval with class-level methods for consistency and improved clarity. - Adjusted the Client class constructor to remove the API key parameter, delegating API key management to class methods. - Enhanced unit tests to reflect changes in Client initialization and API key handling, ensuring comprehensive coverage of new functionality. Refactor method calls in Exchange and Token classes for improved readability - Reformatted method calls in the Exchange and Token classes to enhance code clarity by aligning parameters vertically. - Ensured consistent formatting across the codebase for better maintainability. Refactor Client class to use class-level API key methods across multiple modules - Updated method calls in the Block, Contract, CronosId, Defi, Event, Exchange, Network, Token, Transaction, and Wallet classes to replace instance-level API key retrieval with class-level methods for consistency and clarity. - Enhanced readability and maintainability of the codebase by standardizing API key and provider method usage. Refactor Client class API key and provider methods for consistency - Renamed methods from `get_api_key_class` to `get_api_key` and `get_provider_class` to `get_provider` for clarity. - Updated method calls in the Exchange and Token classes to streamline API key retrieval. - Adjusted unit tests to reflect method name changes and ensure consistent functionality across the codebase. Update test coverage threshold in pyproject.toml from 62 to 80 for improved quality assurance Remove Makefile, GitHub Actions workflow, and all test files to streamline project structure and eliminate unused components. Refactor Client class by removing the constructor and delegating initialization to class methods for improved clarity and consistency in API key management. Refactor pyproject.toml to update dependency definitions and remove unused pytest configuration options for improved clarity and maintainability. Refactor error handling in API integrations by introducing a centralized `handle_api_error` function. This change simplifies error management across multiple modules, improving code readability and maintainability. Refactor error handling in token API methods to use centralized `handle_api_error` function for improved readability and maintainability. Added a blank line to improve code readability in the api_interfaces.py file. Refactor error handling by moving `handle_api_error` function to a new module. Update imports across multiple API integration files for improved organization and maintainability.

leejw51crypto requested a review from a team as a code owner July 21, 2025 15:26

leejw51crypto marked this pull request as draft July 21, 2025 15:26

leejw51crypto requested review from rarcifa and yeehan-crypto-com July 21, 2025 15:26

leejw51crypto force-pushed the feature/280 branch from ef326a9 to 68ca8c1 Compare July 21, 2025 15:34

leejw51crypto marked this pull request as ready for review July 21, 2025 16:10

leejw51crypto force-pushed the feature/280 branch from 41850f9 to 0637e65 Compare July 21, 2025 16:11

leejw51crypto requested a review from eric-cw-crypto July 22, 2025 03:36

yeehan-crypto-com requested a review from Copilot July 22, 2025 05:32

Copilot AI reviewed Jul 22, 2025

View reviewed changes

yeehan-crypto-com approved these changes Jul 22, 2025

View reviewed changes

rarcifa reviewed Jul 22, 2025

View reviewed changes

leejw51crypto force-pushed the feature/280 branch 6 times, most recently from 7f69e8c to f21b0ac Compare July 22, 2025 12:01

leejw51crypto force-pushed the feature/280 branch from f21b0ac to e8cb1cb Compare July 22, 2025 14:45

leejw51crypto changed the title ~~[BLCKCHN-280] Unit Testing for Developer Platform SDK (PY) #17~~ [BLCKCHN-280] Refactor for Unit Testing for Developer Platform SDK (PY) #17 Jul 22, 2025

eric-cw-crypto reviewed Jul 23, 2025

View reviewed changes

leejw51crypto force-pushed the feature/280 branch from 683cbae to 6087ef2 Compare July 23, 2025 03:33

eric-cw-crypto approved these changes Jul 23, 2025

View reviewed changes

rarcifa reviewed Jul 23, 2025

View reviewed changes

rarcifa approved these changes Jul 23, 2025

View reviewed changes

leejw51crypto force-pushed the feature/280 branch from 46c61c2 to abfbc55 Compare July 23, 2025 09:13

leejw51crypto force-pushed the feature/280 branch from a0cffc7 to a40f0e9 Compare July 23, 2025 14:48

leejw51crypto merged commit 26fe382 into main Jul 23, 2025
3 checks passed

	addopts = "--cov=crypto_com_developer_platform_client --cov-report=term-missing --cov-report=html --cov-report=xml --cov-fail-under=62 -v"
	addopts = "--cov=crypto_com_developer_platform_client --cov-report=term-missing --cov-report=html --cov-report=xml --cov-fail-under=80 -v"

		data: Dict[str, Any]


		def handle_api_error(response: requests.Response) -> None:

[BLCKCHN-280] Refactor for Unit Testing for Developer Platform SDK (PY) #17 #18

[BLCKCHN-280] Refactor for Unit Testing for Developer Platform SDK (PY) #17 #18

Uh oh!

Conversation

leejw51crypto commented Jul 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Template

Description

Type of change

Checklist:

Linked Issues

Uh oh!

leejw51crypto commented Jul 21, 2025

Uh oh!

leejw51crypto commented Jul 22, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jul 22, 2025

Choose a reason for hiding this comment

Uh oh!

yeehan-crypto-com left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leejw51crypto Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leejw51crypto Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leejw51crypto Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leejw51crypto Jul 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rarcifa left a comment

Choose a reason for hiding this comment

Uh oh!

leejw51crypto commented Jul 22, 2025

Uh oh!

leejw51crypto commented Jul 22, 2025

Uh oh!

leejw51crypto commented Jul 22, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

leejw51crypto commented Jul 21, 2025 •

edited

Loading

leejw51crypto Jul 22, 2025 •

edited

Loading

leejw51crypto Jul 22, 2025 •

edited

Loading

leejw51crypto Jul 22, 2025 •

edited

Loading

leejw51crypto Jul 22, 2025 •

edited

Loading