test: improve coverage for firestore deploy prepare#10356
test: improve coverage for firestore deploy prepare#10356
Conversation
![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 refactors the unit tests for Firestore deployment preparation, introducing new tests for filtering flags and invalid edition handling. However, the changes result in a significant loss of test coverage for database creation logic and validation scenarios that were previously covered. Feedback also highlights a violation of the style guide regarding the use of 'any' types and suggests improving assertions to verify stub parameters for better reliability.
|
|
||
| describe("firestore prepare", () => { | ||
| let sandbox: sinon.SinonSandbox; | ||
| describe("deploy/firestore/prepare", () => { |
There was a problem hiding this comment.
The refactoring of this test file has resulted in a significant loss of test coverage. Specifically, the following scenarios from the original file are no longer tested:\n- Default database creation settings.\n- Enterprise edition with FIRESTORE_NATIVE data access mode.\n- Enterprise edition with MONGODB_COMPATIBLE data access mode.\n- Validation errors when dataAccessMode is used with the standard edition.\n\nWhile the new tests for filtering flags are valuable, they should not replace the existing logic tests for createDatabase. Please restore the missing test cases to ensure the robustness of the Firestore deployment preparation logic.
| }); | ||
|
|
||
| it("should exit early if no firestore configs are found", async () => { | ||
| const context: any = { projectId: "my-project" }; |
There was a problem hiding this comment.
The use of any as a type is prohibited by the repository style guide. Consider defining a proper interface or using a more specific type for the context and options objects to improve type safety and maintainability.
References
- Never use any or unknown as an escape hatch. Define proper interfaces/types or use type guards. (link)
|
|
||
| await prepare(context, options); | ||
|
|
||
| expect(createDatabaseStub).to.have.been.calledOnce; |
There was a problem hiding this comment.
This assertion only checks that the stub was called, but it doesn't verify the configuration passed to it. It is recommended to verify that createDatabase is called with the correct parameters (e.g., databaseId, databaseEdition, locationId) to ensure the logic in prepare.ts is functioning as expected.
### Description Mock integration bounds for target selection filters during firestore uploads. ### Scenarios Tested - Loading partial overrides
joehan
force-pushed
the
test/firestore-prepare
branch
from
a29fb74 to
2944815
Compare
2 participants
Description
Mock integration bounds for target selection filters during firestore uploads.
Scenarios Tested