Conversation
Added comprehensive unit tests to TestDataConnectApiClient covering client constructor, inputs validation, variables and impersonation serialization, and service URL construction.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a comprehensive suite of unit tests for the _DataConnectApiClient class, covering its constructor, input validation, payload preparation, and service URL generation. The review feedback identifies critical issues in the tests, including a missing import for the dataclass decorator and references to undefined classes and attributes in the dataconnect module, both of which will cause runtime errors during test execution.
| assert client1_app2 is not client1a | ||
|
|
||
|
|
||
| class TestDataConnectApiClientConstructor: |
There was a problem hiding this comment.
The dataclass decorator is used multiple times in the new test classes (e.g., lines 396, 402, 458, 464, 497, 503, 552, 558), but it is not imported in this test file. This will result in a NameError: name 'dataclass' is not defined when running the tests. Please import dataclass from dataclasses.
| class TestDataConnectApiClientConstructor: | |
| from dataclasses import dataclass | |
| class TestDataConnectApiClientConstructor: |
| def test_constructor_invalid_app(self): | ||
| with pytest.raises(ValueError, match="First argument passed to DataConnectApiClient must be a valid Firebase app instance."): | ||
| dataconnect._DataConnectApiClient(BASE_CONFIG, None) |
There was a problem hiding this comment.
The test cases reference dataconnect._DataConnectApiClient, dataconnect.GraphqlOptions, and dataconnect.Impersonation, but these classes/attributes are not defined in firebase_admin/dataconnect.py (which is the only place they are imported from). Running these tests will result in AttributeError. Please ensure that the implementation of these helpers is included in this pull request or that the necessary files/changes are committed.
1 participant
Added comprehensive unit tests to TestDataConnectApiClient covering client constructor, inputs validation, variables and impersonation serialization, and service URL construction.