Skip to content

fix(Storage): Prevent collisions in object context tests#13

Open
mahendra-google wants to merge 1 commit into
mainfrom
fix/flaky-test-listobjectsmatchingcontextkeyvaluepair
Open

fix(Storage): Prevent collisions in object context tests#13
mahendra-google wants to merge 1 commit into
mainfrom
fix/flaky-test-listobjectsmatchingcontextkeyvaluepair

Conversation

@mahendra-google
Copy link
Copy Markdown
Owner

Update copy, list, and patch object tests to utilize unique context keys and values. This ensures isolation during test execution on single-version buckets and prevents cross-test interference.

Update copy, list, and patch object tests to utilize unique context
keys and values. This ensures isolation during test execution on
single-version buckets and prevents cross-test interference.
Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request updates several integration tests by modifying hardcoded Unicode characters in object context keys and values. Additionally, the ListObjectsMatchingContextKeyValuePair test was updated to use SingleVersionBucket instead of ReadBucket. The review feedback suggests further improving test isolation by appending dynamic unique identifiers to the context keys and values to prevent potential collisions during concurrent test runs.

Comment on lines +129 to +130
string contextKey = "A\u00F1\u03A9\U0001F681";
string contextValue = "Ab\u00F1\u03A9\U0001F681";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While changing the Unicode character reduces the immediate collision risk between tests, hardcoding these values still leaves the tests vulnerable to collisions if multiple test runs occur simultaneously against the same bucket (e.g., in a CI environment). To ensure true isolation and prevent flakiness, consider appending a unique identifier to the keys and values.

            string contextKey = $"A\u00F1\u03A9\U0001F681-{IdGenerator.FromGuid()}";
            string contextValue = $"Ab\u00F1\u03A9\U0001F681-{IdGenerator.FromGuid()}";

Comment on lines +131 to +132
string contextKey = "A\u00F1\u03A9\U0001F683";
string contextValue = "Ab\u00F1\u03A9\U0001F683";
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using a dynamic suffix for the context key and value ensures that this test remains isolated even when multiple instances of the test suite run concurrently against the same shared bucket resources.

            string contextKey = $"A\u00F1\u03A9\U0001F683-{IdGenerator.FromGuid()}";
            string contextValue = $"Ab\u00F1\u03A9\U0001F683-{IdGenerator.FromGuid()}";

var custom = new Dictionary<string, ObjectCustomContextPayload>
{
{ "A\u00F1\u03A9\U0001F680", new ObjectCustomContextPayload { Value = "Ab\u00F1\u03A9\U0001F680" } }
{ "A\u00F1\u03A9\U0001F682", new ObjectCustomContextPayload { Value = "Ab\u00F1\u03A9\U0001F682" } }
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Hardcoding the context key and value here still poses a risk of collision during concurrent test execution. Using dynamic values with a unique identifier would provide better isolation and reliability.

                  { $"A\u00F1\u03A9\U0001F682-{IdGenerator.FromGuid()}", new ObjectCustomContextPayload { Value = $"Ab\u00F1\u03A9\U0001F682-{IdGenerator.FromGuid()}" } }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant