Migrate to serverspec tests

[patcon]

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]

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! 😘

[patcon]

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]

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]

We've got some pointers in the Berksfile to unpinned git repos, so I'm wondering whether we should commit the Berksfile lock to eliminate noise from the odd broken commit... Thoughts?

Or we could pin to a specific commit in these repos. Your call

[linc01n]

My only concern of pinning the other cookbook will raise conflict.

e.g. java
Other cookbook may depend to an older version if we pin to the github HEAD.

And if we pinned to a specific version, Berkshelf may not handle the dependency resolution well.

[patcon]

As I understand it, cookbooks that depend on this one never ever look at the Berksfile and its lockfile when doing dependency resolution -- just the metadata.rb. These are just used for us :)

The downside of locking is that our travis tests (once we get them going) might not immediately reveal a failure that I user might be seeing when an upstream cookbook breaks something -- we'd have to explicitly update the lockfile and commit it to see that failure. Either one of these approached could work for us

[patcon]

Yep, seems some cookbooks even add them the chefignore:
https://github.com/ramonskie/bamboo/blob/master/chefignore#L72

[patcon]

Platforms that I've confirmed working:

• centos-6
• centos-7
• ubuntu-12.04
• ubuntu-14.04

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]

Good to go!

[linc01n]

👍 Merged

[legal90]

@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]

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]

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]

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]

@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.

chef-stash/test/integration/helpers/serverspec/spec_helper.rb

Line 6 in 82fa409

$node = ::JSON.parse(File.read('/tmp/test-helper/node.json'))

Well, you can disregard my question. And thank you for explanation. 👍

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]

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 :)