Skip to content

Commit 228eff0

Browse files
authored
batches/executor: fix TestCreateChangesetSpecs (#556)
The `specWith()` and `taskWith()` helpers inadvertently mutate `defaultChangesetSpec` and `defaultTask` fields, rather than isolating their changes to just the returned spec and task, respectively. Practically speaking, this means that all tests have the same spec and task based on the last element in the `tests` slice, which means that all the test cases end up testing the same, identical thing. Instead, we must deep copy the spec and task before invoking the mutator function. I've chosen to pull in a third party package (specifically, copystructure) for this: naïvely using JSON to marshal and unmarshal doesn't work because most of the interesting task fields are excluded when marshalled to JSON.
1 parent a836bf7 commit 228eff0

File tree

3 files changed

+21
-3
lines changed

3 files changed

+21
-3
lines changed

go.mod

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ require (
1212
github.com/json-iterator/go v1.1.11
1313
github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51
1414
github.com/mattn/go-isatty v0.0.12
15+
github.com/mitchellh/copystructure v1.2.0
1516
github.com/neelance/parallel v0.0.0-20160708114440-4de9ce63d14c
1617
github.com/nsf/termbox-go v1.0.0 // indirect
1718
github.com/olekukonko/tablewriter v0.0.4 // indirect

go.sum

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,11 @@ github.com/mattn/go-runewidth v0.0.7/go.mod h1:H031xJmbD/WCDINGzjvQ9THkh0rPKHF+m
6767
github.com/mattn/go-runewidth v0.0.12 h1:Y41i/hVW3Pgwr8gV+J23B9YEY0zxjptBuCWEaxmAOow=
6868
github.com/mattn/go-runewidth v0.0.12/go.mod h1:RAqKPSqVFrSLVXbA8x7dzmKdmGzieGRCM46jaSJTDAk=
6969
github.com/mgutz/ansi v0.0.0-20170206155736-9520e82c474b/go.mod h1:01TrycV0kFyexm33Z7vhZRXopbI8J3TDReVlkTgMUxE=
70+
github.com/mitchellh/copystructure v1.2.0 h1:vpKXTN4ewci03Vljg/q9QvCGUDttBOGBIa15WveJJGw=
71+
github.com/mitchellh/copystructure v1.2.0/go.mod h1:qLl+cE2AmVv+CoeAwDPye/v+N2HKCj9FbZEVFJRxO9s=
7072
github.com/mitchellh/go-wordwrap v1.0.0/go.mod h1:ZXFpozHsX6DPmq2I0TCekCxypsnAUbP2oI0UX1GXzOo=
73+
github.com/mitchellh/reflectwalk v1.0.2 h1:G2LzWKi524PWgd3mLHV8Y5k7s6XUvT0Gef6zxSIeXaQ=
74+
github.com/mitchellh/reflectwalk v1.0.2/go.mod h1:mSTlrgnPZtwu0c4WaC2kGObEpuNDbx0jmZXqmk4esnw=
7175
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421 h1:ZqeYNhU3OHLH3mGKHDcjJRFFRrJa6eAM5H+CtDdOsPc=
7276
github.com/modern-go/concurrent v0.0.0-20180228061459-e0a39a4cb421/go.mod h1:6dJC0mAP4ikYIbvyc7fijjWJddQyLn8Ig3JB5CqoB9Q=
7377
github.com/modern-go/reflect2 v0.0.0-20180701023420-4b7aa43c6742 h1:Esafd1046DLDQ0W1YjYsBW+p8U2u7vzgW2SQVmlNazg=

internal/batches/executor/changeset_specs_test.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
"testing"
66

77
"github.com/google/go-cmp/cmp"
8+
"github.com/mitchellh/copystructure"
89
"github.com/sourcegraph/batch-change-utils/overridable"
910
"github.com/sourcegraph/src-cli/internal/batches"
1011
"github.com/sourcegraph/src-cli/internal/batches/git"
@@ -33,6 +34,12 @@ func TestCreateChangesetSpecs(t *testing.T) {
3334
}
3435

3536
specWith := func(s *batches.ChangesetSpec, f func(s *batches.ChangesetSpec)) *batches.ChangesetSpec {
37+
copy, err := copystructure.Copy(s)
38+
if err != nil {
39+
t.Fatalf("deep copying spec: %+v", err)
40+
}
41+
42+
s = copy.(*batches.ChangesetSpec)
3643
f(s)
3744
return s
3845
}
@@ -54,9 +61,15 @@ func TestCreateChangesetSpecs(t *testing.T) {
5461
Repository: testRepo1,
5562
}
5663

57-
taskWith := func(t *Task, f func(t *Task)) *Task {
58-
f(t)
59-
return t
64+
taskWith := func(task *Task, f func(task *Task)) *Task {
65+
copy, err := copystructure.Copy(task)
66+
if err != nil {
67+
t.Fatalf("deep copying task: %+v", err)
68+
}
69+
70+
task = copy.(*Task)
71+
f(task)
72+
return task
6073
}
6174

6275
defaultResult := executionResult{

0 commit comments

Comments
 (0)