Improper null checking leads to test failures

[jantman]

Running the ./gradlew check --info tests locally on my system resulted in four failed tests:

FileParametersPluginTest > FileParametersPluginTest$GetVariables.handlesCarriageReturnCharacters FAILED
    java.lang.NullPointerException: Cannot get property 'env' on null object
        at org.codehaus.groovy.runtime.NullObject.getProperty(NullObject.java:60)
        at org.codehaus.groovy.runtime.InvokerHelper.getProperty(InvokerHelper.java:174)
        at org.codehaus.groovy.runtime.callsite.NullCallSite.getProperty(NullCallSite.java:47)
        at org.codehaus.groovy.runtime.callsite.GetEffectivePogoFieldSite.callGetProperty(GetEffectivePogoFieldSite.java:42)
        at Jenkinsfile.getEnv(Jenkinsfile.groovy:113)
        at Jenkinsfile$getEnv.call(Unknown Source)
        at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:48)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:113)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:117)
        at FileParametersPlugin.getEnv(FileParametersPlugin.groovy:45)
        at FileParametersPlugin.interpolate(FileParametersPlugin.groovy:41)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:93)
        at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:325)
        at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:384)
        at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1022)
        at org.codehaus.groovy.runtime.callsite.PogoMetaClassSite.callCurrent(PogoMetaClassSite.java:69)
        at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCallCurrent(CallSiteArray.java:52)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callCurrent(AbstractCallSite.java:154)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callCurrent(AbstractCallSite.java:166)
        at FileParametersPlugin$_getVariables_closure2.doCall(FileParametersPlugin.groovy:37)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:93)
        at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:325)
        at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:294)
        at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1022)
        at groovy.lang.Closure.call(Closure.java:414)
        at groovy.lang.Closure.call(Closure.java:430)
        at org.codehaus.groovy.runtime.DefaultGroovyMethods.collect(DefaultGroovyMethods.java:3156)
        at org.codehaus.groovy.runtime.DefaultGroovyMethods.collect(DefaultGroovyMethods.java:3127)
        at org.codehaus.groovy.runtime.dgm$65.invoke(Unknown Source)
        at org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite$PojoMetaMethodSiteNoUnwrapNoCoerce.invoke(PojoMetaMethodSite.java:274)
        at org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite.call(PojoMetaMethodSite.java:56)
        at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:48)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:113)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:125)
        at FileParametersPlugin.getVariables(FileParametersPlugin.groovy:37)
        at FileParametersPlugin$getVariables$0.call(Unknown Source)
        at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:48)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:113)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:125)
        at FileParametersPluginTest$GetVariables.handlesCarriageReturnCharacters(FileParametersPluginTest.groovy:67)

FileParametersPluginTest > FileParametersPluginTest$GetVariables.ignoresTrailingNewline FAILED
    java.lang.NullPointerException: Cannot get property 'env' on null object
        at org.codehaus.groovy.runtime.NullObject.getProperty(NullObject.java:60)
        at org.codehaus.groovy.runtime.InvokerHelper.getProperty(InvokerHelper.java:174)
        at org.codehaus.groovy.runtime.callsite.NullCallSite.getProperty(NullCallSite.java:47)
        at org.codehaus.groovy.runtime.callsite.GetEffectivePogoFieldSite.callGetProperty(GetEffectivePogoFieldSite.java:42)
        at Jenkinsfile.getEnv(Jenkinsfile.groovy:113)
        at Jenkinsfile$getEnv.call(Unknown Source)
        at FileParametersPlugin.getEnv(FileParametersPlugin.groovy:45)
        at FileParametersPlugin.interpolate(FileParametersPlugin.groovy:41)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:93)
        at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:325)
        at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:384)
        at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1022)
        at org.codehaus.groovy.runtime.callsite.PogoMetaClassSite.callCurrent(PogoMetaClassSite.java:69)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callCurrent(AbstractCallSite.java:166)
        at FileParametersPlugin$_getVariables_closure2.doCall(FileParametersPlugin.groovy:37)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:93)
        at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:325)
        at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:294)
        at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1022)
        at groovy.lang.Closure.call(Closure.java:414)
        at groovy.lang.Closure.call(Closure.java:430)
        at org.codehaus.groovy.runtime.DefaultGroovyMethods.collect(DefaultGroovyMethods.java:3156)
        at org.codehaus.groovy.runtime.DefaultGroovyMethods.collect(DefaultGroovyMethods.java:3127)
        at org.codehaus.groovy.runtime.dgm$65.invoke(Unknown Source)
        at org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite$PojoMetaMethodSiteNoUnwrapNoCoerce.invoke(PojoMetaMethodSite.java:274)
        at org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite.call(PojoMetaMethodSite.java:56)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:125)
        at FileParametersPlugin.getVariables(FileParametersPlugin.groovy:37)
        at FileParametersPlugin$getVariables$0.call(Unknown Source)
        at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:48)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:113)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:125)
        at FileParametersPluginTest$GetVariables.ignoresTrailingNewline(FileParametersPluginTest.groovy:56)

FileParametersPluginTest > FileParametersPluginTest$GetVariables.returnsAValueForEachLine FAILED
    java.lang.NullPointerException: Cannot get property 'env' on null object
        at org.codehaus.groovy.runtime.NullObject.getProperty(NullObject.java:60)
        at org.codehaus.groovy.runtime.InvokerHelper.getProperty(InvokerHelper.java:174)
        at org.codehaus.groovy.runtime.callsite.NullCallSite.getProperty(NullCallSite.java:47)
        at org.codehaus.groovy.runtime.callsite.GetEffectivePogoFieldSite.callGetProperty(GetEffectivePogoFieldSite.java:42)
        at Jenkinsfile.getEnv(Jenkinsfile.groovy:113)
        at Jenkinsfile$getEnv.call(Unknown Source)
        at FileParametersPlugin.getEnv(FileParametersPlugin.groovy:45)
        at FileParametersPlugin.interpolate(FileParametersPlugin.groovy:41)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:93)
        at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:325)
        at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:384)
        at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1022)
        at org.codehaus.groovy.runtime.callsite.PogoMetaClassSite.callCurrent(PogoMetaClassSite.java:69)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.callCurrent(AbstractCallSite.java:166)
        at FileParametersPlugin$_getVariables_closure2.doCall(FileParametersPlugin.groovy:37)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.codehaus.groovy.reflection.CachedMethod.invoke(CachedMethod.java:93)
        at groovy.lang.MetaMethod.doMethodInvoke(MetaMethod.java:325)
        at org.codehaus.groovy.runtime.metaclass.ClosureMetaClass.invokeMethod(ClosureMetaClass.java:294)
        at groovy.lang.MetaClassImpl.invokeMethod(MetaClassImpl.java:1022)
        at groovy.lang.Closure.call(Closure.java:414)
        at groovy.lang.Closure.call(Closure.java:430)
        at org.codehaus.groovy.runtime.DefaultGroovyMethods.collect(DefaultGroovyMethods.java:3156)
        at org.codehaus.groovy.runtime.DefaultGroovyMethods.collect(DefaultGroovyMethods.java:3127)
        at org.codehaus.groovy.runtime.dgm$65.invoke(Unknown Source)
        at org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite$PojoMetaMethodSiteNoUnwrapNoCoerce.invoke(PojoMetaMethodSite.java:274)
        at org.codehaus.groovy.runtime.callsite.PojoMetaMethodSite.call(PojoMetaMethodSite.java:56)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:125)
        at FileParametersPlugin.getVariables(FileParametersPlugin.groovy:37)
        at FileParametersPlugin$getVariables$0.call(Unknown Source)
        at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:48)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:113)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:125)
        at FileParametersPluginTest$GetVariables.returnsAValueForEachLine(FileParametersPluginTest.groovy:45)

FileParametersPluginTest > FileParametersPluginTest$GetVariables.interpolatesReferencesToOtherEnvironmentVariables FAILED
    java.lang.NullPointerException: Cannot get property 'env' on null object
        at org.codehaus.groovy.runtime.NullObject.getProperty(NullObject.java:60)
        at org.codehaus.groovy.runtime.InvokerHelper.getProperty(InvokerHelper.java:174)
        at org.codehaus.groovy.runtime.callsite.NullCallSite.getProperty(NullCallSite.java:47)
        at org.codehaus.groovy.runtime.callsite.GetEffectivePogoFieldSite.callGetProperty(GetEffectivePogoFieldSite.java:42)
        at Jenkinsfile.getEnv(Jenkinsfile.groovy:113)
        at Jenkinsfile$getEnv.call(Unknown Source)
        at FileParametersPlugin.getEnv(FileParametersPlugin.groovy:45)
        at FileParametersPlugin$getEnv$1.call(Unknown Source)
        at org.codehaus.groovy.runtime.callsite.CallSiteArray.defaultCall(CallSiteArray.java:48)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:113)
        at org.codehaus.groovy.runtime.callsite.AbstractCallSite.call(AbstractCallSite.java:117)
        at FileParametersPluginTest$GetVariables.interpolatesReferencesToOtherEnvironmentVariables(FileParametersPluginTest.groovy:77)

After tracing through the code, what I found is as follows:

1. Jenkinsfile.groovy defines public static instance = new Jenkinsfile() and public static original
2. and its reset() method sets instance = new Jenkinsfile() and original = null, among other things.
3. As a result of the above two items, Jenkinsfile.original may be null, but Jenkinsfile.instance is never null.
4. Jenkinsfile.getEnv() is a one-line method: return original.env
5. FileParametersPlugin.getEnv() method has the following body: return (Jenkinsfile.instance != null) ? Jenkinsfile.instance.getEnv() : [:]

The result of this is that Jenkinsfile.instance can never be null, but Jenkinsfile.original can be null. However, the null test in FileParametersPlugin.getEnv() is on .instance instead of .original. This renders it a noop (condition will always evaluate to true), yet it's possible that if Jenkinsfile.init() was never called, Jenkinsfile.original is still null.

As far as I know this should only occur during testing, and even then it appears to be order- or system-dependent.

I'll be opening a PR to fix this, but since my Java/Groovy is very rusty, I'd appreciate a sanity check on my understanding of the issue.

[kmanning]

• Fix the broken syntax for stubbing a spy method
• The other GetVariable tests actually pull environment variables from Jenkinsfile. That needs to be stubbed. Stubbing is coincidentally happening because some other test is not cleaning up after itself.
• Identify the other test that's setting Jenkinsfile.original, and make it clean up after itself. Gradle doesn't order unit tests, which means tests that fail to cleanup after themselves can sometimes make other tests to pass when they should not. Unit test execution order should be deterministic gradle/gradle#8520 - This is why the tests are non-deterministic.

[kmanning]

This is related to Issue #271