Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding Govomi Debug Logging Capability and refactoring of integration tests #6893

Merged
merged 8 commits into from
Jun 2, 2016

Conversation

chrislovecnm
Copy link
Contributor

Also refactored some of the integration tests.

@jen20
Copy link
Contributor

jen20 commented May 27, 2016

Hi @chrislovecnm! Initial impressions of this look good - is it ready for integration as far as you are concerned? It would be nice to get it into our beta of Terraform 0.7. I have no way of running the acceptance tests so to some extend I'm relying on your or one of the other contributors running them. It would be great if you could post the output of make testacc TEST=./builtin/provider/vsphere as a comment on this? Thanks for your work here!

@chrislovecnm
Copy link
Contributor Author

@jen20 some of @dkalleg tweaks to our tests have broken running them in my environment :( I have reached out to him to determine what I need to do. The basic test and debug tests are passing on my test cluster.

@chrislovecnm chrislovecnm changed the title Adding Govomi Debug Logging Capability [WIP] Adding Govomi Debug Logging Capability May 27, 2016
@chrislovecnm
Copy link
Contributor Author

Need some TLC on the unit tests :(

@chrislovecnm chrislovecnm changed the title [WIP] Adding Govomi Debug Logging Capability Adding Govomi Debug Logging Capability May 29, 2016
@chrislovecnm
Copy link
Contributor Author

@jen20 here is the results of local testing. https://gist.github.com/chrislovecnm/80dab5b30fad211f6c40fde0bc6b6ac2

The failures are expected:

  • tf_file_test.vmdk - don't have a file for testing
  • ipv6 not working on my network, so it fails
  • createWithExistingVmdk - no existing vmdk

Fixed a couple more of the tests after my refactor. Hopefully the tests are more readable and less fragile now.

@chrislovecnm chrislovecnm changed the title Adding Govomi Debug Logging Capability Adding Govomi Debug Logging Capability and refactoring of integration tests May 29, 2016
@chrislovecnm chrislovecnm force-pushed the vsphere-govomi-debug branch from 1ab92b9 to 1d49df9 Compare May 31, 2016 18:14
@chrislovecnm
Copy link
Contributor Author

@jen20 any comments on this? I can squash the comments and we can get this in

@chrislovecnm
Copy link
Contributor Author

@jen20 @stack72 might we get a merge??

@stack72
Copy link
Contributor

stack72 commented Jun 1, 2016

Hi @chrislovecnm

this will need rebased before we can merge

P.

@chrislovecnm
Copy link
Contributor Author

@stack72 this is fun ...

@chrislovecnm chrislovecnm force-pushed the vsphere-govomi-debug branch from 1d49df9 to 2f9c59b Compare June 2, 2016 17:47
@chrislovecnm
Copy link
Contributor Author

@stack72 ~ Paul all good to go now. We need to get this in before more. This is a major rework of our tests, and it a pain to merge into :( If you don't mind I will let you squash, as I want to keep the commits the same on my branch

@chrislovecnm
Copy link
Contributor Author

@jen20 @stack72 ~ can I ask to get this in next. This is a large refactor on our testing code, and is not a easy merge. The quicker it is in, the less crazy rebases we will have.

@jen20
Copy link
Contributor

jen20 commented Jun 2, 2016

@chrislovecnm Can you post a log of the acceptance tests run with this set of commits integrated?

@chrislovecnm
Copy link
Contributor Author

@jen20

==> Checking that code complies with gofmt requirements...
/Users/clove/Workspace/bin/stringer
go generate $(go list ./... | grep -v /vendor/)
2016/06/02 11:41:06 Generated command/internal_plugin_list.go
TF_ACC=1 go test ./builtin/providers/vsphere -v -run TestAccVSphereVirtualMachine_mac_address -timeout 120m
=== RUN   TestAccVSphereVirtualMachine_mac_address
2016/06/02 11:41:29 [DEBUG] template=
resource "vsphere_virtual_machine" "mac_address" {
    name = "terraform-mac-address"
%s
    vcpu = 2
    memory = 1024
    network_interface {
        label = "%s"
        mac_address = "%s"
    }
    disk {
%s
        template = "%s"
    }
}
2016/06/02 11:41:29 [DEBUG] template config=
resource "vsphere_virtual_machine" "mac_address" {
    name = "terraform-mac-address"
    datacenter = "Engpipeline Datacenter"
    cluster = "Engpipeline-cluster"

    vcpu = 2
    memory = 1024
    network_interface {
        label = "VM-vlan409"
        mac_address = "f6:5c:89:2b:a0:64"
    }
    disk {
        datastore = "BEL-5798813-EMC-VMAX1789-LUN-06"

        template = "centos-clove-tpl"
    }
}
--- PASS: TestAccVSphereVirtualMachine_mac_address (153.03s)
PASS
ok      github.com/hashicorp/terraform/builtin/providers/vsphere    153.056s

@chrislovecnm
Copy link
Contributor Author

@jen20 btw after the PR I am working, the one after this one, on I am going to get Jenkins running our integration testing on one of our clusters.

@chrislovecnm
Copy link
Contributor Author

@jen20 let me now if that is what you need. Always appreciate your and @stack72's help!!

@chrislovecnm
Copy link
Contributor Author

This address #6964 as well

@stack72
Copy link
Contributor

stack72 commented Jun 2, 2016

LGTM! Thanks @chrislovecnm :)

@stack72 stack72 merged commit 7b449b6 into hashicorp:master Jun 2, 2016
@chrislovecnm
Copy link
Contributor Author

Appreciated @stack72 ... now I get the joy of merging my mess back on to master ... rebase hell today :(

markpeek added a commit to markpeek/terraform that referenced this pull request Jun 6, 2016
The change to add govmomi debug paths (hashicorp#6893) required user input even
when the env was not set. This change defaults those values appropriately.
stack72 pushed a commit that referenced this pull request Jun 7, 2016
The change to add govmomi debug paths (#6893) required user input even
when the env was not set. This change defaults those values appropriately.
@ghost
Copy link

ghost commented Apr 25, 2020

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you have found a problem that seems similar to this, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@ghost ghost locked and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants