Skip to content
This repository was archived by the owner on Jun 27, 2023. It is now read-only.

add tests for callSet Add & Remove #108

Merged
merged 2 commits into from
Sep 16, 2017
Merged

add tests for callSet Add & Remove #108

merged 2 commits into from
Sep 16, 2017

Conversation

natebrennand
Copy link

Adds tests to complement the changes introduced in #107.

I validated that these tests would have caught the ordering bug fixed by
PR #107 by reverting the change locally:

revert diff:

diff --git a/gomock/callset.go b/gomock/callset.go
index 05d6fa2..c652f42 100644
--- a/gomock/callset.go
+++ b/gomock/callset.go
@@ -44,7 +44,11 @@ func (cs callSet) Remove(call *Call) {
        for i, c := range sl {
                if c == call {
                        // maintain order for remaining calls
-                       methodMap[call.method] = append(sl[:i], sl[i+1:]...)
+                       // methodMap[call.method] = append(sl[:i], sl[i+1:]...)
+                       if len(sl) > 1 {
+                               sl[i] = sl[len(sl)-1]
+                       }
+                       methodMap[call.method] = sl[:len(sl)-1]
                        break
                }
        }

test output:

=== RUN   TestCallSetAdd
--- PASS: TestCallSetAdd (0.00s)
=== RUN   TestCallSetRemove
--- FAIL: TestCallSetRemove (0.00s)
	callset_test.go:61: found call 1 after call 9
	callset_test.go:61: found call 8 after call 9
	callset_test.go:61: found call 2 after call 8
	callset_test.go:61: found call 8 after call 9
	callset_test.go:61: found call 7 after call 8
	callset_test.go:61: found call 3 after call 7
	callset_test.go:61: found call 8 after call 9
	callset_test.go:61: found call 7 after call 8
	callset_test.go:61: found call 6 after call 7
	callset_test.go:61: found call 4 after call 6
	callset_test.go:61: found call 8 after call 9
	callset_test.go:61: found call 7 after call 8
	callset_test.go:61: found call 6 after call 7
	callset_test.go:61: found call 5 after call 6
	callset_test.go:61: found call 8 after call 9
	callset_test.go:61: found call 7 after call 8
	callset_test.go:61: found call 6 after call 7
	callset_test.go:61: found call 8 after call 9
	callset_test.go:61: found call 7 after call 8
	callset_test.go:61: found call 8 after call 9

Nate Brennand added 2 commits September 15, 2017 14:32
Adds tests to complement the changes introduced in #107.

I validated that these tests would have caught the ordering bug fixed by
PR #107 by reverting the change locally:

revert diff:
```diff
diff --git a/gomock/callset.go b/gomock/callset.go
index 05d6fa2..c652f42 100644
--- a/gomock/callset.go
+++ b/gomock/callset.go
@@ -44,7 +44,11 @@ func (cs callSet) Remove(call *Call) {
        for i, c := range sl {
                if c == call {
                        // maintain order for remaining calls
-                       methodMap[call.method] = append(sl[:i], sl[i+1:]...)
+                       // methodMap[call.method] = append(sl[:i], sl[i+1:]...)
+                       if len(sl) > 1 {
+                               sl[i] = sl[len(sl)-1]
+                       }
+                       methodMap[call.method] = sl[:len(sl)-1]
                        break
                }
        }
```

test output:
```
=== RUN   TestCallSetAdd
--- PASS: TestCallSetAdd (0.00s)
=== RUN   TestCallSetRemove
--- FAIL: TestCallSetRemove (0.00s)
	callset_test.go:61: found call 1 after call 9
	callset_test.go:61: found call 8 after call 9
	callset_test.go:61: found call 2 after call 8
	callset_test.go:61: found call 8 after call 9
	callset_test.go:61: found call 7 after call 8
	callset_test.go:61: found call 3 after call 7
	callset_test.go:61: found call 8 after call 9
	callset_test.go:61: found call 7 after call 8
	callset_test.go:61: found call 6 after call 7
	callset_test.go:61: found call 4 after call 6
	callset_test.go:61: found call 8 after call 9
	callset_test.go:61: found call 7 after call 8
	callset_test.go:61: found call 6 after call 7
	callset_test.go:61: found call 5 after call 6
	callset_test.go:61: found call 8 after call 9
	callset_test.go:61: found call 7 after call 8
	callset_test.go:61: found call 6 after call 7
	callset_test.go:61: found call 8 after call 9
	callset_test.go:61: found call 7 after call 8
	callset_test.go:61: found call 8 after call 9
```
@balshetzer
Copy link
Collaborator

Thanks!

@balshetzer balshetzer merged commit c7d0ee7 into golang:master Sep 16, 2017
@natebrennand natebrennand deleted the test-callset-remove-preserve-ordering branch September 16, 2017 22:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants