fixes #8612 - Makes output consistent for deleting act key, BZ1171092 #4854
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cfouant
force-pushed
the
delete-actkey
branch
from
1b2913b
to
bc25b38
Compare
| @controller.stubs(:sync_task).returns(true) | ||
| delete :destroy, :organization_id => @organization.id, :id => @activation_key.id | ||
| assert_sync_task(::Actions::Katello::ActivationKey::Destroy) do |act_key| | ||
| act_key.id.must_equal @activation_key.id |
There was a problem hiding this comment.
I think this should use asserts. The change is to respond with the task instead of a message; however, this test update won't catch this case if this is ever regressed.
cfouant
force-pushed
the
delete-actkey
branch
from
bc25b38
to
1ea6aa7
Compare
theforeman-bot
added
Needs testing
Needs re-review
and removed
Waiting on contributor
labels
cfouant
force-pushed
the
delete-actkey
branch
2 times, most recently
from
51af34d
to
c99f5ed
Compare
|
Still not sure this is achieving what my previous comment was referring to. Since the change contained within this PR is to respond with the task object instead of a message for consistency, I feel the test should be updated to check that the response from the API is a task object. |
|
I think what ehelms is saying, is that we should try to somehow assert that the response is rendering with the appropriate template. So you may want to use something like: assert_template %w(katello/api/v2/common/async) |
cfouant
force-pushed
the
delete-actkey
branch
from
c99f5ed
to
d4ee2dd
Compare
cfouant
force-pushed
the
delete-actkey
branch
from
d4ee2dd
to
16b0d91
Compare
|
ACK |
cfouant
added a commit
that referenced
this pull request
Jan 30, 2015
fixes #8612 - Makes output consistent for deleting act key, BZ1171092
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.