Skip to content

[test-improver] Improve tests for sys package#7848

Merged
lpcox merged 2 commits into
mainfrom
test-improver/sys-container-tests-ac6d64d1a98fe4c2
Jun 21, 2026
Merged

[test-improver] Improve tests for sys package#7848
lpcox merged 2 commits into
mainfrom
test-improver/sys-container-tests-ac6d64d1a98fe4c2

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

Test Improvements: internal/sys/container_test.go + container_detect_test.go

File Analyzed

  • Test Files: internal/sys/container_test.go, internal/sys/container_detect_test.go
  • Package: internal/sys

Improvements Made

1. Scanner-overflow coverage (new test case)

Added a test case "single line exceeding scanner buffer limit returns empty string" to TestExtractContainerIDFromContent. This covers the previously-untested scanner.Err() != nil branch in extractContainerIDFromContent (triggered when a single line exceeds bufio.MaxScanTokenSize = 65,536 bytes):

{
    name:  "single line exceeding scanner buffer limit returns empty string",
    input: strings.Repeat("x", bufio.MaxScanTokenSize+1),
    want:  "",
},

2. Parallel test execution

Added t.Parallel() to all pure-function tests (safe to run concurrently — no environment mutation, no shared state):

  • TestExtractContainerIDFromContent + all 16 subtests
  • TestContainsAny_Helper + all subtests
  • 6 × TestExtractContainerIDFromCgroupFiles_* tests in container_detect_test.go (temp-file only, no t.Setenv)

3. Increased Coverage

  • Previous coverage: extractContainerIDFromContent → 92.3%, package total → 98.9%
  • New coverage: extractContainerIDFromContent100.0%, package total → 100.0%
  • Improvement: +1.1% package, +7.7% on the targeted function

Test Execution

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

All tests pass with -race flag enabled. Tests run multiple times with consistent results.

Why These Changes?

extractContainerIDFromContent used bufio.Scanner to parse cgroup content but the error path (scanner.Err() != nil) was never exercised — it requires a line longer than 64 KiB, an unusual but valid edge case. Adding the overflow test ensures the error path is verified. The parallelism additions are low-risk improvements to pure-function tests, reducing wall-clock test time with no behavior changes.


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

Generated by Test Improver · 1.8K AIC · ⊞ 29.5K ·

- Add t.Parallel() to TestExtractContainerIDFromContent and all subtests
- Add t.Parallel() to TestContainsAny_Helper and all subtests
- Add t.Parallel() to all TestExtractContainerIDFromCgroupFiles_* tests
- Add scanner-overflow test case (bufio.MaxScanTokenSize+1 line) to cover
  the scanner.Err() != nil path in extractContainerIDFromContent

Coverage: internal/sys 98.9% -> 100.0%

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@lpcox lpcox marked this pull request as ready for review June 21, 2026 00:43
Copilot AI review requested due to automatic review settings June 21, 2026 00:43

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves the internal/sys test suite by adding explicit coverage for an edge-case error path in container ID parsing and by enabling parallel execution for tests that are safe to run concurrently. This helps keep the sys package’s tests more robust while reducing overall test wall-clock time.

Changes:

  • Added a new test case to exercise the bufio.Scanner overflow/error branch in extractContainerIDFromContent.
  • Marked pure-function / temp-file-only tests as parallelizable via t.Parallel() (including table subtests).
Show a summary per file
File Description
internal/sys/container_test.go Adds scanner-overflow test case for extractContainerIDFromContent and parallelizes pure-function table tests.
internal/sys/container_detect_test.go Parallelizes temp-file-only tests for extractContainerIDFromCgroupFiles to reduce runtime without changing behavior.

Copilot's findings

Tip

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

  • Files reviewed: 2/2 changed files
  • Comments generated: 0

Copilot AI commented Jun 21, 2026

Copy link
Copy Markdown
Contributor

@copilot fix these failing ci checks https://github.com/github/gh-aw-mcpg/actions/runs/27888246857/job/82528413281?pr=7848 https://github.com/github/gh-aw-mcpg/actions/runs/27888246857/job/825284133...

Fixed in e871e13. I removed duplicate TLS declarations/tests (internal/httputil/tls_config.go and internal/httputil/tls_config_test.go) that were causing the lint, integration-build, and race-test failures. I also verified locally with make lint, make build, make test-race, and make agent-finished.

Copilot AI requested a review from lpcox June 21, 2026 16:02
Copilot finished work on behalf of lpcox June 21, 2026 16:02
@lpcox lpcox merged commit d1a2acd into main Jun 21, 2026
26 checks passed
@lpcox lpcox deleted the test-improver/sys-container-tests-ac6d64d1a98fe4c2 branch June 21, 2026 16:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants