fake: fix delete element from map operation. #13
Conversation
k8s-ci-robot
added
the
cncf-cla: yes
k8s-ci-robot
added
the
size/S
fake.go
Outdated
| @@ -351,7 +351,7 @@ func (fake *Fake) run(tx *Transaction) (*FakeTable, error) { | |||
| return nil, fmt.Errorf("unhandled operation %q", op.verb) | |||
| } | |||
| case *Element: | |||
| if len(obj.Value) == 0 { | |||
| if len(obj.Set) != 0 { | |||
There was a problem hiding this comment.
I'd usually say:
| if len(obj.Set) != 0 { | |
| if obj.Set != "" { |
(because you don't care about the length, you care about whether it's set/unset.)
fake_test.go
Outdated
| @@ -243,7 +243,7 @@ func TestFakeRun(t *testing.T) { | |||
| } | |||
|
|
|||
| // Neither element should have been added | |||
| m = table.Maps["map1"] | |||
| m = fake.Table.Maps["map1"] | |||
| if m == nil || len(table.Maps) != 1 { | |||
| t.Fatalf("unexpected contents of table.Maps: %+v", table.Maps) | |||
There was a problem hiding this comment.
We still end up referring to the stale table value in both of the lines above.
You should probably get rid of the table variable altogether.
| } | ||
| m = fake.Table.Maps["map1"] | ||
| if len(m.Elements) != 1 { | ||
| t.Errorf("unexpected contents of map1: %+v", m) |
There was a problem hiding this comment.
If there's not already an Element deletion test then it would be good to check that we actually deleted the right element too (either do a FindElement on the deleted element and ensure it fails or on the remaining element and ensure it doesn't fail).
On delete map element value is not required. Fix unit test reference to the stale table. fake.Table is a new object after every transaction, so after each Run() it should be re-assigned. Signed-off-by: Nadia Pinaeva <n.m.pinaeva@gmail.com>
npinaeva
force-pushed
the
fix-fake-delete-element
branch
from
b6373cf to
8da7cec
Compare
k8s-ci-robot
added
size/M
|
/lgtm |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: danwinship, npinaeva The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
k8s-ci-robot
added
the
approved
On delete map element value is not required.
Fix unit test reference to the stale table. fake.Table is a new object after every transaction, so after each Run() it should be re-assigned.