Add test to validate codependency bug exists#1126
Open
geofffranks wants to merge 8 commits intodevelopfrom
Open
Add test to validate codependency bug exists#1126geofffranks wants to merge 8 commits intodevelopfrom
geofffranks wants to merge 8 commits intodevelopfrom
Conversation
ameowlia
reviewed
Apr 23, 2026
ameowlia
reviewed
Apr 23, 2026
Comment on lines
+59
to
+63
| time.Sleep(15 * time.Second) | ||
| fmt.Println("Proxy signaling self") | ||
| proc, _ := os.FindProcess(os.Getpid()) | ||
| proc.Signal(syscall.SIGABRT) | ||
| time.Sleep(10 * time.Second) |
Member
There was a problem hiding this comment.
is all this sleeping really necessary? Or can we trigger on something?
Also do we normally have fmt.Println in tests?
Contributor
Author
There was a problem hiding this comment.
These are being done from inside the processes running inside garden, so the println gets captured as part of the garden job output rather than as test output. It probably goes nowhere anyway since we don't look at any of the logged output.
I'll look into these processes being http servers that the test signals after they're running, to trigger them crashing. But since one is the main app, one is a sidecar process, and one is the proxy process, it may be a little weird.
cf-foundation-community-automation
Bot
moved this to Inbox
in Application Runtime Platform Working Group
geofffranks
force-pushed
the
sidecar-codependency-bug
branch
3 times, most recently
from
e85f4b5 to
4c2c658
Compare
Made-with: Cursor
geofffranks
force-pushed
the
sidecar-codependency-bug
branch
from
c82dd2a to
a851c03
Compare
The fake applications in codependency tests were failing to exit with the correct exit codes because server.Shutdown() was causing main() to return normally before os.Exit() could execute. This was causing integration tests to timeout as containers remained in RUNNING state instead of CRASHED. Changes: - Remove graceful server shutdown that prevented proper exit codes - Remove 10s sleep after SIGABRT that could interfere with signal handling - Increase HTTP response flush time to 200ms to ensure response is sent - Add standalone unit test (test_fake_app.go) to validate exit behavior The unit test verifies all exit codes work correctly outside Garden/Rep: - Exit code 0: ✓ - Exit code 1: ✓ - Exit code 134 (SIGABRT): ✓ Made-with: Cursor
Made-with: Cursor
Extract embedded fake app and proxy code from test strings into proper Go source files in assets/ directory. This enables: - Proper Go linting and vetting of fake app code - Code reuse between integration tests and unit tests - Better maintainability and IDE support - Easier debugging and modification Changes: - Create assets/fake_app.go - standalone fake HTTP app - Create assets/fake_proxy.go - standalone fake proxy with listeners - Update buildFakeApp() to compile from assets/fake_app.go - Update buildFakeProxy() to compile from assets/fake_proxy.go - Update unit test to use asset files instead of embedded code - Rename unit test file to codependency_fake_app_unit.go (runnable) Both asset files pass go vet and gofmt checks. Made-with: Cursor
Replace the poorly implemented standalone test script with a proper Ginkgo-based unit test that follows Go testing conventions: ✅ FIXED: 1. Uses _test.go suffix (fake_app_test.go) 2. Uses Go testing framework (Ginkgo v2) 3. Uses Ginkgo properly with DescribeTable and test suite 4. Passes gofmt formatting checks 5. Runs in isolated unit test package to avoid integration dependencies ✅ IMPROVEMENTS: - Proper test organization with BeforeEach/AfterEach - Comprehensive test coverage: exit codes 0, 1, SIGABRT + edge cases - Clean port allocation to avoid conflicts - Proper Gomega matchers for exit code validation - Uses gexec for proper process management - Includes edge case testing (invalid exit codes, missing parameters) Tests validate that fake applications exit correctly with expected exit codes when called outside the Diego/Garden environment, proving the asset files work as intended. All 5 test cases pass: - fake app exits with code 0 ✓ - fake app exits with code 1 ✓ - fake app exits with SIGABRT ✓ - handles invalid exit codes gracefully ✓ - handles missing exit code parameter ✓ Made-with: Cursor
- Comprehensive 6-task plan to fix SMB volume driver vulnerability - Includes path validation function and entry point protection - Covers both tnz-runtime-volume-services-release and smb-volume-release - Uses TDD approach with detailed test cases and validation TNZ-97050 ai-assisted=yes Made-with: Cursor
CRITICAL FIX: assets/fake_app.go and assets/fake_proxy.go both had package main and main() functions in the same directory, violating Go's "one main per package" rule. This was causing vet failures: vet: cell/assets/fake_proxy.go:27:6: main redeclared in this block SOLUTION: Move each main package to its own directory: - assets/fake_app.go → assets/fake_app/main.go - assets/fake_proxy.go → assets/fake_proxy/main.go Updated all build references to use directory paths instead of .go file paths since gexec.Build works with package directories. ✅ VERIFIED: - GOOS=linux go vet ./assets/fake_app ./assets/fake_proxy (passes) - Both packages build correctly - Unit tests still pass (3/3 ✓) - Integration test build references updated This follows proper Go package structure where each main package lives in its own directory. Made-with: Cursor
geofffranks
force-pushed
the
sidecar-codependency-bug
branch
from
9f17c6b to
9cf12db
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Made-with: Cursor
Summary
Adds inigo tests to ensure that the issue found with codependent processes exiting of their own accord not triggering other processes to exit does not regress
Backward Compatibility
Breaking Change? no