Skip to content
This repository has been archived by the owner on Nov 9, 2020. It is now read-only.

Add log to dump kv_str #1388

Merged
merged 5 commits into from
Jun 12, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions esx_service/utils/kvESX.py
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,8 @@ def load(volpath):
try:
return json.loads(kv_str)
except ValueError:
# Adding this log for DEBUG
logging.warning("kv_str from meta file is %s ", kv_str)
logging.exception("Failed to decode meta-data for %s", volpath)
return None

Expand Down
2 changes: 1 addition & 1 deletion tests/e2e/volume_access_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// The goal of this test suite is to verify read/write consistency on volumes
// in accordance with the access updates on the volume

// +build runonce
// +build runalways
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is the default and hence need not be specified.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Specified explicitly is more clear, and I found "basic_test.go" also specify "runalways".

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually we do need to add "runalways" explicitly - @shuklanirdesh82 can you please explain the difference between default (no tag) and "runalways"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a limitation of go test -tags <tag_name> command. There is no way to exclude any tag, as per -tags tests are picked up with passed value and run. Any test files not labeled with tag falls into default category and end up running multiple times hence tag is required.

Let me explain with following example. let's say we have 3 files a_test.go, b_test.go, c_test.go.

a_test.go => // +build runalways
b_test.go => // +build runonce
c_test.go => no label 

We have two targets runalways and runonce (https://github.com/vmware/docker-volume-vsphere/blob/master/vmdk_plugin/Makefile#L357).

targets runalways: includes and ends up running a_test.go & c_test.go
targets runonce: includes and ends up running b_test.go & c_test.go

c_test.go falls into default and ends up running multiple times hence the tag explicitly needed.


package e2e

Expand Down
4 changes: 4 additions & 0 deletions tests/e2e/vsan_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,10 @@ func (s *VsanTestSuite) SetUpSuite(c *C) {
}
}

func (s *VsanTestSuite) SetUpTest(c *C) {
s.policyList = []string{}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor: As this is cleaning the list, this step should go to teardown.
You can empty this slice as s.policyList = s.policyList[:0]

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I put it in the setup is to make sure each test is running from a clean state. I think either way works. So I will keep it as it is.

}

func (s *VsanTestSuite) TearDownTest(c *C) {
if s.volumeName != "" {
out, err := dockercli.DeleteVolume(s.config.DockerHosts[0], s.volumeName)
Expand Down