PR and Regression testing for Go snippets #973
Conversation
ivanmkc
force-pushed
the
imkc--go-regression-testing
branch
from
693c41d to
d1260fa
Compare
ivanmkc
force-pushed
the
imkc--go-regression-testing
branch
from
48dc600 to
3f07437
Compare
|
@ivanmkc : Does this update have any dependencies on ADK Go releases beyond v0.1.0? If so, you should mark this as BLOCKED until an official release is available with these changes. |
ivanmkc
force-pushed
the
imkc--go-regression-testing
branch
from
cd0c700 to
f76b10d
Compare
| inMemoryService := session.InMemoryService() | ||
| fmt.Println("Initialized InMemoryService.") | ||
|
|
||
| // --8<-- [start:vertexai_service] |
There was a problem hiding this comment.
Removed as it is unreferenced and uses an old API
tools/go-snippets/runner.sh
Outdated
| # Update to the latest version of the ADK. | ||
| # This ensures that we are always testing against the most recent release. | ||
| echo "Updating google.golang.org/adk to latest..." | ||
| (cd examples/go && go get google.golang.org/adk@latest) |
There was a problem hiding this comment.
@kdroste-google ensures we run with the latest version of adk-go released.
There was a problem hiding this comment.
Yes, looks good. Maybe it would make sense to check the exit code here as well (network issues etc).
kdroste-google
left a comment
kdroste-google
left a comment
There was a problem hiding this comment.
Pls check the comments
tools/go-snippets/runner.sh
Outdated
| # Update to the latest version of the ADK. | ||
| # This ensures that we are always testing against the most recent release. | ||
| echo "Updating google.golang.org/adk to latest..." | ||
| (cd examples/go && go get google.golang.org/adk@latest) |
There was a problem hiding this comment.
Yes, looks good. Maybe it would make sense to check the exit code here as well (network issues etc).
| The scripts are primarily designed to be run automatically by GitHub Actions. | ||
|
|
||
| - **On Pull Requests:** When a pull request is opened, two workflows are triggered: | ||
| 1. **Go Snippets Build on PR and Schedule:** This workflow runs `check_go_snippets.sh` to ensure new files are registered. It then intelligently builds **only the `.go` files that were changed** in the PR. |
There was a problem hiding this comment.
I'm not sure about that. There are dependencies like: examples/go/snippets/tools-custom/doc_analysis/doc_analysis.go and examples/go/snippets/tools-custom/doc_analysis/main.go - that imho may break - a change in doc_analysis.go itself can be fine, but main.go (no changes) may stop compiling.
There was a problem hiding this comment.
Good point. In our setup, multi-file snippets (like doc_analysis) are listed on the same line in files_to_test.txt. The runner works by matching the changed file to that line and executing the full build command for that entry (e.g., go build main.go doc_analysis.go). This ensures that main.go is indeed recompiled and verified. I've updated tools/go snippets/README.md to explicitly document this requirement.
…te ADK dependency Consolidate all Go snippet module dependencies to a single file located at . This simplifies dependency management and ensures consistent versions across all Go examples. Update Usage: tools/go-snippets/runner.sh <build|run> [file1 file2 ...] to dynamically fetch the latest module () before running any builds or tests. This ensures that PR and regression checks always validate against the most recent ADK release, preventing outdated dependency issues. Address breaking changes in by removing the initialization from , which is no longer available. Cleaned up extraneous and files from subdirectories within , as well as the unused file.
Reverts previous attempts to infer Go snippet dependencies and clarifies the strict testing model. - **`runner.sh`**: Restores `find_snippet_line` to rely on exact substring matching of changed files within `files_to_test.txt` lines. Adds `go get` exit code check. - **`tools/go-snippets/README.md`**: Adds "Understanding the Test Runner" section detailing `files_to_test.txt` adherence, multi-file `package main` requirements, and `_test.go` file handling. - **`runner_test.sh`**: Adds `test_find_snippet_line_multi_file_match` to verify correct behavior for multi-file snippets listed on a single line.
ivanmkc
force-pushed
the
imkc--go-regression-testing
branch
from
d1ece7c to
12597ff
Compare
This dynamically uses the latest release of ADK-Go from the package manager. |
4 participants
This PR introduces new tooling for managing and validating Go snippets within the repository.
Key additions include:
tools/go-snippets/runner.sh): A new script to build and run Go snippets, supporting both targeted execution for changed files and full regression testing. This script replaces the need for separatebuild_all.shandrun_all.shscripts.tools/go-snippets/files_to_test.txt): A new configuration file that lists all Go snippet files to be included in the build and run processes. It supports single files and multiple files for packages with shared dependencies, and allows for comments.tools/go-snippets/check_go_snippets.sh): A new script to ensure that all.gofiles in theexamples/godirectory are properly referenced infiles_to_test.txt, preventing new snippets from being overlooked in the test suite..github/workflows/go-snippets-pr-check.yamlworkflow utilizes these new scripts for comprehensive Go snippet testing on pull requests and a weekly schedule.These new files establish a robust and centralized system for Go snippet management, ensuring consistency and test coverage.