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

Acceptance test for create, get and delete aggregate #745

Merged
merged 3 commits into from
Feb 2, 2018

Conversation

dstdfx
Copy link
Contributor

@dstdfx dstdfx commented Jan 30, 2018

@coveralls
Copy link

coveralls commented Jan 30, 2018

Coverage Status

Coverage remained the same at 73.304% when pulling c1b5375 on dstdfx:aggregates-acceptance-test into bb5adf2 on gophercloud:master.

@theopenlab-ci
Copy link

theopenlab-ci bot commented Jan 30, 2018

Build succeeded.

@dstdfx
Copy link
Contributor Author

dstdfx commented Jan 30, 2018

@jtopjian This is ready for review.

@dstdfx dstdfx mentioned this pull request Jan 30, 2018
@jtopjian
Copy link
Contributor

@dstdfx Thanks for adding the tests for this. This works, but since you're going to implement the full suite of aggregate support, it would be beneficial in the long-term to make the acceptance tests work like the others. In particular, there should be helper functions for both create and delete.

An easy example to base this off of is "flavors": https://github.com/gophercloud/gophercloud/blob/master/acceptance/openstack/compute/v2/flavors_test.go#L78-L91

Note how there's a CreateFlavor and DeleteFlavor helper function. These are defined here:

This way, along with the user-facing support for aggregates, internally Gophercloud also has helper functions which can be used (and re-used) for various acceptance tests. For example, in theory, one could create an acceptance test to create an aggregate and launch a server in that aggregate easily by using the helper functions in compute.go.

Let me know if you have any questions or need help with this.

@dstdfx dstdfx changed the title Acceptance test for create and delete aggregate Acceptance test for create, get details and delete aggregate Jan 31, 2018
@dstdfx dstdfx changed the title Acceptance test for create, get details and delete aggregate Acceptance test for create, get and delete aggregate Jan 31, 2018
@theopenlab-ci
Copy link

theopenlab-ci bot commented Jan 31, 2018

Build failed.

@dstdfx
Copy link
Contributor Author

dstdfx commented Jan 31, 2018

recheck

@theopenlab-ci
Copy link

theopenlab-ci bot commented Feb 1, 2018

Build failed.

@dstdfx
Copy link
Contributor Author

dstdfx commented Feb 1, 2018

recheck

@theopenlab-ci
Copy link

theopenlab-ci bot commented Feb 1, 2018

Build failed.

@jtopjian
Copy link
Contributor

jtopjian commented Feb 2, 2018

recheck

@theopenlab-ci
Copy link

theopenlab-ci bot commented Feb 2, 2018

Build failed.

Copy link
Contributor

@jtopjian jtopjian left a comment

Choose a reason for hiding this comment

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

@dstdfx The OpenLab tests aren't showing a test results page, but the failure is in the raw output:

2018-02-02 20:46:06.256799 | ubuntu-xenial | acceptance/openstack/compute/v2/aggregates_test.go:44:35: undefined: aggregate
2018-02-02 20:46:06.257086 | ubuntu-xenial | FAIL	github.com/gophercloud/gophercloud/acceptance/openstack/compute/v2 [build failed]

if err != nil {
t.Fatalf("Unable to create an aggregate: %v", err)
}
defer DeleteAggregate(t, client, aggregate)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be

defer DeleteAggregate(t, client, createdAggregate)

@theopenlab-ci
Copy link

theopenlab-ci bot commented Feb 2, 2018

Build succeeded.

@jtopjian
Copy link
Contributor

jtopjian commented Feb 2, 2018

LGTM! Thanks, @dstdfx :)

@jtopjian jtopjian merged commit 1861349 into gophercloud:master Feb 2, 2018
@dstdfx dstdfx deleted the aggregates-acceptance-test branch February 2, 2018 21:43
cardoe pushed a commit to cardoe/gophercloud that referenced this pull request Aug 27, 2020
* vendor: github.com/gophercloud/gophercloud@latest

- go get github.com/gophercloud/gophercloud@latest
- go mod tidy && go mod vendor
- update resources that now use pointer references

* vendor: github.com/gophercloud/utils@latest

go get github.com/gophercloud/utils@latest
go mod tidy && go mod vendor
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants