Issue 175: Pass terraform plan file to apply

[jleopold28]

Adding support for Issue #175

[codecov-commenter]

Codecov Report

Merging #294 into master will increase coverage by 0.47%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #294      +/-   ##
==========================================
+ Coverage   76.04%   76.52%   +0.47%     
==========================================
  Files          38       39       +1     
  Lines         981     1001      +20     
  Branches      233      233              
==========================================
+ Hits          746      766      +20     
  Misses        122      122              
  Partials      113      113              

Impacted Files	Coverage Δ
src/PassPlanFilePlugin.groovy	100.00% <100.00%> (ø)

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 83486b5...712ab07. Read the comment docs.

[kmanning]

The two new closures aren't getting exercised in any test. https://codecov.io/gh/manheim/terraform-pipeline/pull/294/diff?src=pr&el=tree#diff-c3JjL1Bhc3NQbGFuRmlsZVBsdWdpbi5ncm9vdnk=

Just for the sake of ensuring there are no syntax errors, can you add 2 tests that do something as simple as exercising the closure, without validating anything. The test name can be something as simple as "runsWithoutError". Even without behavior assertions, merely executing the method under a unit test context will provide some benefit.

Eg: https://github.com/manheim/terraform-pipeline/blob/master/test/BuildStageTest.groovy#L18-L28

[jleopold28]

https://codecov.io/gh/manheim/terraform-pipeline/pull/294/diff?src=pr&el=tree#diff-c3JjL1Bhc3NQbGFuRmlsZVBsdWdpbi5ncm9vdnk=

@kmanning I added those tests. Let me know if there is anything else you want changed

[kmanning]

Nice. The change to use stash/unstash looks great =).

[kmanning]

I see the new runsStashPlan runsUnstashPlan tests - thanks for adding that!

I'm looking at the code coverage however, and it doesn't seem to have changed. Digging into the details of the test, it looks like you're mocking the closures, and the closures are the lines of code missing coverage.

The closure is the thing that I was interested in getting coverage on. If there were, say, a syntactic error, simply exercising the closure would catch that. Instead of exercising stashPlan and unstashPlan through apply, could we break it down to a more fine-grained unit test, exercise those methods directly?

Something along the lines of:

class UnstashPlan {
        @Test
        void justExercisingNotValidatingBehavior() {
            def plugin = new PassPlanFilePlugin()

            def unstashClosure = plugin.unstashPlan('myenv')
            unstashClosure.call { -> }
        }
}

^-- if you did the above, we should see code coverage go up, because we're actually exercising the closure.

If we wanted to take this further, we could even validate some amount of behavior. It's really important that the inner closure is executed - if calling the inner closure were mistakenly left off, it would have really detrimental impact on the behavior of the pipeline. So we could build more safety into our unit tests with something like:

class UnstashPlan {
        @Test
        void executesPassedClosure() {
            def wasCalled = false
            def passedClosure = { -> wasCalled = true }
            def plugin = new PassPlanFilePlugin()

            def unstashClosure = plugin.unstashPlan('myenv')
            unstashClosure.call(passedClosure)

            assertTrue(wasCalled)
        }

[jleopold28]

@kmanning I tried implementing that second block but I'm getting some errors.

Any ideas what to do about groovy.lang.MissingMethodException: No signature of method: PassPlanFilePlugin.echo() is applicable for argument types: (org.codehaus.groovy.runtime.GStringImpl) values: [Stashing tfplan-dev file]

[kmanning]

That makes sense. Your closure is running Jenkinsfile DSL methods, like echo.

    public Closure unstashPlan(String env) {
        return { closure ->
            String planFile = "tfplan-" + env
            echo "Unstashing ${planFile} file"
            unstash planFile
            closure()
        }

echo isn't defined in the plugin, and it's not defined in your test. When executing a closure, when the VM comes across a method, it does a search. See here:

1. this
2. owner
3. delegate

If the method can't be found after traversing those objects, it throws the MissingMethodException you're seeing.

In a "normal" terraform-pipeline, the owner or delegate would be the actual Jenkinsfile executing on Jenkins. But you're just in a unit test - you don't have access to the normal Jenkinsfile DSL methods. So what you can do is explicitly set a delegate, and use the DummyJenkinsfile to stub in for what would normally happen in Jenkins.

As an example, you can check out this test:

terraform-pipeline/test/TerraformEnvironmentStageTest.groovy

Lines 168 to 178 in 9bac14b

	@Test
	void doesNotBlowUpWhenRunningClosure() {
	Jenkinsfile.instance = spy(new Jenkinsfile())
	doReturn([:]).when(Jenkinsfile.instance).getEnv()
	Jenkinsfile.defaultNodeName = 'foo'
	def stage = new TerraformEnvironmentStage('foo')
	def closure = stage.pipelineConfiguration()
	closure.delegate = new DummyJenkinsfile()
	closure()
	}