add bitbucket repo url handling

[stubblyhead]

The current master is smart enough to recognize urls for githib repositories/branches/commits and transform them into the url for the corresponding gzip, but it doesn't know how to handle bitbucket urls. I've added a couple of regexes to recognize these patterns, and a few additional elsifs to make the necessary modifications when they're used.

[adamleff]

Hey @stubblyhead! Thanks for your first contribution to InSpec!

This seems like a pretty reasonable addition. Could you please add some unit tests to test this new functionality? They would look something like this, which is what we do for GitHub:

https://github.com/chef/inspec/blob/master/test/unit/fetchers/url_test.rb#L61-L91

[adamleff]

Can we remove changes to the .gitignore file from this PR? If we feel these changes are necessary, I'd like to address them separately. Thanks!

[stubblyhead]

Yes definitely. My intent was that my changes to .gitignore would themselves be ignored, but it didn't seem to work out that way. I'll have a go at the unit tests as well.

[stubblyhead]

A quick question about the unit tests--I'm not familiar with some of the gems being used in the tests, but if I'm reading things correctly it doesn't actually do an HTTP GET. Assuming that's correct, would that mean it doesn't really matter what urls are being used since it's just testing that the transformations are being applied correctly?

[adamleff]

@stubblyhead did you mean to close this PR?

re: your question, that's correct - the actual outbound HTTP GET is intercepted with these lines:

res = fetcher.resolve(url)
res.expects(:open).returns(mock_open)

That returned a Mock called mock_open that is created here: https://github.com/chef/inspec/blob/master/test/unit/fetchers/url_test.rb#L29-L34

That will prevent the outbound connection and instead leave you free to check that the transformed URL is correct.

[stubblyhead]

Oops sure didn't mean to close this, hit the wrong button when adding my comment. I'll commit my unit tests momentarily.

[adamleff]

@stubblyhead awesome tests, thank you! Just need to fix one indentation issue and then I think this is good to go.

[adamleff]

Lines 99-104 are indented too far. I realize this is a slight nitpick, but I was initially confused where the enumerator loop ended (i.e. was the test starting at line 106 part of the loop and just outdented incorrectly?) and that shows how important indentation should be :)

99-102 should be indented 2 spaces in from the it in line 98, the end on 103 should line up with the it on 98, and the end on 104 should line up with the %w on 92.

[stubblyhead]

Done

[adamleff]

@stubblyhead this looks great, thank you for your contribution! I'm rerunning the two failed Travis tests now which should go green - the failures are unrelated to your changes.

NOTE TO NEXT APPROVER: this should be squashed via GitHub instead of merged.

[arlimus]

Well done, thank you @stubblyhead for adding Bitbucket support!!
Kudos @adamleff for the review and support