infrablocks / ruby_terraform

A simple Ruby wrapper for invoking terraform commands.
MIT License
109 stars 33 forks source link

DRY command specs and minor cleanup of command class requires #48

Closed sa73917 closed 3 years ago

sa73917 commented 3 years ago

I've been working on adding support for all the new CLI options available in Terraform 0.14 and 0.15. As there are quite a few parameters available in the CLI that are not in RubyTerraform the PR was starting to look way too large to be digestable :-).

Rather than post a PR with a gazillion changes I thought this seemed like a good first pre-factoring step to seek review on. Essentially replacing the vast majority of the tests with shared examples. It drastically simplifies the specs and will make the new Terraform CLI options much easier to include in future PRs.

Notes:

  1. There were a couple of specs that had an initial test like:
  it 'calls the terraform init command' do
    command = RubyTerraform::Commands::Init.new(binary: 'terraform')

    expect(Open4).to(
        receive(:spawn)
            .with('terraform init', any_args))

    command.execute
  end

I didn't bother replicating these, given that there's usually 10-20 tests following them that all require their command line to start with the same string these were testing for (eg. terraform init <thing being tested>). Otherwise, as far as I can tell, I've replicated all the existing tests.

  1. There were a few specs (eg refresh_spec.rb) that included a -var-files test and a -var-file with -var-files test, but no singular -var-file test. I built the shared example from apply_spec.rb (that did include all three tests) so all the specs that use that shared example now include all three tests.
sa73917 commented 3 years ago

Thanks for the quick review. I've added a commit on the end of the branch that includes the guard gem in the development dependencies. I've found it really useful to get it to run the appropriate rspec on code change. More than happy to drop the commit if its of no interest, I can just include it locally when I'm developing but figured I'd check.