search

bflad / chef-stash

Chef Cookbook for Atlassian Stash
Other
37 stars 42 forks source link

Migrate to serverspec tests #105

Closed patcon closed 9 years ago

patcon commented 9 years ago

The current minitest chef_handler stuff seems to be out-of-vogue. Normally, this wouldn't really matter, but given that it required chef_gem resources and compile time activity (which is now considered something to be wary of and uses minimally), I think we'd do well to move to busser-serverspec.

I've got most of the work done on a branch, but haven't create a PR yet. If I submit a minimally intrusive PR (with no new tests to begin with), would that be acceptable?

linc01n commented 9 years ago

Yes... I think we should move to serverspec. Can you remove the minitest in your PR? Thank you very much for all your contribution lately! :kissing_heart:

patcon commented 9 years ago

haha no worries! I would encourage you to merge the other now, and I'll work off it and likely submit for tomorrow. It'll be a bit of change, and the other PR is pretty simple, so I'd love to see that get in first :)

patcon commented 9 years ago

Heh. Took less effort than expected. I'll get the postgresql tests going in another PR :)

To be fair, I'm still running test-kitchen on the platforms, so probably good to wait on that first :)

patcon commented 9 years ago

Platforms that I've confirmed working:

The boxes are taking awhile to download for the remaining platforms, so I'll have to leave that for later. Feel free to test those yourself if you have time, but otherwise I'll confirm tomorrow

patcon commented 9 years ago

Good to go!

linc01n commented 9 years ago

:+1: Merged

legal90 commented 8 years ago

@patcon Could you please explain why have you added test-helper dependency to the Berksfile? As I know, chef-zero provisioner for TestKitchen automatically dumps the result node object to JSON: /tmp/kitchen/nodes/<instance>.json (on the node itself). So, what is the benefit of this helper?

patcon commented 8 years ago

Good point. I think I started using this pattern before chef_zero provisioner was default, and just never re-thought it :)

I suspect it's not needed, but I opened an issue in their queue for feedback (auto-ref'd above)

patcon commented 8 years ago

hm. As was brought up in the linked issue, the node object isn't a deepmerged hash, so it would seem that in order to use that, chef would need to be installed in the busser-serverspec gemspec

So it seems the cookbook is still useful. But I could be wrong

patcon commented 8 years ago

Or wait, we'd need a custom gemfile with chef: https://github.com/test-kitchen/busser-serverspec#-usage

All-in-all, I'd almost rather use the test-helper cookbook, but open to other options if someone wants to investigate

legal90 commented 8 years ago

@patcon Ah, now I got how it works. You dump the node object to the JSON file on the host machine in order to use this JSON in integration tests on "verify" step. https://github.com/bflad/chef-stash/blob/82fa40956d4116866b28368741dea4db7d77d5f7/test/integration/helpers/serverspec/spec_helper.rb#L6 Well, you can disregard my question. And thank you for explanation. :+1:

P.s. But personally I don't like this approach. IMO, it's more reliable to use pre-defined values in tests rather than ones generated dynamically (example in confluence cookbook).

patcon commented 8 years ago

fwiw, it's not always the right way, but when the logic is a little odd (particularly legacy anti-patterns like calculated attributes or state storing in attributes), sometimes you want it. We had a situation when setting a software version wasn't resulting in that version being installed, so it was very helpful to cross-check the /bin/mytool --version response with that specified in attr :)