[test-improver] Improve tests for sys/container package

[github-actions]

Test Improvements: internal/sys/container_test.go

File Analyzed

• Test File: internal/sys/container_test.go
• Package: internal/sys
• Lines Added: +80

---

Improvements Made

1. Increased Coverage

• ✅ Added TestExtractContainerIDFromCgroup — directly exercises the unexported extractContainerIDFromCgroup() function which had 0% coverage. The test calls the function and verifies any returned ID is at least 12 characters (the documented minimum), tolerating both container and non-container environments.
• ✅ Added 7 new sub-tests to TestExtractContainerIDFromContent:
  • docker at end of path with no following segment — "0::/docker" (no ID segment)
  • containerd at end of path with no following segment — "0::/containerd"
  • docker with ID exactly 11 chars (below minimum) — boundary case just under the 12-char threshold
  • containerd with ID exactly 11 chars (below minimum) — same boundary for containerd
  • match on second line only — multi-line cgroup content where only the second line contains a match
  • first line no match, second line containerd with valid ID — similar multi-line scenario
  • docker appears in non-segment context within a non-matching path — "0::/system.slice/docker.service" (docker in service name, not a path segment with a following ID)

2. New Invariant Tests for DetectContainerID

• ✅ Added TestDetectContainerID_ReturnedIDIsValidOrEmpty — asserts the 12-character minimum invariant on any non-empty container ID returned by DetectContainerID, making the contract explicit.
• ✅ Added TestDetectContainerID_EnvVarReturnsEmptyID — verifies that container detection via the RUNNING_IN_CONTAINER env-var path (the only detection method that has no ID extraction) returns an empty container ID when no file-based indicator is present.

3. Coverage Improvement

Function	Before	After
extractContainerIDFromCgroup	0.0%	71.4%
extractContainerIDFromContent	92.3%	92.3% (unchanged — remaining gap is a scanner.Err() dead-code branch, unreachable with strings.NewReader)
DetectContainerID	59.1%	59.1% (unchanged — /.dockerenv branch requires root to create)
Overall package	61.4%	72.7%

---

Test Execution

All internal/sys tests pass:

ok  github.com/github/gh-aw-mcpg/internal/sys   0.009s   coverage: 72.7% of statements

Note: A pre-existing unrelated failure exists in internal/config (TestFetchAndFixSchema_NetworkError) that is not caused by these changes.

---

Why These Changes?

extractContainerIDFromCgroup was the only unexported function in the package with zero test coverage — a clear gap since it is called from the DetectContainerID production path. The boundary-value edge cases added to TestExtractContainerIDFromContent (11-char IDs, trailing docker/containerd path segments, multi-line content) make the tests more robust against regressions in the minimum-length check and multi-line scanning logic.

---

Generated by Test Improver Workflow
Focuses on better patterns, increased coverage, and more stable tests

Warning

⚠️ Firewall blocked 1 domain

The following domain was blocked by the firewall during workflow execution:

• invalidhostthatdoesnotexist12345.com

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "invalidhostthatdoesnotexist12345.com"

See Network Configuration for more information.

Generated by Test Improver · ● 2.4M · ◷

[Copilot]

Pull request overview

Improves test coverage and edge-case handling for container detection logic in internal/sys, focusing on container ID extraction from cgroup content and the DetectContainerID() contract.

Changes:

• Added additional table-driven cases for extractContainerIDFromContent (boundary lengths, path endings, multi-line inputs, non-segment “docker” occurrences).
• Added tests that exercise cgroup-based ID extraction and assert basic invariants on IDs returned from DetectContainerID.
• Added a test intended to validate the env-var-only detection path returns an empty container ID.

Show a summary per file

File	Description
internal/sys/container_test.go	Adds new subtests and new test functions to increase coverage and validate invariants for container ID detection/extraction.

Copilot's findings

Tip

Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comments suppressed due to low confidence (3)

internal/sys/container_test.go:400

• TestDetectContainerID_EnvVarReturnsEmptyID can become flaky because it decides whether the env-var path is the “sole detection method” by only reading /proc/1/cgroup (and it ignores read errors). DetectContainerID() also checks /proc/self/cgroup, so on systems where /proc/1/cgroup doesn’t show container indicators (or can’t be read) but /proc/self/cgroup does, this test may assert an empty ID even though DetectContainerID() correctly returns a non-empty ID from the cgroup path. Consider checking both cgroup paths with error handling, and t.Skip when you can’t reliably force the env-var-only path.

	cgroupData, _ := os.ReadFile("/proc/1/cgroup")
	cgroupHasIndicator := containsAny(string(cgroupData), []string{"docker", "containerd", "kubepods", "lxc"})

	// Only assert the empty ID if the env-var path is the sole detection method.
	if !dockerEnvExists && !cgroupHasIndicator {
		_, containerID := DetectContainerID()

internal/sys/container_test.go:392

• The comment says “Override file-based detection by using the env var path”, but DetectContainerID() checks /.dockerenv and cgroups before RUNNING_IN_CONTAINER. This is misleading for future readers of the test; please reword to reflect that the env-var branch is only reached when file-based detection does not trigger.

	// Override file-based detection by using the env var path.
	// We can only guarantee an empty ID when /.dockerenv does not exist and
	// no cgroup indicator is present.
	_, dockerEnvErr := os.Stat("/.dockerenv")

internal/sys/container_test.go:397

• The container indicator list is duplicated here as a hard-coded slice. Since the test is in the same package, it can reuse the containerIndicators var from container.go to stay in sync if the indicators change.

	cgroupHasIndicator := containsAny(string(cgroupData), []string{"docker", "containerd", "kubepods", "lxc"})

• Files reviewed: 1/1 changed files
• Comments generated: 1