Add unit tests for VCH create#6764
Conversation
jzt
force-pushed
the
vic-machine/6018
branch
2 times, most recently
from
41a6a1c to
f4b1c0b
Compare
zjs
force-pushed
the
feature/vic-machine-service
branch
from
501e8a9 to
208aa39
Compare
zjs
force-pushed
the
feature/vic-machine-service
branch
from
208aa39 to
729d15f
Compare
jzt
force-pushed
the
vic-machine/6018
branch
from
f4b1c0b to
54560ad
Compare
| expected = m.ID | ||
| actual, err = fromManagedObject(op, mf, "t", m) | ||
| assert.NoError(t, err, "Expected nil error, got %#v", err) | ||
| assert.Equal(t, expected, actual, "Expected %s, got %s", expected, actual) |
There was a problem hiding this comment.
Curious (since I'm new to unit testing in go): what's the goal of explicitly passing Expected %s, got %s", expected, actual? Shouldn't the default behavior be to print something along those lines?
There was a problem hiding this comment.
I've been using this pattern since I started because of some other test I referenced in the beginning. :)
But, upon investigating:
func TestTest(t *testing.T) {
assert.Equal(t, "expected", "actual")
}--- FAIL: TestTest (0.00s)
Error Trace: vch_create_test.go:47
Error: Not equal: "expected" (expected)
!= "actual" (actual)
| assert.NoError(t, err, "Expected nil error, got %#v", err) | ||
| assert.Equal(t, expected, actual, "Expected %s, got %s", expected, actual) | ||
|
|
||
| m.ID = "testID" |
There was a problem hiding this comment.
This doesn't clear the Name. Should there be a separate test case for having an ID and no name?
There was a problem hiding this comment.
Edit: sorry, misread this the first time around:
fromManagedObject will return finder.Element.Path if m.ID is not empty. Otherwise, it will return m.Name.
So even if there's an ID and no Name, we still get ID.
| "github.com/vmware/vic/pkg/trace" | ||
| ) | ||
|
|
||
| type mockFinder struct { |
There was a problem hiding this comment.
Would it be valuable to make this embed a testify mock.Mock so that we can verify that the Element method is called with the expected arguments?
There was a problem hiding this comment.
Sure, I will look into this.
There was a problem hiding this comment.
Will add this at a later time as per our slack convo.
jzt
force-pushed
the
vic-machine/6018
branch
2 times, most recently
from
ba864e1 to
b983a04
Compare
zjs
force-pushed
the
feature/vic-machine-service
branch
from
729d15f to
dd3128a
Compare
jzt
force-pushed
the
vic-machine/6018
branch
from
b983a04 to
20693d8
Compare
cmd/vic-machine/create/create.go
Outdated
| "github.com/vmware/vic/lib/install/data" | ||
| "github.com/vmware/vic/lib/install/management" | ||
| "github.com/vmware/vic/lib/install/validate" | ||
|
|
There was a problem hiding this comment.
nit: I don't think this line should be here
4 participants
This main purpose of this change is to test the
buildCreatefunction in vch_create.go, to verify that theCreatestruct that is generated matches the same struct that would be generated using the CLI.There are some tests around help functions as well.
Fixes #6018