feat(librarian): add test-container test logic by zhumin8 · Pull Request #2656 · googleapis/librarian

zhumin8 · 2025-10-22T21:43:15Z

Adding test logic according to go/sdk-librarian:container-test.

Tested locally with a couple of libraries in google-cloud-go.
Note: Removed changes for adding the command, will address in separate PR.

For #2623

codecov · 2025-10-22T21:46:36Z

Codecov Report

❌ Patch coverage is 79.11647% with 52 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.54%. Comparing base (b5021b8) to head (d840ba4).
⚠️ Report is 24 commits behind head on main.

Files with missing lines	Patch %	Lines
internal/librarian/test_container_generate.go	80.29%	26 Missing and 14 partials ⚠️
internal/gitrepo/gitrepo.go	73.91%	6 Missing and 6 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2656      +/-   ##
==========================================
+ Coverage   86.69%   87.54%   +0.85%     
==========================================
  Files         118      120       +2     
  Lines        9814    11281    +1467     
==========================================
+ Hits         8508     9876    +1368     
- Misses        921      976      +55     
- Partials      385      429      +44

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ldetmer · 2025-10-24T14:25:56Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a new librarian test-container generate command, which is a valuable addition for testing language-specific generation containers. The implementation is well-structured and includes comprehensive tests for the new functionality. I've included a couple of minor suggestions to improve code clarity and documentation.

internal/librarian/flags.go

internal/librarian/test_container_generate.go

internal/gitrepo/gitrepo.go

internal/librarian/test_container_generate.go

JoeWang1127

I think you can split the PR into some smaller ones.

For example, add git functions, add command skeletons, add testing functions, etc.

JoeWang1127 · 2025-10-24T20:41:29Z

Given the complexity of the implementation, e.g., additional git features, finding protos and insertion points, validation steps, write tests for testing functions, do you think this is better than alternative option 1?

zhumin8 · 2025-10-24T21:39:25Z

Given the complexity of the implementation, e.g., additional git features, finding protos and insertion points, validation steps, write tests for testing functions, do you think this is better than alternative option 1?

One major drawback of that approach is the maintenance burden of the golden files and less tolerance to changes in container. e.g., change in generator or config files would need an update.
We can discuss further in go/sdk-librarian:container-test if you have more doubts.

…er repo

zhumin8 · 2025-10-29T15:48:21Z

Changes since live review:

removed changes to introduce command, keeping PR focused on test logic. f43079f
improved log and error messages, especially at validation failures. d89d848
filter changes within source root for validation b55de4a
change cleanup behavior to only discard code repo changes when tests are successful b23d7e3
add logic to delete sourceRepo branches introduced in test (only if no failures)e58992f

This reverts commit fe6da5e.

zhumin8 · 2025-10-30T13:51:24Z

ci failures resolved in #2718, reverted changes related to that.

ldetmer · 2025-10-30T14:51:03Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a container-based testing framework for the librarian tool, which is a significant feature. The changes include extending the gitrepo interface and implementation with new git operations, and adding the core test runner logic in test_container_generate.go. The accompanying tests are thorough and cover many edge cases.

I've found a couple of correctness issues in the test runner logic that could cause tests to fail in valid scenarios. My review includes suggestions to fix these.

In prepareForGenerateTest, there's a bug where it attempts to create a commit on a clean worktree if no proto files are modified, which will cause an error.
In validateGenerateTest, there's an incorrect validation step that fails the test if any new files are generated, even if they are an expected result of the code generation.

After addressing these points, the new testing framework should be robust. Great work on the comprehensive test coverage for the new functionality.

internal/librarian/test_container_generate.go

…no generation, fail otherwise. fix cleanup to fail on error

zhumin8 · 2025-10-30T21:18:29Z

/gemini review

gemini-code-assist

Code Review

This pull request introduces a new testing mechanism for library generation using test containers. The core logic involves injecting temporary changes (GUIDs) into proto files, running the generator, and then validating that the generated code reflects these changes. New git helper functions and corresponding tests have been added to support this workflow. The overall approach is solid and the code is well-tested. I have two points of feedback: one on the validation logic, which seems overly strict and might cause tests to fail for valid generation scenarios, and another on improving the robustness of the new DeleteLocalBranches function.

internal/librarian/test_container_generate.go

internal/gitrepo/gitrepo.go

ldetmer

In general looks good to me, please just update logging to start with lowercase.

PR created by the Librarian CLI to initialize a release. Merging this PR will auto trigger a release. Librarian Version: not available Language Image: us-central1-docker.pkg.dev/cloud-sdk-librarian-prod/images-prod/librarian-release-container:latest <details><summary>librarian: 1.0.0</summary> ## [1.0.0](v0.5.0...v1.0.0) (2025-11-06) ### Features * Remove `..._gax` dependency (#2713) ([01675b7](01675b72)) * add context to Librarian PRs (#2698) ([0523329](0523329a)) * add a package comment linter (#2712) ([1bd3e32](1bd3e32e)) * generate pom.xml files (#2682) ([50b95f2](50b95f20)) * add test-container test logic (#2656) ([514cf7e](514cf7e5)) * improve rust-publish logging (#2671) ([585ed50](585ed50b)) * write a timing log at the end of update-image (#2771) ([5fc9e3b](5fc9e3b6)) * migrate Java container from sdk-platform-java (#2670) ([69ac47f](69ac47fd)) * switch to original head after running update-image (#2696) ([7a3e404](7a3e404a)) * format bulk commit from other sources in release notes (#2665) ([7c52da2](7c52da2b)) * enable multi-version API generation (#2699) ([86c5250](86c52507)) * release stage to update pom.xml files (#2772) ([be56755](be567550)) * use proto presence info to generate null-safe code (#2726) ([e36fb81](e36fb81a)) * Base messages in google_cloud_protobuf (#2660) ([e607ea6](e607ea63)) ### Bug Fixes * retry Github 503 requests up to 3 times (#2650) ([09468fa](09468faf)) * Changed docs to ConfigurationException (#2697) ([0950c1e](0950c1e3)) * fix the reference to the old "librariangen" folder (#2677) ([09dc53f](09dc53fe)) * run godoclint via golangci-lint (#2751) ([264a6a0](264a6a0e)) * pass context as the first argument (#2769) ([298a3bd](298a3bd8)) * Move HTTP client creation to `_gax` (#2672) ([4968d63](4968d63d)) * deduplicate bulk commits (#2758) ([4dfae9a](4dfae9ae)) * Support additional api paths for an existing library (#2666) ([50046f5](50046f55)) * remove stray TODO (#2748) ([5072f0e](5072f0e0)) * omit status field when empty (#2654) ([572ae4f](572ae4f3)) * use T.Context in tests (#2768) ([7e7cd2d](7e7cd2dd)) * wrap error to provide more context for commitAndPush (#2767) ([a2a41a4](a2a41a4a)) * use t.Fatal in tests for proper failure handling (#2759) ([cdabb28](cdabb287)) * enforce removal before copying library files (#2765) ([d5ac6c8](d5ac6c87)) * change log level to debug (#2798) ([f042d0b](f042d0bd)) * change path to `doc.go` in docgen test (#2700) ([fd6bae4](fd6bae40)) ### Documentation * use consistent library id in flags and examples (#2770) ([87a1005](87a10056)) </details>

zhumin8 added 8 commits October 14, 2025 17:09

feat: add skeleton for librarian test-container command

e94afb7

implement test command

e03f502

add flag check-unexpected-changes, add test

4c7a8f7

comments and minor refactorings

ffce6e4

Merge branch 'main' into test-cmd

2932906

move r.runGenerateCommand, and a few small fixes/refactor

6a0dbdb

add test and comment for readability

c514450

minor changes

24b143e

zhumin8 added 7 commits October 23, 2025 11:43

run test for all managed libraries if not specified via flag

b5dea6e

removed unused fields from runner

2b22533

change error message

e78c343

add documentation for command

c91f8e2

doc edits

96c1005

fix for empty file check, adding test cases.

8833520

fix lint, re generate doc

d5f88ab

zhumin8 marked this pull request as ready for review October 23, 2025 21:29

zhumin8 requested a review from a team as a code owner October 23, 2025 21:29

zhumin8 requested review from JoeWang1127 and codyoss October 23, 2025 21:50

gemini-code-assist bot reviewed Oct 24, 2025

View reviewed changes

internal/librarian/flags.go Outdated Show resolved Hide resolved

internal/librarian/test_container_generate.go Outdated Show resolved Hide resolved

remove unused code, edit doc language.

5d902f8

JoeWang1127 reviewed Oct 24, 2025

View reviewed changes

internal/gitrepo/gitrepo.go Show resolved Hide resolved

JoeWang1127 reviewed Oct 24, 2025

View reviewed changes

internal/librarian/test_container_generate.go Show resolved Hide resolved

JoeWang1127 reviewed Oct 24, 2025

View reviewed changes

zhumin8 added 2 commits October 24, 2025 17:46

minor cleanup to gitrepo functions

55cd8ab

refactor testSingleLibrary to be reusable, move outputDir calculate p…

d9bad2c

…er repo

change to only cleanup code repo when tests are successful

b23d7e3

zhumin8 mentioned this pull request Oct 29, 2025

librarian: container testing - implement test logic #2623

Closed

zhumin8 added 6 commits October 29, 2025 14:52

Add logic to cleanup on failure to delete branches created during test

e58992f

fix gitrepo delete issue and add tests

c3916b2

fix lint

701f7ec

buildtag: +build line is no longer needed (govet)

fe6da5e

Merge branch 'main' into test-cmd

09567d7

Revert "buildtag: +build line is no longer needed (govet)"

2ff1d7e

This reverts commit fe6da5e.

zhumin8 changed the title ~~librarian: add test-container test logic~~ feat(librarian): add test-container test logic Oct 30, 2025

gemini-code-assist bot reviewed Oct 30, 2025

View reviewed changes

internal/librarian/test_container_generate.go Show resolved Hide resolved

internal/librarian/test_container_generate.go Show resolved Hide resolved

add error handling for cases nothing to generate, skip if configured …

43d4a9c

…no generation, fail otherwise. fix cleanup to fail on error

gemini-code-assist bot reviewed Oct 30, 2025

View reviewed changes

internal/librarian/test_container_generate.go Show resolved Hide resolved

internal/gitrepo/gitrepo.go Show resolved Hide resolved

zhumin8 added 2 commits October 31, 2025 09:40

add doc comment for runTests

964f281

fix DeleteLocalBranches to handle error getting headRef

9ec4158

ldetmer reviewed Nov 3, 2025

View reviewed changes

internal/gitrepo/gitrepo.go Outdated Show resolved Hide resolved

ldetmer reviewed Nov 3, 2025

View reviewed changes

internal/gitrepo/gitrepo.go Outdated Show resolved Hide resolved

ldetmer reviewed Nov 3, 2025

View reviewed changes

internal/gitrepo/gitrepo.go Outdated Show resolved Hide resolved

ldetmer reviewed Nov 3, 2025

View reviewed changes

zhumin8 mentioned this pull request Nov 3, 2025

feat(internal/librarian): add test flag to update-image command #2729

Merged

fix slog entry to follow no capital, no period style

d840ba4

ldetmer approved these changes Nov 3, 2025

View reviewed changes

zhumin8 merged commit 514cf7e into main Nov 3, 2025
9 checks passed

zhumin8 deleted the test-cmd branch November 3, 2025 21:24

cloud-sdk-librarian-robot mentioned this pull request Nov 6, 2025

chore: librarian release pull request: 20251106T080249Z #2805

Closed

ldetmer mentioned this pull request Nov 6, 2025

chore: librarian release pull request: 20251106T145301Z #2808

Merged

Conversation

zhumin8 commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Oct 22, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ldetmer commented Oct 24, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JoeWang1127 left a comment

Choose a reason for hiding this comment

Uh oh!

JoeWang1127 commented Oct 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhumin8 commented Oct 24, 2025

Uh oh!

zhumin8 commented Oct 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

zhumin8 commented Oct 30, 2025

Uh oh!

ldetmer commented Oct 30, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

zhumin8 commented Oct 30, 2025

Uh oh!

gemini-code-assist bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ldetmer left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

zhumin8 commented Oct 22, 2025 •

edited

Loading

codecov bot commented Oct 22, 2025 •

edited

Loading

JoeWang1127 commented Oct 24, 2025 •

edited

Loading

zhumin8 commented Oct 29, 2025 •

edited

Loading