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

Fixes #17552 - set default api version to v2 #34

Merged
merged 1 commit into from Dec 6, 2016

Conversation

swapab
Copy link
Member

@swapab swapab commented Dec 1, 2016

Apipie doc falsely states url(s) for bootdisk API calls.
Example: /api/generic should have been /bootdisk/api/v2/generic.
I wasn't sure how to test it. Should the API url be tested from foreman?

@domcleal
Copy link
Contributor

domcleal commented Dec 2, 2016

The URL is correct if the version header is passed - please instead fix the default API version to match Foreman, instead of changing the documentation URL. The question (which I didn't understand) should not be in the commit message.

@swapab swapab changed the title Fixes #17552 - apidoc correction for v2 Fixes #17552 - set default api version to v2 Dec 3, 2016
@swapab
Copy link
Member Author

swapab commented Dec 3, 2016

@domcleal Thanks for the review.
Updated the patch to set default API version to v2 using ApiConstraints from Foreman.

The question I asked was about testing the API URL (integration testing most likely). How is it usually done via foreman or within the plugin itself or not required at all since functional tests are in place ?

Didn't found any tests for api_constraints.rb as well. Should I add them in a different foreman patch ?

@domcleal
Copy link
Contributor

domcleal commented Dec 5, 2016

You can add integration tests if you wish, or use Rails' routing test helpers. If you're doing that, I'll set this to WoC.

@swapab
Copy link
Member Author

swapab commented Dec 5, 2016

Okay I'll test using Rails routing helpers.

Alongside while testing entire test suite of foreman_bootdisk I am getting a bunch of errors. Thought it might be familiar to you

centos7-devel :: ~/foreman ‹develop*› » rake test:foreman_bootdisk

.EEEE..EEE.......................

Finished in 6.314247s, 5.2263 runs/s, 9.5023 assertions/s.

  1) Error:
ForemanBootdisk::Api::V2::DisksControllerTest::#host#test_0001_should generate host image:
ActiveRecord::RecordInvalid: Validation failed: Download policy must be one of the following: immediate, on_demand, background, inherit
    test_after_commit (1.1.0) lib/test_after_commit/database_statements.rb:11:in `block in transaction'
    test_after_commit (1.1.0) lib/test_after_commit/database_statements.rb:5:in `transaction'
    /home/vagrant/katello/app/lib/katello/lazy_accessor.rb:87:in `save!'
    /home/vagrant/foreman_bootdisk/test/test_plugin_helper.rb:29:in `setup_subnet'

Full trace : https://gist.github.com/swapab/abe825973a6db64d7408ab0fb30050f8


Any idea how to fix those ?

@domcleal
Copy link
Contributor

domcleal commented Dec 5, 2016

No, that does not look like foreman_bootdisk, disable your other plugins.

@swapab
Copy link
Member Author

swapab commented Dec 6, 2016

@domcleal Test cases added : 403315b#diff-3be020c4f93f48ef73b2df6232dfddd1

@domcleal domcleal merged commit 2dfb64b into theforeman:master Dec 6, 2016
@domcleal
Copy link
Contributor

domcleal commented Dec 6, 2016

Merged, thanks @swapab.

@swapab swapab deleted the 17552-correct-apidoc-v2 branch December 14, 2016 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants