HTTPS support for web hooks

[Ken Dreyer]

Hi folks,

I've added preliminary support for HTTPS web hooks:

https://gitorious.org/~ktdreyer/gitorious/ktdreyers-mainline/commits/https-webhooks

I'm a newbie to Ruby testing so please be kind :-) ... I'm not sure
how to update the unit tests to match this change. See my "FIXME"
lines in test/unit/processors/web_hook_processor_test.rb. I can't just
mock Net::HTTP's post_form anymore. Can someone tell me how I should
adjust the unit tests?

Assuming I can get the tests adjusted, I'd like to submit a merge
request to get this into mainline.

- Ken

[Christian Johansen]

> I've added preliminary support for HTTPS web hooks:
>
> https://gitorious.org/~ktdreyer/gitorious/ktdreyers-mainline/commits/https-webhooks

Cool!

> I'm a newbie to Ruby testing so please be kind :-) ... I'm not sure
> how to update the unit tests to match this change. See my "FIXME"
> lines in test/unit/processors/web_hook_processor_test.rb. I can't just
> mock Net::HTTP's post_form anymore. Can someone tell me how I should
> adjust the unit tests?

Well, the old test simply stubs the method being used. In your updated
example that's not going to turn out pretty. I suggest giving FakeWeb a
look: https://github.com/chrisk/fakeweb

Let me know if you need some help making it work.

>
> Assuming I can get the tests adjusted, I'd like to submit a merge
> request to get this into mainline.

Sweet!

Christian

[Ken Dreyer]

Hi Christian,

On Wed, May 15, 2013 at 11:44 PM, Christian Johansen
<chri...@gmail.com> wrote:
> Well, the old test simply stubs the method being used. In your updated
> example that's not going to turn out pretty. I suggest giving FakeWeb a
> look: https://github.com/chrisk/fakeweb
>
> Let me know if you need some help making it work.

Thanks for the offer! I looked at FakeWeb and I see it doesn't
currently parse request bodies. Previously we "parsed" them by
stubbing post_form, so this would be a slight decrease in coverage: we
would only test that a POST has been made to the hook.url; we wouldn't
have insight into whether the payload is correct or not.

Is that acceptable?

- Ken

[Christian Johansen]

Sorry for the late response, this email drowned in my inbox...

> Thanks for the offer! I looked at FakeWeb and I see it doesn't
> currently parse request bodies. Previously we "parsed" them by
> stubbing post_form, so this would be a slight decrease in coverage: we
> would only test that a POST has been made to the hook.url; we wouldn't
> have insight into whether the payload is correct or not.
>
> Is that acceptable?

Hmm. I wasn't aware of this. The request body is kinda crucial here, but
I guess this will suffice for a unit test. In the long run, it would
probably be a good idea to have a proper integration test for the web
hooks. Until we have a UI for adding them, however, this will do.

Christian