{Do not review} Test rapid appends package #3573

anushka567 · 2025-07-23T08:31:29Z

Please ensure your PR title follows the format:

type(scope): subject

Example:
feat(api): add user login endpoint

Available types:

feat: A new feature
fix: A bug fix
docs: Documentation only changes
style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
refactor: A code change that neither fixes a bug nor adds a feature
perf: A code change that improves performance
test: Adding missing tests or correcting existing tests
build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
chore: Other changes that don't modify src or test files
revert: Reverts a previous commit

Description

Link to the issue in case of a bug fix.

Testing details

Manual - NA
Unit tests - NA
Integration tests - NA

Any backward incompatible change? If so, please explain.

github-actions · 2025-07-23T08:31:46Z

Hey there and thank you for opening this pull request! 👋🏼

We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted.

Details:

No release type found in pull request title "{Do not review} Test rapid appends package". Add a prefix to indicate what kind of release this pull request corresponds to. For reference, see https://www.conventionalcommits.org/

Available types:
 - feat: A new feature
 - fix: A bug fix
 - docs: Documentation only changes
 - style: Changes that do not affect the meaning of the code (white-space, formatting, missing semi-colons, etc)
 - refactor: A code change that neither fixes a bug nor adds a feature
 - perf: A code change that improves performance
 - test: Adding missing tests or correcting existing tests
 - build: Changes that affect the build system or external dependencies (example scopes: gulp, broccoli, npm)
 - ci: Changes to our CI configuration files and scripts (example scopes: Travis, Circle, BrowserStack, SauceLabs)
 - chore: Other changes that don't modify src or test files
 - revert: Reverts a previous commit

gemini-code-assist

Summary of Changes

Hello @anushka567, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request is a work-in-progress test PR focused on developing and validating the "rapid appends" functionality. It introduces a new, critical integration test case to ensure correct behavior when multiple clients attempt to append to the same file, specifically testing session invalidation and takeover scenarios. The changes also include refactoring of test setup and isolation of the "rapid appends" tests in the e2e test runner.

Highlights

Test Isolation: The e2e test runner script (improved_run_e2e_tests.sh) was modified to exclusively run the "rapid_appends" tests for zonal buckets. This change allows for focused development and debugging of the rapid appends feature by isolating its test execution.
Test File Renaming: The primary test file for rapid appends was renamed from rapid_appends_test.go to appends_test.go. This is likely a refactoring for better naming consistency within the rapid_appends package.
Test Setup Refactoring: The test setup in appends_test.go was refactored. The logic for creating an unfinalized object was extracted into a new helper function createUnfinalizedObject(). Both SetupSubTest() and a newly added SetupTest() now utilize this helper, reducing code duplication and improving test setup clarity.
New Integration Test Case: A significant new integration test, TestAppendSessionInvalidatedByAnotherClientUponTakeover, was added to appends_test.go. This test verifies the behavior of append sessions when a takeover occurs, ensuring that the original client's session is correctly invalidated and subsequent operations fail with an ESTALE error, while the new client can successfully append and sync.
Test Constants: New constants initialContent and appendContent were introduced in setup_test.go to provide clear, descriptive values for use in the newly added test case, enhancing readability and maintainability.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	`/gemini review`	Performs a code review for the current pull request in its current state.
Pull Request Summary	`/gemini summary`	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	`/gemini help`	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

gemini-code-assist

Code Review

This pull request appears to be focused on testing the rapid_appends package. The changes include renaming the test file, refactoring test setup, adding a new test case for append session invalidation, and modifying the E2E test script to focus only on this package. The review has identified a few areas for improvement: a critical test cleanup function (TearDownSubTest) has been removed without a replacement, which could lead to test artifacts being left behind; the E2E test script has been modified to reduce test coverage; and there's an inconsistency in how test state is managed within the test suite.

gemini-code-assist · 2025-07-23T08:33:50Z

tools/integration_tests/rapid_appends/appends_test.go

 func (t *RapidAppendsSuite) SetupSubTest() {
+	t.createUnfinalizedObject()
+}
+
+func (t *RapidAppendsSuite) SetupTest() {
+	t.createUnfinalizedObject()
+}


The TearDownSubTest function, which was responsible for cleaning up test files, has been removed. Additionally, SetupTest has been added without a corresponding TearDownTest. Failing to clean up resources created during tests is poor practice and can lead to an accumulation of artifacts, potentially causing side effects in other tests or consuming unnecessary disk space.

func (t *RapidAppendsSuite) TearDownTest() { err := os.Remove(path.Join(primaryMntTestDirPath, t.fileName)) // It's possible the file doesn't exist if the test removed it, so we can ignore 'not exist' errors. if err != nil && !os.IsNotExist(err) { require.NoError(t.T(), err) } } func (t *RapidAppendsSuite) TearDownSubTest() { t.TearDownTest() }

gemini-code-assist · 2025-07-23T08:33:51Z

tools/integration_tests/improved_run_e2e_tests.sh

@@ -222,7 +222,7 @@ TEST_PACKAGES_COMMON=(
 # Test packages for regional buckets.
 TEST_PACKAGES_FOR_RB=("${TEST_PACKAGES_COMMON[@]}" "inactive_stream_timeout" "cloud_profiler")
 # Test packages for zonal buckets.
-TEST_PACKAGES_FOR_ZB=("${TEST_PACKAGES_COMMON[@]}" "unfinalized_object" "rapid_appends")
+TEST_PACKAGES_FOR_ZB=("rapid_appends")


This change reduces test coverage for zonal buckets by only running the rapid_appends package. While this is useful for focused testing during development, it should be reverted before merging to ensure all tests for zonal buckets are executed in the CI pipeline.

Suggested change

TEST_PACKAGES_FOR_ZB=("rapid_appends")

TEST_PACKAGES_FOR_ZB=("${TEST_PACKAGES_COMMON[@]}" "unfinalized_object" "rapid_appends")

gemini-code-assist · 2025-07-23T08:33:51Z

tools/integration_tests/rapid_appends/appends_test.go

+	_, err := appendFileHandle.WriteString(initialContent)
+	require.NoError(t.T(), err)


In this test, t.fileContent is not updated after this WriteString call. This is inconsistent with the appendToFile helper function used in TestAppendsAndRead, which does update t.fileContent. While this behavior is correct for this specific test's logic (as the write is expected to be lost), the inconsistency can make the test suite harder to understand and maintain.

// This write is expected to be lost due to session takeover, so we don't update t.fileContent. _, err := appendFileHandle.WriteString(initialContent) require.NoError(t.T(), err)

meet2mky and others added 16 commits July 8, 2025 15:11

Add initial test for unfinalized appends

c8de116

Add initial two append test

f1a251d

fix test for mountedDir

198478f

test unfinalized_appends package

57bb435

test mount failure

c786b78

test changes

b43aa56

stop kokoro machine from getting killed on completion/failure

abe9a29

fix test package

00b5581

fix tests

20dd0cf

revert temporary changes

a28c2de

ft: append after takeover by another client must result in precond error

84a7ca4

merge to master

a214bd4

updated with latest change

dd9cc6d

other validations

4cf2d1e

update setuptest

aaf282e

test

6b211b7

anushka567 added the execute-integration-tests-on-zb To run E2E tests on zonal bucket. label Jul 23, 2025

gemini-code-assist bot reviewed Jul 23, 2025

View reviewed changes

anushka567 added 3 commits July 23, 2025 08:40

check2

9505442

final test

927e06f

check

b7eeba0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

{Do not review} Test rapid appends package #3573

{Do not review} Test rapid appends package #3573

Uh oh!

anushka567 commented Jul 23, 2025

Uh oh!

github-actions bot commented Jul 23, 2025

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot left a comment

Uh oh!

gemini-code-assist bot Jul 23, 2025

Uh oh!

gemini-code-assist bot Jul 23, 2025

Uh oh!

gemini-code-assist bot Jul 23, 2025

Uh oh!

Uh oh!

	TEST_PACKAGES_FOR_ZB=("rapid_appends")
	TEST_PACKAGES_FOR_ZB=("${TEST_PACKAGES_COMMON[@]}" "unfinalized_object" "rapid_appends")

		_, err := appendFileHandle.WriteString(initialContent)
		require.NoError(t.T(), err)

{Do not review} Test rapid appends package #3573

Are you sure you want to change the base?

{Do not review} Test rapid appends package #3573

Uh oh!

Conversation

anushka567 commented Jul 23, 2025

Description

Link to the issue in case of a bug fix.

Testing details

Any backward incompatible change? If so, please explain.

Uh oh!

github-actions bot commented Jul 23, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Summary of Changes

Highlights

Footnotes

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist bot Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

gemini-code-assist bot Jul 23, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!