diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 66b4001f..b6bcfd26 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -34,12 +34,13 @@ related API documentation will benefit users. ### Linters -golangci-lint is used to verify that the style of the code remains consistent. - -`make lint` can be used to verify style before creating a pull request. +`golangci-lint` is used to verify that the style of the code remains consistent. Before committing, it's a good idea to run `goimports -w .`. -([goimports](https://pkg.go.dev/golang.org/x/tools/cmd/goimports?tab=doc)) +([goimports](https://pkg.go.dev/golang.org/x/tools/cmd/goimports?tab=doc)) and +`gofmt -w *.go`. ([gofmt](https://golang.org/cmd/gofmt/)) + +`make lint` can be used to verify style before creating a pull request. ## Building and Testing @@ -62,9 +63,10 @@ make test BUILD=local ### Acceptance Tests -If you want to run tests against the actual Packet API, you must set envvar -`PACKET_TEST_ACTUAL_API` to non-empty string for the `go test`. The device tests -wait for the device creation, so it's best to run a few in parallel. +If you want to run tests against the actual Packet API, you must set the +environment variable `PACKET_TEST_ACTUAL_API` to a non-empty string and set +`PACKNGO_TEST_RECORDER` to `disabled`. The device tests wait for the device +creation, so it's best to run a few in parallel. To run a particular test, you can do @@ -79,6 +81,36 @@ string, for example: PACKNGO_DEBUG=1 PACKNGO_TEST_ACTUAL_API=1 go test -v -run=TestAccVolumeUpdate ``` +### Test Fixtures + +By default, `go test ./...` will skip most of the tests unless +`PACKNGO_TEST_ACTUAL_API` is non-empty. + +With the `PACKNGO_TEST_ACTUAL_API` environment variable set, tests will be run +against the Packet API, creating real infrastructure and incurring costs. + +The `PACKNGO_TEST_RECORDER` variable can be used to record and playback API +responses to test code changes without the delay and costs of making actual API +calls. When unset, `PACKNGO_TEST_RECORDER` acts as though it was set to +`disabled`. This is the default behavior. This default behavior may change in +the future once fixtures are available for all tests. + +When `PACKNGO_TEST_RECORDER` is set to `play`, tests will playback API responses +from recorded HTTP response fixtures. This is idea for refactoring and making +changes to request and response handling without introducing changes to the data +sent or received by the Packet API. + +When adding support for new end-points, recorded test sessions should be added. +Record the HTTP interactions to fixtures by setting the environment variable +`PACKNGO_TEST_RECORDER` to `record`. + +The fixtures are automatically named according to the test they were run from. +They are placed in `fixtures/`. The API token used during authentication is +automatically removed from these fixtures. Nonetheless, caution should be +exercised before committing any fixtures into the project. Account details +includes API tokens, contact, and payment details could easily be leaked by +committing fixtures that haven't been thoroughly reviewed. + ### Automation (CI/CD) Today, Drone tests pull requests using tests defined in diff --git a/README.md b/README.md index 185a81e1..bd4e62b7 100644 --- a/README.md +++ b/README.md @@ -128,6 +128,7 @@ func main() { listProjectsAndUsers(loMembersOut) } ``` + ## Contributing diff --git a/apikeys_test.go b/apikeys_test.go index 1690cfac..b4699d1b 100644 --- a/apikeys_test.go +++ b/apikeys_test.go @@ -60,7 +60,8 @@ func TestAccAPIKeyListProject(t *testing.T) { func TestAccAPIKeyListUser(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) t.Parallel() - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() nKeys := 10 @@ -95,7 +96,8 @@ func TestAccAPIKeyListUser(t *testing.T) { func TestAccAPIKeyCreateUser(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) t.Parallel() - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() req := APIKeyCreateRequest{ Description: "PACKNGO_TEST_KEY_DELETE_ME-" + randString8(), ReadOnly: true, diff --git a/batches_test.go b/batches_test.go index e877e2c6..addb6f14 100644 --- a/batches_test.go +++ b/batches_test.go @@ -58,7 +58,7 @@ func TestAccInstanceBatches(t *testing.T) { } for _, d := range batch.Devices { - waitDeviceActive(d.ID, c) + waitDeviceActive(t, c, d.ID) } _, err = c.Batches.Delete(batchID, true) diff --git a/bgp_neighbors_test.go b/bgp_neighbors_test.go index c6f0cbef..9c62e227 100644 --- a/bgp_neighbors_test.go +++ b/bgp_neighbors_test.go @@ -21,10 +21,7 @@ func createBGPDevice(t *testing.T, c *Client, projectID string) *Device { t.Fatal(err) } - d, err = waitDeviceActive(d.ID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, d.ID) aTrue := true _, _, err = c.BGPSessions.Create(d.ID, @@ -57,7 +54,7 @@ func TestAccBGPNeighbors(t *testing.T) { d := createBGPDevice(t, c, projectID) - defer c.Devices.Delete(d.ID, false) + defer deleteDevice(t, c, d.ID, false) bgpNeighbors, _, err := c.Devices.ListBGPNeighbors(d.ID, nil) if err != nil { diff --git a/bgp_sessions_test.go b/bgp_sessions_test.go index 63c3ae38..9b8b6085 100644 --- a/bgp_sessions_test.go +++ b/bgp_sessions_test.go @@ -35,11 +35,9 @@ func TestAccBGPSession(t *testing.T) { if err != nil { t.Fatal(err) } + defer deleteDevice(t, c, d.ID, false) - d, err = waitDeviceActive(d.ID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, d.ID) aTrue := true @@ -111,6 +109,4 @@ func TestAccBGPSession(t *testing.T) { if err == nil { t.Fatal("Session not deleted") } - - c.Devices.Delete(d.ID, false) } diff --git a/capacities_test.go b/capacities_test.go index f3f78ba1..ea4da54d 100644 --- a/capacities_test.go +++ b/capacities_test.go @@ -7,7 +7,8 @@ import ( func TestAccCheckCapacity(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() input := &CapacityInput{ []ServerInfo{ diff --git a/devices_test.go b/devices_test.go index 2a486e4a..6f72bde8 100644 --- a/devices_test.go +++ b/devices_test.go @@ -8,27 +8,66 @@ import ( "time" ) -func waitDeviceActive(id string, c *Client) (*Device, error) { +func waitDeviceActive(t *testing.T, c *Client, id string) *Device { // 15 minutes = 180 * 5sec-retry for i := 0; i < 180; i++ { <-time.After(5 * time.Second) d, _, err := c.Devices.Get(id, nil) if err != nil { - return nil, err + t.Fatal(err) + return nil } if d.State == "active" { - return d, nil + return d } if d.State == "failed" { - return nil, fmt.Errorf("device %s provisioning failed", id) + t.Fatal(fmt.Errorf("device %s provisioning failed", id)) + return nil } } - return nil, fmt.Errorf("device %s is still not active after timeout", id) + + t.Fatal(fmt.Errorf("device %s is still not active after timeout", id)) + return nil } -func deleteDevice(t *testing.T, c *Client, id string) { - _, err := c.Devices.Delete(id, false) - if err != nil { +func deleteDevice(t *testing.T, c *Client, id string, force bool) { + if _, err := c.Devices.Delete(id, force); err != nil { + t.Fatal(err) + } +} + +func deleteSpotMarketRequest(t *testing.T, c *Client, id string, force bool) { + if _, err := c.SpotMarketRequests.Delete(id, force); err != nil { + t.Fatal(err) + } +} + +func deleteSSHKey(t *testing.T, c *Client, id string) { + if _, err := c.SSHKeys.Delete(id); err != nil { + t.Fatal(err) + } +} + +func deleteVolume(t *testing.T, c *Client, id string) { + if _, err := c.Volumes.Delete(id); err != nil { + t.Fatal(err) + } +} + +func deleteVolumeAttachments(t *testing.T, c *Client, id string) { + if _, err := c.VolumeAttachments.Delete(id); err != nil { + t.Fatal(err) + } +} + +func deleteProjectIP(t *testing.T, c *Client, id string) { + if _, err := c.ProjectIPs.Remove(id); err != nil { + t.Fatal(err) + } +} + +func deleteProjectVirtualNetwork(t *testing.T, c *Client, id string) { + if _, err := c.ProjectVirtualNetworks.Delete(id); err != nil { t.Fatal(err) } } @@ -56,14 +95,11 @@ func TestAccDeviceUpdate(t *testing.T) { if err != nil { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) dID := d.ID - d, err = waitDeviceActive(dID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, dID) if len(d.RootPassword) == 0 { t.Fatal("root_password is empty or non-existent") @@ -110,14 +146,11 @@ func TestAccDeviceBasic(t *testing.T) { if err != nil { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) dID := d.ID - d, err = waitDeviceActive(dID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, dID) if len(d.ShortID) == 0 { t.Fatal("Device should have shortID") @@ -189,12 +222,9 @@ func TestAccDevicePXE(t *testing.T) { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) - d, err = waitDeviceActive(d.ID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, d.ID) // Check that settings were persisted if !d.AlwaysPXE { @@ -254,12 +284,9 @@ func TestAccDeviceAssignGlobalIP(t *testing.T) { if err != nil { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) - d, err = waitDeviceActive(d.ID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, d.ID) req := IPReservationRequest{ Type: "global_ipv4", @@ -369,7 +396,7 @@ func TestAccDeviceCreateWithReservedIP(t *testing.T) { if err != nil { t.Fatal(err) } - defer c.ProjectIPs.Remove(reservation.ID) + defer deleteProjectIP(t, c, reservation.ID) cr := DeviceCreateRequest{ Hostname: hn, @@ -390,11 +417,9 @@ func TestAccDeviceCreateWithReservedIP(t *testing.T) { if err != nil { t.Fatal(err) } - d, err = waitDeviceActive(d.ID, c) - if err != nil { - t.Fatal(err) - } - defer c.Devices.Delete(d.ID, false) + d = waitDeviceActive(t, c, d.ID) + + defer deleteDevice(t, c, d.ID, false) reservation, _, err = c.ProjectIPs.Get(reservation.ID, nil) if err != nil { @@ -429,12 +454,9 @@ func TestAccDeviceAssignIP(t *testing.T) { if err != nil { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) - d, err = waitDeviceActive(d.ID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, d.ID) req := IPReservationRequest{ Type: PublicIPv4, @@ -562,12 +584,9 @@ func TestAccDeviceAttachVolumeForceDelete(t *testing.T) { if err != nil { t.Fatal(err) } - //defer deleteDevice(t, c, d.ID) + // defer deleteDevice(t, c, d.ID, false) - d, err = waitDeviceActive(d.ID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, d.ID) vcr := VolumeCreateRequest{ Size: 10, @@ -580,7 +599,7 @@ func TestAccDeviceAttachVolumeForceDelete(t *testing.T) { if err != nil { t.Fatal(err) } - defer c.Volumes.Delete(v.ID) + defer deleteVolume(t, c, v.ID) v, err = waitVolumeActive(v.ID, c) if err != nil { @@ -592,7 +611,7 @@ func TestAccDeviceAttachVolumeForceDelete(t *testing.T) { t.Fatal(err) } - v, _, err = c.Volumes.Get(v.ID, + _, _, err = c.Volumes.Get(v.ID, &GetOptions{Includes: []string{"attachments.device"}}) if err != nil { t.Fatal(err) @@ -603,7 +622,7 @@ func TestAccDeviceAttachVolumeForceDelete(t *testing.T) { t.Fatal(err) } - _, err = c.Devices.Delete(d.ID, true) + defer deleteDevice(t, c, d.ID, true) if err != nil { t.Fatal(err) } @@ -631,12 +650,9 @@ func TestAccDeviceAttachVolume(t *testing.T) { if err != nil { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) - d, err = waitDeviceActive(d.ID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, d.ID) vcr := VolumeCreateRequest{ Size: 10, @@ -649,7 +665,7 @@ func TestAccDeviceAttachVolume(t *testing.T) { if err != nil { t.Fatal(err) } - defer c.Volumes.Delete(v.ID) + defer deleteVolume(t, c, v.ID) v, err = waitVolumeActive(v.ID, c) if err != nil { @@ -687,11 +703,7 @@ func TestAccDeviceAttachVolume(t *testing.T) { t.Fatalf("wrong volume linked in device.volumes: %s, should be %s", d.Volumes[0].Href, v.ID) } - _, err = c.VolumeAttachments.Delete(a.ID) - if err != nil { - t.Fatal(err) - } - + defer deleteVolumeAttachments(t, c, a.ID) } func TestAccDeviceSpotInstance(t *testing.T) { @@ -722,12 +734,9 @@ func TestAccDeviceSpotInstance(t *testing.T) { if err != nil { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) - d, err = waitDeviceActive(d.ID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, d.ID) if !d.SpotInstance { t.Fatal("spot_instance is false, should be true") @@ -769,14 +778,11 @@ func TestAccDeviceCustomData(t *testing.T) { if err != nil { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) dID := d.ID - d, err = waitDeviceActive(dID, c) - if err != nil { - t.Fatal(err) - } + _ = waitDeviceActive(t, c, dID) device, _, err := c.Devices.Get(dID, nil) if err != nil { @@ -847,12 +853,9 @@ func TestAccListDeviceEvents(t *testing.T) { if err != nil { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) - d, err = waitDeviceActive(d.ID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, d.ID) events, _, err := c.Devices.ListEvents(d.ID, nil) if err != nil { @@ -871,9 +874,11 @@ func TestAccDeviceSSHKeys(t *testing.T) { defer teardown() hn := randString8() userKey := createKey(t, c, "") - defer c.SSHKeys.Delete(userKey.ID) + defer deleteSSHKey(t, c, userKey.ID) + projectKey := createKey(t, c, projectID) - defer c.SSHKeys.Delete(projectKey.ID) + defer deleteSSHKey(t, c, projectKey.ID) + cr := DeviceCreateRequest{ Hostname: hn, Facility: []string{testFacility()}, @@ -886,12 +891,11 @@ func TestAccDeviceSSHKeys(t *testing.T) { if err != nil { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) + dID := d.ID - d, err = waitDeviceActive(dID, c) - if err != nil { - t.Fatal(err) - } + _ = waitDeviceActive(t, c, dID) + d, _, err = c.Devices.Get(dID, &GetOptions{Includes: []string{"ssh_keys"}}) if err != nil { t.Fatal(err) @@ -921,11 +925,11 @@ func TestAccDeviceListedSSHKeys(t *testing.T) { defer teardown() hn := randString8() userKey := createKey(t, c, "") - defer c.SSHKeys.Delete(userKey.ID) + defer deleteSSHKey(t, c, userKey.ID) projectKey := createKey(t, c, projectID) - defer c.SSHKeys.Delete(projectKey.ID) + defer deleteSSHKey(t, c, projectKey.ID) projectKey2 := createKey(t, c, projectID) - defer c.SSHKeys.Delete(projectKey2.ID) + defer deleteSSHKey(t, c, projectKey2.ID) cr := DeviceCreateRequest{ Hostname: hn, Facility: []string{testFacility()}, @@ -939,12 +943,10 @@ func TestAccDeviceListedSSHKeys(t *testing.T) { if err != nil { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) dID := d.ID - d, err = waitDeviceActive(dID, c) - if err != nil { - t.Fatal(err) - } + _ = waitDeviceActive(t, c, dID) + d, _, err = c.Devices.Get(dID, &GetOptions{Includes: []string{"ssh_keys"}}) if err != nil { t.Fatal(err) @@ -999,14 +1001,11 @@ func TestAccDeviceCreateFacilities(t *testing.T) { if err != nil { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) dID := d.ID - d, err = waitDeviceActive(dID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, dID) placedInRequestedFacility := false for _, fac := range facilities { @@ -1046,14 +1045,11 @@ func TestAccDeviceIPAddresses(t *testing.T) { if err != nil { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) dID := d.ID - d, err = waitDeviceActive(dID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, dID) _, err = d.GetNetworkType() if err != nil { t.Fatal(err) diff --git a/email_test.go b/email_test.go index d2b8d901..40934b5d 100644 --- a/email_test.go +++ b/email_test.go @@ -7,7 +7,8 @@ import ( func TestAccCreateEmail(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) updatedAddress := "update@domain.com" - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() req := &EmailRequest{ Address: "test@domain.com", diff --git a/events_test.go b/events_test.go index abb9bcdc..ec295160 100644 --- a/events_test.go +++ b/events_test.go @@ -6,7 +6,8 @@ import ( func TestAccListEvents(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() events, _, err := c.Events.List(&ListOptions{Page: 1, PerPage: 9}) if err != nil { diff --git a/facilities_test.go b/facilities_test.go index d734ddf9..b53a1c28 100644 --- a/facilities_test.go +++ b/facilities_test.go @@ -8,7 +8,8 @@ import ( func TestAccFacilities(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() l, _, err := c.Facilities.List(&ListOptions{Includes: []string{"address"}}) if err != nil { diff --git a/fixtures/.placeholder b/fixtures/.placeholder new file mode 100644 index 00000000..7b097112 --- /dev/null +++ b/fixtures/.placeholder @@ -0,0 +1 @@ +fixtures placeholder diff --git a/go.mod b/go.mod index 8a05f1a4..1a982cbe 100644 --- a/go.mod +++ b/go.mod @@ -3,6 +3,7 @@ module github.com/packethost/packngo go 1.13 require ( + github.com/dnaeon/go-vcr v1.0.1 github.com/hashicorp/go-retryablehttp v0.6.6 github.com/stretchr/testify v1.5.1 golang.org/x/crypto v0.0.0-20200420201142-3c4aac89819a diff --git a/go.sum b/go.sum index b2d833de..5b7a82d4 100644 --- a/go.sum +++ b/go.sum @@ -1,6 +1,8 @@ github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/dnaeon/go-vcr v1.0.1 h1:r8L/HqC0Hje5AXMu1ooW8oyQyOFv4GxqpL0nRP7SLLY= +github.com/dnaeon/go-vcr v1.0.1/go.mod h1:aBB1+wY4s93YsC3HHjMBMrwTj2R9FHDzUr9KyGc8n1E= github.com/hashicorp/go-cleanhttp v0.5.1 h1:dH3aiDG9Jvb5r5+bYHsikaOUIpcM0xvgMXVoDkXMzJM= github.com/hashicorp/go-cleanhttp v0.5.1/go.mod h1:JpRdi6/HCYpAwUzNwuwqhbovhLtngrth3wmdIIUrZ80= github.com/hashicorp/go-hclog v0.9.2 h1:CG6TE5H9/JXsFWJCfoIVpKFIkFe6ysEuHirp4DxCsHI= diff --git a/ip_test.go b/ip_test.go index 1a1e1848..063cf96f 100644 --- a/ip_test.go +++ b/ip_test.go @@ -97,10 +97,8 @@ func TestAccPublicIPReservation(t *testing.T) { quantity, availableAddresses) } - _, err = c.ProjectIPs.Remove(res.ID) - if err != nil { - t.Fatal(err) - } + deleteProjectIP(t, c, res.ID) + _, _, err = c.ProjectIPs.Get(res.ID, nil) if err == nil { t.Fatalf("Reservation %s should be deleted at this point", res) diff --git a/metadata/metadata_test.go b/metadata/metadata_test.go index ea23e770..54d47277 100644 --- a/metadata/metadata_test.go +++ b/metadata/metadata_test.go @@ -23,9 +23,11 @@ func Test_Deserialization(t *testing.T) { mux.HandleFunc("/metadata", func(w http.ResponseWriter, req *http.Request) { b, err := ioutil.ReadFile("testdata/" + "device.json") if err != nil { - panic(err) + t.Fatal(err) + } + if _, err := w.Write(b); err != nil { + t.Fatal(err) } - w.Write(b) }) device, err := GetMetadataFromURL(baseURL) diff --git a/notifications_test.go b/notifications_test.go index 68eb8821..e3b12c30 100644 --- a/notifications_test.go +++ b/notifications_test.go @@ -6,7 +6,8 @@ import ( func TestAccListNotifications(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() notifications, _, err := c.Notifications.List(nil) if err != nil { diff --git a/operatingsystems_test.go b/operatingsystems_test.go index 5791c4ff..377b44b5 100644 --- a/operatingsystems_test.go +++ b/operatingsystems_test.go @@ -7,7 +7,8 @@ import ( func TestAccOS(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() l, _, err := c.OperatingSystems.List() if len(l) == 0 { diff --git a/organizations_test.go b/organizations_test.go index 30b63238..4a2e14d0 100644 --- a/organizations_test.go +++ b/organizations_test.go @@ -8,7 +8,8 @@ import ( func TestAccOrgList(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() defer organizationTeardown(c) rs := testProjectPrefix + randString8() @@ -49,7 +50,8 @@ func TestAccOrgList(t *testing.T) { func TestAccOrgBasic(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() defer organizationTeardown(c) rs := testProjectPrefix + randString8() @@ -107,7 +109,8 @@ func TestAccOrgListPaymentMethods(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) // setup - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() defer organizationTeardown(c) rs := testProjectPrefix + randString8() diff --git a/packngo.go b/packngo.go index 919dfb66..10fe1c19 100644 --- a/packngo.go +++ b/packngo.go @@ -211,6 +211,18 @@ type Client struct { Volumes VolumeService } +// requestDoer provides methods for making HTTP requests and receiving the +// response, errors, and a structured result +// +// This interface is used in *ServiceOp as a mockable alternative to a full +// Client object. +type requestDoer interface { + NewRequest(method, path string, body interface{}) (*retryablehttp.Request, error) + Do(req *retryablehttp.Request, v interface{}) (*Response, error) + DoRequest(method, path string, body, v interface{}) (*Response, error) + DoRequestWithHeader(method string, headers map[string]string, path string, body, v interface{}) (*Response, error) +} + // NewRequest inits a new http request with the proper headers func (c *Client) NewRequest(method, path string, body interface{}) (*retryablehttp.Request, error) { // relative path to append to the endpoint url, no leading slash please @@ -271,7 +283,10 @@ func (c *Client) Do(req *retryablehttp.Request, v interface{}) (*Response, error if v != nil { // if v implements the io.Writer interface, return the raw response if w, ok := v.(io.Writer); ok { - io.Copy(w, resp.Body) + _, err = io.Copy(w, resp.Body) + if err != nil { + return &response, err + } } else { err = json.NewDecoder(resp.Body).Decode(v) if err != nil { @@ -451,8 +466,15 @@ func checkResponse(r *http.Response) error { errorResponse := &ErrorResponse{Response: r} data, err := ioutil.ReadAll(r.Body) // if the response has a body, populate the message in errorResponse - if err == nil && len(data) > 0 { - json.Unmarshal(data, errorResponse) + if err != nil { + return err + } + + if len(data) > 0 { + err = json.Unmarshal(data, errorResponse) + if err != nil { + return err + } } return errorResponse diff --git a/packngo_test.go b/packngo_test.go index 007d070e..db4858e2 100644 --- a/packngo_test.go +++ b/packngo_test.go @@ -4,10 +4,15 @@ import ( "fmt" "math/rand" "os" + "path" "regexp" "strings" "testing" "time" + + "github.com/dnaeon/go-vcr/cassette" + "github.com/dnaeon/go-vcr/recorder" + "github.com/hashicorp/go-retryablehttp" ) const ( @@ -15,6 +20,13 @@ const ( packngoAccTestVar = "PACKNGO_TEST_ACTUAL_API" testProjectPrefix = "PACKNGO_TEST_DELME_2d768716_" testFacilityVar = "PACKNGO_TEST_FACILITY" + testRecorderEnv = "PACKNGO_TEST_RECORDER" + + testRecorderRecord = "record" + testRecorderPlay = "play" + testRecorderDisabled = "disabled" + + recorderDefaultMode = recorder.ModeDisabled ) func testFacility() string { @@ -26,6 +38,12 @@ func testFacility() string { } func randString8() string { + // test recorder needs replayable names, not randoms + mode, _ := testRecordMode() + if mode != recorder.ModeDisabled { + return "testrand" + } + n := 8 rand.Seed(time.Now().UnixNano()) letterRunes := []rune("acdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ") @@ -36,8 +54,40 @@ func randString8() string { return string(b) } +// MockClient makes it simpler to test the Client +type MockClient struct { + fnNewRequest func(method, path string, body interface{}) (*retryablehttp.Request, error) + fnDo func(req *retryablehttp.Request, v interface{}) (*Response, error) + fnDoRequest func(method, path string, body, v interface{}) (*Response, error) + fnDoRequestWithHeader func(method string, headers map[string]string, path string, body, v interface{}) (*Response, error) +} + +var _ requestDoer = &MockClient{} + +// NewRequest uses the mock NewRequest function +func (mc *MockClient) NewRequest(method, path string, body interface{}) (*retryablehttp.Request, error) { + return mc.fnNewRequest(method, path, body) +} + +// Do uses the mock Do function +func (mc *MockClient) Do(req *retryablehttp.Request, v interface{}) (*Response, error) { + return mc.fnDo(req, v) +} + +// DoRequest uses the mock DoRequest function +func (mc *MockClient) DoRequest(method, path string, body, v interface{}) (*Response, error) { + return mc.fnDoRequest(method, path, body, v) +} + +// DoRequestWithHeader uses the mock DoRequestWithHeader function +func (mc *MockClient) DoRequestWithHeader(method string, headers map[string]string, path string, body, v interface{}) (*Response, error) { + return mc.fnDoRequestWithHeader(method, headers, path, body, v) +} + +// setupWithProject returns a client, project id, and teardown function +// configured for a new project with a test recorder for the named test func setupWithProject(t *testing.T) (*Client, string, func()) { - c := setup(t) + c, stopRecord := setup(t) p, _, err := c.Projects.Create(&ProjectCreateRequest{Name: testProjectPrefix + randString8()}) if err != nil { t.Fatal(err) @@ -48,6 +98,7 @@ func setupWithProject(t *testing.T) (*Client, string, func()) { if err != nil { panic(fmt.Errorf("while deleting %s: %s", p, err)) } + stopRecord() } } @@ -58,20 +109,68 @@ func skipUnlessAcceptanceTestsAllowed(t *testing.T) { } } -func setup(t *testing.T) *Client { +// testRecorder creates the named recorder +func testRecorder(t *testing.T, name string, mode recorder.Mode) (*recorder.Recorder, func()) { + r, err := recorder.NewAsMode(path.Join("fixtures", name), mode, nil) + if err != nil { + t.Fatal(err) + } + + r.AddFilter(func(i *cassette.Interaction) error { + delete(i.Request.Headers, "X-Auth-Token") + return nil + }) + + return r, func() { + if err := r.Stop(); err != nil { + t.Fatal(err) + } + } +} + +func testRecordMode() (recorder.Mode, error) { + modeRaw := os.Getenv(testRecorderEnv) + mode := recorderDefaultMode + + switch strings.ToLower(modeRaw) { + case testRecorderRecord: + mode = recorder.ModeRecording + case testRecorderPlay: + mode = recorder.ModeReplaying + case "": + // no-op + case testRecorderDisabled: + // no-op + default: + return mode, fmt.Errorf("invalid %s mode: %s", testRecorderEnv, modeRaw) + } + return mode, nil +} + +func setup(t *testing.T) (*Client, func()) { + name := t.Name() apiToken := os.Getenv(packetTokenEnvVar) if apiToken == "" { t.Fatalf("If you want to run packngo test, you must export %s.", packetTokenEnvVar) } + + mode, err := testRecordMode() + if err != nil { + t.Fatal(err) + } apiURL := os.Getenv(packetURLEnvVar) if apiURL == "" { apiURL = baseURL } - c, err := NewClientWithBaseURL("packngo test", apiToken, nil, apiURL) + r, stopRecord := testRecorder(t, name, mode) + httpClient := retryablehttp.NewClient() + httpClient.HTTPClient.Transport = r + c, err := NewClientWithBaseURL("packngo test", apiToken, httpClient, apiURL) if err != nil { t.Fatal(err) } - return c + + return c, stopRecord } func projectTeardown(c *Client) { diff --git a/plans_test.go b/plans_test.go index cc425719..18c4a3e1 100644 --- a/plans_test.go +++ b/plans_test.go @@ -8,7 +8,8 @@ import ( func TestAccPlans(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() l, _, err := c.Plans.List(&ListOptions{Includes: []string{"available_in"}}) avail := map[string][]string{} diff --git a/ports.go b/ports.go index d4c8a948..63074af8 100644 --- a/ports.go +++ b/ports.go @@ -276,11 +276,11 @@ func (i *DevicePortServiceOp) Convert1BondDevice(d *Device, targetType string) e } } if targetType == "hybrid" { - bond0, _, err = i.client.DevicePorts.Bond(bond0, false) + _, _, err = i.client.DevicePorts.Bond(bond0, false) if err != nil { return err } - bond0, _, err = i.client.DevicePorts.PortToLayerThree(d.ID, "bond0") + _, _, err = i.client.DevicePorts.PortToLayerThree(d.ID, "bond0") if err != nil { return err } @@ -310,7 +310,7 @@ func (i *DevicePortServiceOp) Convert1BondDevice(d *Device, targetType string) e return err } } - bond0, _, err = i.client.DevicePorts.PortToLayerTwo(d.ID, "bond0") + _, _, err = i.client.DevicePorts.PortToLayerTwo(d.ID, "bond0") if err != nil { return err } diff --git a/ports_test.go b/ports_test.go index be826d77..b059c1cc 100644 --- a/ports_test.go +++ b/ports_test.go @@ -37,7 +37,7 @@ func TestAccPort1E(t *testing.T) { if err != nil { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) dID := d.ID // If you need to test this, run a 1e device in your project in a faciltiy @@ -46,20 +46,18 @@ func TestAccPort1E(t *testing.T) { /* - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() - dID := "414f52d3-022a-420d-a521-915fdcc66801" - projectID := "52000fb2-ee46-4673-93a8-de2c2bdba33b" - fac := "atl1" - d := &Device{} - err := fmt.Errorf("hi") + dID := "414f52d3-022a-420d-a521-915fdcc66801" + projectID := "52000fb2-ee46-4673-93a8-de2c2bdba33b" + fac := "atl1" + d := &Device{} + err := fmt.Errorf("hi") */ - d, err = waitDeviceActive(dID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, dID) nType, err := c.DevicePorts.DeviceNetworkType(d.ID) if err != nil { @@ -89,7 +87,7 @@ func TestAccPort1E(t *testing.T) { if err != nil { t.Fatal(err) } - defer c.ProjectVirtualNetworks.Delete(vlan.ID) + defer deleteProjectVirtualNetwork(t, c, vlan.ID) par := PortAssignRequest{ PortID: eth1.ID, VirtualNetworkID: vlan.ID} @@ -165,7 +163,7 @@ func testL2HybridL3Convert(t *testing.T, plan string) { if err != nil { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) dID := d.ID // If you need to test this, run a ${plan} device in your project in a @@ -174,23 +172,21 @@ func testL2HybridL3Convert(t *testing.T, plan string) { // Fill the values from youri testing device, project and facility. /* - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() - projectID := "52000fb2-ee46-4673-93a8-de2c2bdba33b" - dID := "b7515d6b-6a86-4830-ab50-9bca3aa51e1c" - fac := "dfw2" + projectID := "52000fb2-ee46-4673-93a8-de2c2bdba33b" + dID := "b7515d6b-6a86-4830-ab50-9bca3aa51e1c" + fac := "dfw2" - //dID := "131dfaf1-e38a-4963-86d6-dbc2531f85d7" - //fac := "ewr1" + //dID := "131dfaf1-e38a-4963-86d6-dbc2531f85d7" + //fac := "ewr1" - d := &Device{} - err := fmt.Errorf("hi") + d := &Device{} + err := fmt.Errorf("hi") */ - d, err = waitDeviceActive(dID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, dID) nType, err := c.DevicePorts.DeviceNetworkType(d.ID) if err != nil { @@ -232,7 +228,7 @@ func testL2HybridL3Convert(t *testing.T, plan string) { if err != nil { t.Fatal(err) } - defer c.ProjectVirtualNetworks.Delete(vlan.ID) + defer deleteProjectVirtualNetwork(t, c, vlan.ID) par := PortAssignRequest{ PortID: eth1.ID, @@ -325,7 +321,7 @@ func testL2L3Convert(t *testing.T, plan string) { if err != nil { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) dID := d.ID /* @@ -334,7 +330,8 @@ func testL2L3Convert(t *testing.T, plan string) { // and then comment code from MARK_2 to here and uncomment following. // Fill the values from youri testing device, project and facility. - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() // dID := "131dfaf1-e38a-4963-86d6-dbc2531f85d7" dID := "b7515d6b-6a86-4830-ab50-9bca3aa51e1c" @@ -345,10 +342,7 @@ func testL2L3Convert(t *testing.T, plan string) { err := fmt.Errorf("hi") */ - d, err = waitDeviceActive(dID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, dID) nType, err := c.DevicePorts.DeviceNetworkType(d.ID) if err != nil { @@ -391,7 +385,7 @@ func testL2L3Convert(t *testing.T, plan string) { if err != nil { t.Fatal(err) } - defer c.ProjectVirtualNetworks.Delete(vlan.ID) + defer deleteProjectVirtualNetwork(t, c, vlan.ID) par := PortAssignRequest{ PortID: bond0.ID, @@ -452,7 +446,8 @@ func deviceToNetworkType(t *testing.T, c *Client, deviceID, targetNetworkType st func TestXXX(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) t.Parallel() - c := setup(t) + c, stopRecord := setup(t) +defer stopRecord() // deviceID := "131dfaf1-e38a-4963-86d6-dbc2531f85d7" deviceID := "b7515d6b-6a86-4830-ab50-9bca3aa51e1c" d, _, err := c.Devices.Get(deviceID, nil) @@ -490,13 +485,10 @@ func TestAccPortNetworkStateTransitions(t *testing.T) { if err != nil { t.Fatal(err) } - defer deleteDevice(t, c, d.ID) + defer deleteDevice(t, c, d.ID, false) deviceID := d.ID - d, err = waitDeviceActive(deviceID, c) - if err != nil { - t.Fatal(err) - } + d = waitDeviceActive(t, c, deviceID) networkType, err := d.GetNetworkType() if err != nil { @@ -546,11 +538,8 @@ func TestAccPortNativeVlan(t *testing.T) { } deviceID := d.ID - d, err = waitDeviceActive(deviceID, c) - if err != nil { - t.Fatal(err) - } - defer deleteDevice(t, c, d.ID) + d = waitDeviceActive(t, c, deviceID) + defer deleteDevice(t, c, d.ID, false) deviceToNetworkType(t, c, deviceID, "hybrid") @@ -563,12 +552,12 @@ func TestAccPortNativeVlan(t *testing.T) { if err != nil { t.Fatal(err) } - defer c.ProjectVirtualNetworks.Delete(vlan1.ID) + defer deleteProjectVirtualNetwork(t, c, vlan1.ID) vlan2, _, err := c.ProjectVirtualNetworks.Create(&vncr) if err != nil { t.Fatal(err) } - defer c.ProjectVirtualNetworks.Delete(vlan2.ID) + defer deleteProjectVirtualNetwork(t, c, vlan2.ID) eth1, err := c.DevicePorts.GetPortByName(d.ID, "eth1") if err != nil { @@ -580,18 +569,18 @@ func TestAccPortNativeVlan(t *testing.T) { par1 := PortAssignRequest{ PortID: eth1.ID, VirtualNetworkID: vlan1.ID} - p, _, err := c.DevicePorts.Assign(&par1) + _, _, err = c.DevicePorts.Assign(&par1) if err != nil { t.Fatal(err) } par2 := PortAssignRequest{ PortID: eth1.ID, VirtualNetworkID: vlan2.ID} - p, _, err = c.DevicePorts.Assign(&par2) + _, _, err = c.DevicePorts.Assign(&par2) if err != nil { t.Fatal(err) } - p, _, err = c.DevicePorts.AssignNative(&par1) + _, _, err = c.DevicePorts.AssignNative(&par1) if err != nil { t.Fatal(err) } @@ -606,15 +595,15 @@ func TestAccPortNativeVlan(t *testing.T) { } else { t.Fatal("No native virtual network at the test device") } - p, _, err = c.DevicePorts.UnassignNative(eth1.ID) + _, _, err = c.DevicePorts.UnassignNative(eth1.ID) if err != nil { t.Fatal(err) } - p, _, err = c.DevicePorts.Unassign(&par2) + _, _, err = c.DevicePorts.Unassign(&par2) if err != nil { t.Fatal(err) } - p, _, err = c.DevicePorts.Unassign(&par1) + p, _, err := c.DevicePorts.Unassign(&par1) if err != nil { t.Fatal(err) } diff --git a/projects_test.go b/projects_test.go index f4901dbd..2dd1f566 100644 --- a/projects_test.go +++ b/projects_test.go @@ -7,7 +7,8 @@ import ( func TestAccProjectBasic(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() defer projectTeardown(c) rs := testProjectPrefix + randString8() @@ -49,7 +50,8 @@ func TestAccProjectBasic(t *testing.T) { func TestAccProjectExtra(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() defer projectTeardown(c) u, _, err := c.Users.Current() if err != nil { @@ -107,7 +109,8 @@ func TestAccProjectExtra(t *testing.T) { func TestAccCreateOrgProject(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() defer projectTeardown(c) u, _, err := c.Users.Current() @@ -131,7 +134,8 @@ func TestAccCreateOrgProject(t *testing.T) { func TestAccCreateNonDefaultOrgProject(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() defer organizationTeardown(c) defer projectTeardown(c) @@ -177,7 +181,8 @@ func TestAccCreateNonDefaultOrgProject(t *testing.T) { func TestAccListProjects(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() defer projectTeardown(c) @@ -220,7 +225,8 @@ func TestAccListProjects(t *testing.T) { func TestAccProjectListPagination(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() defer projectTeardown(c) for i := 0; i < 3; i++ { pcr := ProjectCreateRequest{ diff --git a/spotmarket_test.go b/spotmarket_test.go index b403dcfc..9c8a947b 100644 --- a/spotmarket_test.go +++ b/spotmarket_test.go @@ -6,7 +6,8 @@ func TestAccSpotMarketBasic(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) t.Parallel() - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() prices, _, err := c.SpotMarket.Prices() if err != nil { t.Fatal(err) diff --git a/spotmarketrequest_test.go b/spotmarketrequest_test.go index 089aec1f..58e1a35b 100644 --- a/spotmarketrequest_test.go +++ b/spotmarketrequest_test.go @@ -44,7 +44,7 @@ func TestAccSpotMarketRequestBasic(t *testing.T) { if err != nil { t.Fatal(err) } - defer c.SpotMarketRequests.Delete(smr.ID, true) + defer deleteSpotMarketRequest(t, c, smr.ID, true) if smr.Project.ID != projectID { t.Fatal("Strange project ID in SpotMarketReuqest:", smr.Project.ID) @@ -111,43 +111,38 @@ func TestAccSpotMarketRequestPriceAware(t *testing.T) { out: for { - select { - case <-time.Tick(5 * time.Second): - smr, _, err = c.SpotMarketRequests.Get( - smr.ID, - &GetOptions{Includes: []string{"devices"}}, - ) - if err != nil { - t.Fatal(err) - } - activeDevs := 0 - for _, d := range smr.Devices { - if d.State == "active" { - activeDevs++ - } - } - if activeDevs == nDevices { - break out + <-time.Tick(5 * time.Second) + smr, _, err = c.SpotMarketRequests.Get( + smr.ID, + &GetOptions{Includes: []string{"devices"}}, + ) + if err != nil { + t.Fatal(err) + } + activeDevs := 0 + for _, d := range smr.Devices { + if d.State == "active" { + activeDevs++ } } + if activeDevs == nDevices { + break out + } } log.Println("all devices active") - c.SpotMarketRequests.Delete(smr.ID, true) + defer deleteSpotMarketRequest(t, c, smr.ID, true) out2: // wait for devices to disappear .. takes ~5 minutes for { - select { - case <-time.Tick(5 * time.Second): - ds, _, err := c.Devices.List(projectID, nil) - if err != nil { - t.Fatal(err) - } - if len(ds) > 0 { - log.Println(len(ds), "devices still exist") - } else { - break out2 - } - + <-time.Tick(5 * time.Second) + ds, _, err := c.Devices.List(projectID, nil) + if err != nil { + t.Fatal(err) + } + if len(ds) > 0 { + log.Println(len(ds), "devices still exist") + } else { + break out2 } } } diff --git a/sshkeys_test.go b/sshkeys_test.go index 02c57261..83641d1e 100644 --- a/sshkeys_test.go +++ b/sshkeys_test.go @@ -43,7 +43,7 @@ func TestAccSSHKeyList(t *testing.T) { c, _, teardown := setupWithProject(t) defer teardown() key := createKey(t, c, "") - defer c.SSHKeys.Delete(key.ID) + defer deleteSSHKey(t, c, key.ID) keys, _, err := c.SSHKeys.List() if err != nil { @@ -68,7 +68,7 @@ func TestAccSSHKeyProjectList(t *testing.T) { defer teardown() key := createKey(t, c, projectID) - defer c.SSHKeys.Delete(key.ID) + defer deleteSSHKey(t, c, key.ID) keys, _, err := c.SSHKeys.ProjectList(projectID) if err != nil { @@ -96,7 +96,7 @@ func TestAccSSHKeyGet(t *testing.T) { c, projectID, teardown := setupWithProject(t) defer teardown() user := createKey(t, c, "") - defer c.SSHKeys.Delete(user.ID) + defer deleteSSHKey(t, c, user.ID) proj := createKey(t, c, projectID) for _, k := range []*SSHKey{user, proj} { diff --git a/user_test.go b/user_test.go index cef64fec..402c341d 100644 --- a/user_test.go +++ b/user_test.go @@ -7,7 +7,8 @@ import ( func TestAccUserCurrent(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() u, _, err := c.Users.Current() if err != nil { @@ -22,7 +23,8 @@ func TestAccUserCurrent(t *testing.T) { func TestAccUsersList(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() us, _, err := c.Users.List(nil) if err != nil { diff --git a/utils.go b/utils.go index db67f1f4..42124ba4 100644 --- a/utils.go +++ b/utils.go @@ -9,14 +9,22 @@ import ( var ( timestampType = reflect.TypeOf(Timestamp{}) - Facilities = []string{ + + // Facilities DEPRECATED Use Facilities.List + Facilities = []string{ "yyz1", "nrt1", "atl1", "mrs1", "hkg1", "ams1", "ewr1", "sin1", "dfw1", "lax1", "syd1", "sjc1", "ord1", "iad1", "fra1", "sea1", "dfw2"} + + // FacilityFeatures DEPRECATED Use Facilities.List FacilityFeatures = []string{ "baremetal", "layer_2", "backend_transfer", "storage", "global_ipv4"} + + // UtilizationLevels DEPRECATED UtilizationLevels = []string{"unavailable", "critical", "limited", "normal"} - DevicePlans = []string{"c2.medium.x86", "g2.large.x86", + + // DevicePlans DEPRECATED Use Plans.List + DevicePlans = []string{"c2.medium.x86", "g2.large.x86", "m2.xlarge.x86", "x2.xlarge.x86", "baremetal_2a", "baremetal_2a2", "baremetal_1", "baremetal_3", "baremetal_2", "baremetal_s", "baremetal_0", "baremetal_1e", @@ -24,18 +32,23 @@ var ( ) // Stringify creates a string representation of the provided message +// DEPRECATED This is used internally and should not be exported by packngo func Stringify(message interface{}) string { var buf bytes.Buffer v := reflect.ValueOf(message) - stringifyValue(&buf, v) + // TODO(displague) errors here are not reported + _ = stringifyValue(&buf, v) return buf.String() } // StreamToString converts a reader to a string -func StreamToString(stream io.Reader) string { +// DEPRECATED This is unused and should not be exported by packngo +func StreamToString(stream io.Reader) (string, error) { buf := new(bytes.Buffer) - buf.ReadFrom(stream) - return buf.String() + if _, err := buf.ReadFrom(stream); err != nil { + return "", err + } + return buf.String(), nil } // contains tells whether a contains x. @@ -49,41 +62,55 @@ func contains(a []string, x string) bool { } // stringifyValue was graciously cargoculted from the goprotubuf library -func stringifyValue(w io.Writer, val reflect.Value) { +func stringifyValue(w io.Writer, val reflect.Value) error { if val.Kind() == reflect.Ptr && val.IsNil() { - w.Write([]byte("")) - return + _, err := w.Write([]byte("")) + return err } v := reflect.Indirect(val) switch v.Kind() { case reflect.String: - fmt.Fprintf(w, `"%s"`, v) + if _, err := fmt.Fprintf(w, `"%s"`, v); err != nil { + return err + } case reflect.Slice: - w.Write([]byte{'['}) + if _, err := w.Write([]byte{'['}); err != nil { + return err + } for i := 0; i < v.Len(); i++ { if i > 0 { - w.Write([]byte{' '}) + if _, err := w.Write([]byte{' '}); err != nil { + return err + } } - stringifyValue(w, v.Index(i)) + if err := stringifyValue(w, v.Index(i)); err != nil { + return err + } } - w.Write([]byte{']'}) - return + if _, err := w.Write([]byte{']'}); err != nil { + return err + } + return nil case reflect.Struct: if v.Type().Name() != "" { - w.Write([]byte(v.Type().String())) + if _, err := w.Write([]byte(v.Type().String())); err != nil { + return err + } } // special handling of Timestamp values if v.Type() == timestampType { - fmt.Fprintf(w, "{%s}", v.Interface()) - return + _, err := fmt.Fprintf(w, "{%s}", v.Interface()) + return err } - w.Write([]byte{'{'}) + if _, err := w.Write([]byte{'{'}); err != nil { + return err + } var sep bool for i := 0; i < v.NumField(); i++ { @@ -96,20 +123,34 @@ func stringifyValue(w io.Writer, val reflect.Value) { } if sep { - w.Write([]byte(", ")) + if _, err := w.Write([]byte(", ")); err != nil { + return err + } } else { sep = true } - w.Write([]byte(v.Type().Field(i).Name)) - w.Write([]byte{':'}) - stringifyValue(w, fv) + if _, err := w.Write([]byte(v.Type().Field(i).Name)); err != nil { + return err + } + if _, err := w.Write([]byte{':'}); err != nil { + return err + } + + if err := stringifyValue(w, fv); err != nil { + return err + } } - w.Write([]byte{'}'}) + if _, err := w.Write([]byte{'}'}); err != nil { + return err + } default: if v.CanInterface() { - fmt.Fprint(w, v.Interface()) + if _, err := fmt.Fprint(w, v.Interface()); err != nil { + return err + } } } + return nil } diff --git a/virtualnetworks_test.go b/virtualnetworks_test.go index 3bb6e94e..c2f0bac5 100644 --- a/virtualnetworks_test.go +++ b/virtualnetworks_test.go @@ -34,6 +34,9 @@ func TestAccVirtualNetworks(t *testing.T) { vlan, _, err = c.ProjectVirtualNetworks.Get(vlan.ID, &GetOptions{Includes: []string{"assigned_to"}}) + if err != nil { + t.Fatal(err) + } if vlan.Project.ID != projectID { t.Fatalf("VLAN's project ID should be %s, was %s", projectID, diff --git a/volumes_test.go b/volumes_test.go index cf826606..059d39e2 100644 --- a/volumes_test.go +++ b/volumes_test.go @@ -6,6 +6,10 @@ import ( "time" ) +const ( + minVolumeSize = 100 +) + func waitVolumeActive(id string, c *Client) (*Volume, error) { // 15 minutes = 180 * 5sec-retry for i := 0; i < 180; i++ { @@ -33,7 +37,7 @@ func TestAccVolumeBasic(t *testing.T) { } vcr := VolumeCreateRequest{ - Size: 10, + Size: minVolumeSize, BillingCycle: "hourly", PlanID: "storage_1", FacilityID: testFacility(), @@ -51,7 +55,7 @@ func TestAccVolumeBasic(t *testing.T) { if err != nil { t.Fatal(err) } - defer c.Volumes.Delete(v.ID) + defer deleteVolume(t, c, v.ID) v, _, err = c.Volumes.Get(v.ID, &GetOptions{Includes: []string{"snapshot_policies", "facility"}}) @@ -93,7 +97,7 @@ func TestAccVolumeUpdate(t *testing.T) { } vcr := VolumeCreateRequest{ - Size: 10, + Size: minVolumeSize, BillingCycle: "hourly", PlanID: "storage_1", FacilityID: testFacility(), @@ -109,7 +113,7 @@ func TestAccVolumeUpdate(t *testing.T) { if err != nil { t.Fatal(err) } - defer c.Volumes.Delete(v.ID) + defer deleteVolume(t, c, v.ID) vDesc := "new Desc" @@ -179,7 +183,7 @@ func TestAccVolumeLargeList(t *testing.T) { } vcr := VolumeCreateRequest{ - Size: 10, + Size: minVolumeSize, BillingCycle: "hourly", PlanID: "storage_1", FacilityID: testFacility(), @@ -194,7 +198,7 @@ func TestAccVolumeLargeList(t *testing.T) { if err != nil { t.Fatal(err) } - defer c.Volumes.Delete(v.ID) + defer deleteVolume(t, c, v.ID) createdVolumes[i] = *v } diff --git a/vpn_test.go b/vpn_test.go index 57104a28..2fd5152d 100644 --- a/vpn_test.go +++ b/vpn_test.go @@ -8,7 +8,8 @@ func TestAccVPN(t *testing.T) { skipUnlessAcceptanceTestsAllowed(t) - c := setup(t) + c, stopRecord := setup(t) + defer stopRecord() u, _, err := c.Users.Current() if err != nil {