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 primarily focuses on code formatting improvements, minor refactoring, and updates to the pull request template and documentation. The reviewer identified that the logic for generating activeTestRunId is duplicated across multiple locations in src/lib/e2e/E2E.ts and recommends centralizing this into a static helper method. Additionally, the reviewer noted that the current test case for getUuid in src/lib/e2e/E2E.spec.ts is ineffective because it tests local logic rather than the production implementation, suggesting it should be updated once the refactoring is complete.
src/lib/e2e/E2E.ts
Outdated
| const activeTestRunId = | ||
| testRunId ?? | ||
| (ctx && | ||
| ctx.env && | ||
| ctx.env.utilities && | ||
| typeof ctx.env.utilities.getUuid === "function" | ||
| ? ctx.env.utilities.getUuid().substring(0, 8) | ||
| : Math.random().toString(36).substring(2, 8)) |
There was a problem hiding this comment.
The logic for generating activeTestRunId is duplicated in multiple places within this file (lines 370-377, 484-491, and 510-517). This increases maintenance overhead and the risk of logic drift. Consider refactoring this into a static helper method, such as E2E.getTestRunId(ctx, testRunId), to centralize the tiered fallback logic (provided ID -> environment UUID -> random string).
| const activeTestRunId = | ||
| runId ?? | ||
| (mockCtx && | ||
| mockCtx.env && | ||
| mockCtx.env.utilities && | ||
| typeof mockCtx.env.utilities.getUuid === "function" | ||
| ? mockCtx.env.utilities.getUuid().substring(0, 8) | ||
| : "test-id") |
There was a problem hiding this comment.
This test case for getUuid is currently testing a local implementation of the ID generation logic (a local variable assignment) rather than the actual production code. This makes the test ineffective as it doesn't verify the behavior of the E2E class. Once the ID generation logic is refactored into a reusable method in E2E.ts, this test should be updated to call that method and verify its output under various conditions (e.g., when testRunId is provided, when the environment provides a UUID, and when it falls back to a default/random value).
Co-authored-by: chrismazanek <chrismazanek@users.noreply.github.com> AI-Assisted: true AI-Tool: Antigravity
Coverage Report for CI Build 24152570323Coverage decreased (-0.06%) to 92.795%Details
Uncovered Changes
Coverage RegressionsNo coverage regressions found. Coverage Stats
💛 - Coveralls |
|
2 participants
Description
Adds the conflict strategy
increment, addressing issue #633.When creating a file in Google Drive and a conflict occurs, the
incrementstrategy automatically appends a number (e.g.(1)) to the filename until an available name is found.Optional parameters were added to
StoreActionBaseArgsfor greater control over the increment logic:incrementStart: The initial number to append (default:1)incrementPrefix: The string added before the increment counter (default:" (")incrementSuffix: The string added after the increment counter (default:")")This allows for matching typical Windows naming (e.g.,
file (1).txt) or other custom conventions (e.g.,file_2.txt).Unit tests for the new conflict strategy and parameter variations have been added to
GDriveAdapter.spec.ts. Documentation, JSON schemas, and examples have been regenerated.Closes #633
Type of change
How has this been tested?
Added exhaustive unit tests to GDriveAdapter.spec.ts establishing correct invocation for the
INCREMENTstrategy including safety verifications enforcing no file modifications viaDRY_RUNbounds.Furthermore, built out a new dedicated E2E scenario evaluating increment behavior fully documented under
docs/docs/examplesgenerated by the test runner.Checklist
Credits & Transparency
Co-authored-bycommit trailers or referencing issues).