# Create a new branch
git checkout -b test-assist/ops-partition-lockedwriter-2026-04-22
# Apply the patch below (save as .patch file first)
# Push and create PR
git push origin test-assist/ops-partition-lockedwriter-2026-04-22
gh pr create --title '[Test Improver] Add tests for ops partition/lockedWriter and uniqueOrgs' --base main
```
<details>
<summary>Show patch (178 lines)</summary>
```
From a353715ea34f3fba7cdaa1db0c92fb5026589312 Mon Sep 17 00:00:00 2001
From: "github-actions[bot]" <github-actions[bot]`@users`.noreply.github.com>
Date: Wed, 22 Apr 2026 20:36:03 +0000
Subject: [PATCH] test: add ops partition/lockedWriter tests and uniqueOrgs
test
- Add TestPartitionRebuildTargets (4 cases) to internal/ops/ops_test.go
- Add TestLockedWriter_serializesWrites and TestLockedWriter_concurrentWritesDoNotInterleave
- Add TestApplyContainerImageExtras_nilManager/nilConfig/withConfig
- Add TestUniqueOrgs to internal/doctor/doctor_test.go
🤖 Test Improver
---
internal/doctor/doctor_test.go | 20 +++++
internal/ops/ops_test.go | 158 +++++++++++++++++++++++++++++++++
2 files changed, 178 insertions(+)
create mode 100644 internal/ops/ops_test.go
diff --git a/internal/doctor/doctor_test.go b/internal/doctor/doctor_test.go
index bc29f53..c79ae60 100644
--- a/internal/doctor/doctor_test.go
+++ b/internal/doctor/doctor_test.go
@@ -50,6 +50,26 @@ func TestUniqueRepos(t *testing.T) {
}
}
+func TestUniqueOrgs(t *testing.T) {
+ t.Parallel()
+ runners := []config.RunnerConfig{
+ {Org: "z-org", Host: "a"},
+ {Org: "a-org", Host: "a"},
+ {Org: "a-org", Host: "b"},
+ {Repo: "x/y", Host: "c"}, // repos should be skipped
+ }
+ got := uniqueOrgs(runners)
+ want := []string{"a-org", "z-org"}
+ if len(got) != len(want) {
+ t.Fatalf("len %d, want %d", len(got), len(want))
+ }
+ for i := range want {
+ if got[i] != want[i] {
+ t.Fatalf("got %v, want %v", got, want)
+ }
+ }
+}
+
func TestUniqueHostNames(t *testing.T) {
t.Parallel()
runners := []config.RunnerConfig{
diff --git a/internal/ops/ops_test.go b/internal/ops/ops_test.go
new file mode 100644
index 0000000..1579e53
--- /dev/null
+++ b/internal/ops/ops_test.go
@@ -0,0 +1,158 @@
+package ops
+
+import (
+ "bytes"
+ "sync"
+ "testing"
+
+ "github.com/an-lee/gh-sr/internal/config"
+ "github.com/an-lee/gh-sr/internal/runner"
+)
+
+func TestPartitionRebuildTargets(t *testing.T) {
+ t.Parallel()
+ cases := []struct {
+ name string
+ runners []config.RunnerConfig
+ wantCont []string
+ wantSkip []string
+ }{
+ {
+ name: "all_native",
+ runners: []config.RunnerConfig{
+ {Name: "r1", Repo: "o/r", Host: "h", Count: 1},
+ {Name: "r2", Repo: "o/r", Host: "h", Count: 1},
+ },
+ wantCont: nil,
+ wantSkip: []string{"r1", "r2"},
+ },
+ {
+ name: "all_container",
+ runners: []config.RunnerConfig{
+ {Name: "c1", Repo: "o/r", Host: "h", Count: 1, RunnerMode: config.RunnerModeContainer},
+ {Name: "c2", Repo: "o/r", Host: "h", Count: 1, RunnerMode: config.RunnerModeContainer},
+ },
+ wantCont: []string{"c1", "c2"},
+ wantSkip: nil,
+ },
+ {
+ name: "mixed",
+ runners: []config.RunnerConfig{
+ {Name: "n1", Repo: "o/r", Host: "h", Count: 1},
+ {Name: "c1", Repo: "o/r", Host: "h", Count: 1, RunnerMode: config.RunnerModeContainer},
+ {Name: "n2", Repo: "o/r", Host: "h", Count: 1},
+ },
+ wantCont: []string{"c1"},
+ wantSkip: []string{"n1", "n2"},
+ },
+ {
+ name: "empty",
+ runners: nil,
+ wantCont: nil,
+ wantSkip: nil,
+ },
+ }
+ for _, tc := range cases {
+ t.Run(tc.name, func(t *testing.T) {
+ t.Parallel()
+ cont, skip := partitionRebuildTargets(tc.runners)
+ if len(cont) != len(tc.wantCont) {
+ t.Fatalf("container len: got %d want %d", len(cont), len(tc.wantCont))
+ }
+ if len(skip) != len(tc.wantSkip) {
+ t.Fatalf("skipped len: got %d want %d", len(skip), len(tc.wantSkip))
+ }
+ for i := range cont {
+ if cont[i].Name != tc.wantCont[i] {
+ t.Errorf("cont[%d]: got %q want %q", i, cont[i].Name, tc.wantCont[i])
+ }
+ }
+ for i := range skip {
+ if skip[i].Name != tc.wantSkip[i] {
+ t.Errorf("skip[%d]: got %q want %q", i, skip[i].Name, tc.wantSkip[i])
+ }
+ }
+ })
+ }
+}
+
+func TestLockedWriter_serializesWrites(t *testing.T) {
+ t.Parallel()
+ var buf bytes.Buffer
+ lw := &lockedWriter{w: &buf}
+
+ var wg sync.WaitGroup
+ n := 100
+ wg.Add(n)
+ for i := 0; i < n; i++ {
+ go func() {
+ defer wg.Done()
+ _, _ = lw.Write([]byte("x"))
+ }()
+ }
+ wg.Wait()
+
+ if buf.Len() != n {
+ t.Errorf("lockedWriter wrote %d bytes, want %d", buf.Len(), n)
+ }
+}
+
+func TestLockedWriter_concurrentWritesNoDataLoss(t *testing.T) {
+ t.Parallel()
+ var buf bytes.Buffer
+ lw := &lockedWriter{w: &buf}
+
+ var wg sync.WaitGroup
+ writers := 5
+ writesPerWriter := 100
+
+ for w := 0; w < writers; w++ {
+ wg.Add(1)
+ go func(id int) {
+ defer wg.Done()
+ for i := 0; i < writesPerWriter; i++ {
+ _, _ = lw.Write([]byte{byte(id)})
+ }
+ }(w)
+ }
+ wg.Wait()
+
+ expectedLen := writers * writesPerWriter
+ if buf.Len() != expectedLen {
+ t.Errorf("expected %d bytes, got %d", expectedLen, buf.Len())
+ }
+}
+
+func TestApplyContainerImageExtras_nilManager(t *testing.T) {
+ t.Parallel()
+ applyContainerImageExtras(nil, nil)
+}
+
+func TestApplyContainerImageExtras_nilConfig(t *testing.T) {
+ t.Parallel()
+ mgr := &runner.Manager{}
+ applyContainerImageExtras(mgr, nil)
+ if mgr.ContainerImageExtraApt != nil {
+ t.Errorf("expected nil ExtraApt, got %v", mgr.ContainerImageExtraApt)
+ }
+}
+
+func TestApplyContainerImageExtras_withConfig(t *testing.T) {
+ t.Parallel()
+ mgr := &runner.Manager{}
+ cfg := &config.Config{
+ ContainerRunnerImage: config.ContainerRunnerImageConfig{
+ ExtraAptPackages: []string{"curl", "git"},
+ },
+ }
+ applyContainerImageExtras(mgr, cfg)
+ if len(mgr.ContainerImageExtraApt) != 2 {
+ t.Fatalf("got %d packages, want 2", len(mgr.ContainerImageExtraApt))
+ }
+ if mgr.ContainerImageExtraApt[0] != "curl" || mgr.ContainerImageExtraApt[1] != "git" {
+ t.Errorf("unexpected packages: %v", mgr.ContainerImageExtraApt)
+ }
+}
🤖 This was created by Test Improver, an automated AI assistant focused on improving tests for this repository.
Goal and Rationale
Three functions in
internal/ops/ops.goand one ininternal/doctor/doctor.gohad zero tests:partitionRebuildTargets(ops.go): splits runners into container-mode vs native rows for rebuild logic. Critical for correctness — wrong partitioning silently skips rebuilds.lockedWriter(ops.go): serializes concurrent writes to anio.Writer. Used byrunPerHostParallel— incorrect serialization causes interleaved output.applyContainerImageExtras(ops.go): copies extra apt packages from config to manager. Edge cases (nil manager, nil config) were untested.uniqueOrgs(doctor.go): deduplicates and sorts org names from runner configs. Was missing a direct test even thoughuniqueReposanduniqueHostNameshad coverage.Approach
internal/ops/ops_test.go(new file, 158 lines):TestPartitionRebuildTargetsTestLockedWriter_serializesWritesTestLockedWriter_concurrentWritesNoDataLossTestApplyContainerImageExtras_nilManagerTestApplyContainerImageExtras_nilConfigTestApplyContainerImageExtras_withConfiginternal/doctor/doctor_test.go— addsTestUniqueOrgs:Test Status
go.modrequires Go 1.25.0 but the sandbox has Go 1.24.13 with network firewalled.gofmt -lconfirms correct formatting. CI will validate.Reproducibility
To create a pull request with the changes:
Warning
The following domain was blocked by the firewall during workflow execution:
proxy.golang.orgTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.