-
Notifications
You must be signed in to change notification settings - Fork 193
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
Handling Status for revoking request and test for revoke action #773
Conversation
'/updates/%s/request' % args['builds'], | ||
{'request': 'revoke', 'csrf_token': self.get_csrf_token()}) | ||
eq_(resp.json['update']['request'], None) | ||
eq_(resp.json['update']['status'], 'pending') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't the status be unpushed
here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pypingou I did unpushed
. But It is failing for unpushed
. So I kept it as pending
intentionally for the review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@trishnaguha if the test is failing then I guess there is a bug in the code :/
Since clearly according to your code the status should be unpushed
: https://github.com/fedora-infra/bodhi/pull/773/files#diff-55c0d4c740a773e117d1f620f264f793R1119
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@pypingou I'm trying to fix it. It would be helpful if you can give me some clue here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have honestly none, I just ran into this while reading your code nothing more.
Maybe try to add some debugging prints and see if the test goes through the code you added?
01d90f7
to
59c6e9c
Compare
notifications.publish(topic=topic, msg=dict( | ||
update=self, agent=username)) | ||
return | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So what happens to a request that is in testing, not pending for anything and being revoked?
This looks good to me, nice job @trishnaguha :) |
Looks good to me as well. Nice work @trishnaguha 💥 👍 ⚡ |
Handling Status for revoking request and test for revoke action
Trying to fix #645