JenkinsPipelineUnit icon indicating copy to clipboard operation
JenkinsPipelineUnit copied to clipboard

Transforming vars scripts to CpsScripts causes the tests to fail

Open mahmoud-ashi opened this issue 3 years ago • 4 comments

Hi. First of all, thanks for this great library. It has helped me a lot testing my Jenkin shared library!

This issue is inspired by the proposal from @haridsv here https://github.com/jenkinsci/JenkinsPipelineUnit/issues/103#issuecomment-596219941 and @nre-ableton comment here https://github.com/jenkinsci/JenkinsPipelineUnit/issues/308#issuecomment-707078194. I consider this issue as a continuation to the discussion in https://github.com/jenkinsci/JenkinsPipelineUnit/issues/308.

For my team, I have a big Jenkins shared library with many vars scripts. Our vars scripts do not contain only a simple call(...) function but instead many functions related to a specific topic. For example, a single vars script called gitlib with many functions related to git.

I have successfully wrote the library's tests and they work perfectly. There are however 2 issues which I am attempting to solve now:

  • Collect code coverage from the tests (jacoco apparently can collect coverage fine from the src classes but not from the vars scripts)
  • Speedup the setup of tests by moving the logic from the vars scripts to src classes as suggested by @nre-ableton

Therefore, I followed the proposal from @haridsv here https://github.com/jenkinsci/JenkinsPipelineUnit/issues/103#issuecomment-596219941 and moved all the vars scritps to classes as shows below. Surprisingly, my real client code was not affected by this change and worked just like before. However, now all the tests fail with some weird error.

Here is my entire repo structure and code (stripped down to minimum required to reproduce the issue): First of all, my 2 src classes (which were previously vars scripts). src/com/company/vars/Lib.groovy

package com.company.vars

import org.jenkinsci.plugins.workflow.cps.CpsScript

abstract class Lib extends CpsScript {
    void exec(cmd, label = "") {
        label = label ?: cmd

        if (isUnix()) {
            sh script: cmd, label: label
        } else {
            bat script: cmd, label: label
        }
    }
}

src/com/company/vars/Archive.groovy

package com.company.vars

import org.jenkinsci.plugins.workflow.cps.CpsScript

abstract class Archive extends CpsScript {
    void createFolders(String rootPath, String component) {
        String path = "${rootPath}/${component}"
        List folders = [
            "${path}/${component}/level1",
            "${path}/${component}/level2"
        ]

        folders.each { f ->
            lib.exec("mkdir -p -m 775 ${f}")
        }
    }

    void call(String component, String sourcesFile) {
        String rootPath = '/root_path'
        String componentPath = "${rootPath}/${component}"

        createFolders(rootPath, component)
    }
}

Next, my 2 vars scripts now vars/lib.groovy

import com.company.vars.Lib

@groovy.transform.BaseScript Lib lib

vars/archive.groovy

import com.company.vars.Archive 

@groovy.transform.BaseScript Archive archive

And finally, my tests: test/BaseTest.groovy

import com.lesfurets.jenkins.unit.BasePipelineTest
import org.junit.Before

class BaseTest extends BasePipelineTest {
    // Provides an easy way to access vars scripts instead of using `binding.getVariable("lib")`
    protected def vars = [:]

    @Before
    void beforeTest() {
        super.setUp()
        addBindings()
    }

    /**
     * Loads the specified vars script into the `binding`
     *
     * @param varsScriptName the name of the vars script to load. The name should not include the ".groovy" extension.
     */
    protected void loadVarsScript(String... varsScriptNames) {
        if (!varsScriptNames) {
            return
        }

        for (String varsScriptName in varsScriptNames) {
            if (binding.hasVariable(varsScriptName)) {
                continue
            }
            def s = loadScript("vars/${varsScriptName}.groovy")
            binding.setVariable(varsScriptName, s)
            vars[varsScriptName] = s
        }
    }

    protected void addBindings() {
        File varsDir = new File("vars")

        // Add bindings to main "vars" scripts
        for (File f : varsDir.listFiles()) {
            if (!f.name.endsWith(".groovy")) {
                continue
            }
            String fileName = f.name.replace(".groovy", "")
            loadVarsScript(fileName)
        }
    }

    @Override
    void registerAllowedMethods() {
        super.registerAllowedMethods()

        helper.registerAllowedMethod("isUnix", [], { return true })
    }
}

test/ArchiveTest.groovy

import com.lesfurets.jenkins.unit.MethodCall
import org.junit.Test

import static org.junit.Assert.assertTrue

class ArchiveTest extends BaseTest {
    @Test
    void "Calling archive function copies right files to the right destination"() {
        String component = "dummy_component"
        String sourceFiles = ["a", "b", "c"].join(",")

        vars.archive.call(component, sourceFiles)

        // Make sure fileCopyOperation was called with right args
        MethodCall fileCopyOperationCall = helper.getCallStack().find {
            it.methodName == "fileCopyOperation"
        }
        Map callArgs = fileCopyOperationCall.args[0] as Map
        assertTrue(callArgs.includes == sourceFiles)
        String targetLocation = callArgs.targetLocation as String
        assertTrue(targetLocation.contains(component))
    }
}

When running the ./gradlew clean test command, I am getting the following error:

No signature of method: static lib.exec() is applicable for argument types: (org.codehaus.groovy.runtime.GStringImpl) values: [mkdir -p -m 775 /root_path/dummy_component/dummy_component/level1]

Possible solutions: exec(java.lang.Object), exec(java.lang.Object, java.lang.Object), every(), grep(), grep(java.lang.Object), every(groovy.lang.Closure)

All the following experiments resulted also in the same error:

  • Originally in the this comment https://github.com/jenkinsci/JenkinsPipelineUnit/issues/103#issuecomment-596219941, the suggestion was to have the content of the vars script transform to the steps variable so I tried that but got the error above from the original code.
  • I tried adding a return this at the end of the vars scipts after the transform, again same error
  • I reverted again the vars scripts to have something like following (note that now I'm returning the object transformed) but again same error
import com.company.vars.Lib

@groovy.transform.BaseScript Lib lib
return this

Note that the error suggests that the I should replace the line lib.exec(...) with exec(...). When I do so, I get a java.lang.NoClassDefFoundError: javax/servlet/ServletException at the same line.

I feel that I am too close to the solution but I'm running out of options. I feel that the problem is either in the vars scripts syntax, or in the addBindings() method in the BaseTest class.

I would really appreciate your help here. Thanks in advance.

mahmoud-ashi avatar Mar 15 '21 19:03 mahmoud-ashi

Friendly reminder please :).

mahmoud-ashi avatar Mar 23 '21 21:03 mahmoud-ashi

Dumb question, but have you tried properly declaring the types in the method?

The error is No signature of method: static lib.exec() is applicable for argument types: (org.codehaus.groovy.runtime.GStringImpl) values: ..., and the signature of exec is void exec(cmd, label = ""). Have you tried declaring it as void exec(String cmd, String label = "")?

nre-ableton avatar Mar 24 '21 06:03 nre-ableton

@mahmoud-ashi The error is familiar to me and I have a workaround which involves changing this line:

        lib.exec("mkdir -p -m 775 ${f}")

to this line

        this.lib.exec("mkdir -p -m 775 ${f}")

For some reason, the this. qualifier is only needed for the custom steps. Unfortunately, this is one drawback of this approach, but the extra qualifier doesn't harm the runtime in Jenkins pipeline so I find it an acceptable thing to do. In fact, one could get away with the var file completely (though having it is helpful in a couple of scenarios) by instantiating the dependency explicitly (though you would need to make it a non-abstract class by adding a dummy run method), like this:

abstract class Archive extends CpsScript {
    Lib lib = new Lib()

In the above scenario, you would access the lib member as this.lib anyway. This approach will also makes it simpler to build objects via composition.

haridsv avatar Mar 24 '21 13:03 haridsv

Thanks @nre-ableton and @haridsv for the suggestions. I took some time trying what @haridsv suggested and that actually made the tests work now with related to functions that are calling functions from other scripts. However, calling println in functions still does not work. Is my loadVarsScript(...) function correct? Do you think the problem might be in it? I'm wondering what is special with loading the "vars" scripts with the tests in contrast to what Jenkins is doing as part of the "shared library plugin" that makes the functions work in both cases (with and without this) in Jenkins while they require the this for the tests to work. Could you please clarify? In my test, I only considered the test the I had in the original question. I will try today to update all the scripts I have and rerun all my tests collection to see what fails and where. I will update this issue so hopefully people seeing this post can benefit as well. Thanks again guys!

mahmoud-ashi avatar Mar 29 '21 09:03 mahmoud-ashi