Implement propertyMissing method by Willem1987 · Pull Request #299 · jenkinsci/JenkinsPipelineUnit

Willem1987 · 2020-10-01T14:00:27Z

Undo extending all Declarations from GenericPipelineDeclaration (this adds unwanted properties)
Remove GenericPipelineDeclaration implementation of getProperty.
Instead implement propertyMissing method to fallback to env and params.
Append test to use env var before pipeline closure
Remove duplicate method executeOn (use executeWith instead)
Remove name conflicts between JenkinsFile closures and Declaration fields

Our JenkinsFile had this:

def call() {
    String someValue = SomeHelper.someMap.get(ENV_VAR);

    pipeline {
    }
}

It seems the previous fix to handle looking into env. and params. does not work outside the pipeline closure. Therefore i looked for another way to fix this.
During this i noticed the effect extending GenericPipelineDeclaration has on the function printNonNullProperties of ObjectUtils.
So i undid the extending of PipelineDeclaration for declarations that should not be able to handle all those functions(agent, steps etc).

stchar

Have a question regarding reverting some changes

src/main/groovy/com/lesfurets/jenkins/unit/declarative/AgentDeclaration.groovy

stchar · 2020-10-16T11:03:26Z

src/main/groovy/com/lesfurets/jenkins/unit/declarative/GenericPipelineDeclaration.groovy

@@ -14,24 +15,14 @@ abstract class GenericPipelineDeclaration {
    static <T> T createComponent(Class<T> componentType, @DelegatesTo(strategy = DELEGATE_FIRST) Closure<T> closure) {
        // declare componentInstance as final to prevent any multithreaded issues, since it is used inside closure
        final def componentInstance = componentType.newInstance()


I think at the moment we create a new instance of whatever we should call our custom class loader.
the problem is createComponent method is static so I'm not sure how to retrieve it.

so no additional juggling with bindings are required in execute neither with componentInstance

I'm sorry i don't quite follow. Are you saying we want to get rid of the createComponent call alltogether? The only change i did in this MR is flip 2 vars around. Making closure the delegate and componentInstance the owner. So that the closure is evaluated before accessing the Declaration class methods/vars

@stchar I tried simplifying the whole class loading.

The idea:
We only need stack interception for Script calls? (not the declaration setup calls, as far as i understand it)
The declaration classes get called to set properties as a sort of setup of the stack. When execute is called on the Declaration it uses the Script to get the correct depth and stack logging to work.
The parameters declaration now uses the binding of the closure (coming from PipelineTestHelper/BasePipelineTest)

A problem i ran into when trying to intercept the declaration classes is calling super methods. More specifically the super.execute call in NotDeclaration. The current MethodInterceptor seems unable to distinguish when to call NotDeclaration.execute vs super(/WhenDeclaration).execute. It seems to keep trying to intercept and invoke NotDeclaration.execute. This caused when_not_branch_master to fail.

So from that i tried just making sure the closures get bound to a Declaration delegate and the Declaration class delegates calls throug the script to get through the Stack interception

src/main/groovy/com/lesfurets/jenkins/unit/declarative/PostDeclaration.groovy

src/main/groovy/com/lesfurets/jenkins/unit/declarative/StageDeclaration.groovy

stchar · 2020-10-16T11:11:24Z

src/main/groovy/com/lesfurets/jenkins/unit/declarative/StageDeclaration.groovy


        if (!when || when.execute(delegate)) {
-            super.execute(delegate)
+            // Set environment for stage


I would prefer not to copy-paste same code in different classes inheritance here is a good practice

The stage should clear the environment/params after finishing so the next stage could inherit pipeline vars. But i guess we could implement the cleanup at GenericPipeline level

I created a public static method to init and reset the environment props. This way we can share the logic, but choose when to call the parts.

We want to init the env in a stage, but only reset it after the stage is complete. Super.execute would have immediately cleared the stage custom env overrides.

src/test/jenkins/jenkinsfiles/Environment_Jenkinsfile

… adds unwanted properties) Remove GenericPipelineDeclaration implementation of getProperty. Instead implement propertyMissing method to fallback to env and params. Append test to use env var before pipeline closure Remove duplicate method executeOn (use executeWith instead) Remove name conflicts between JenkinsFile closures and Declaration fields

…eclarations

Willem1987 force-pushed the implement-property-missing-method branch from 683593f to a08f1d5 Compare October 1, 2020 14:09

Willem1987 mentioned this pull request Oct 1, 2020

Fix MissingPropertyException to show the actual property missing #300

Merged

Willem1987 mentioned this pull request Oct 16, 2020

fix(declarative) withCredentails usernamePassword mocking conflict #311

Merged

stchar reviewed Oct 16, 2020

View reviewed changes

src/main/groovy/com/lesfurets/jenkins/unit/declarative/AgentDeclaration.groovy Show resolved Hide resolved

stchar reviewed Oct 16, 2020

View reviewed changes

src/main/groovy/com/lesfurets/jenkins/unit/declarative/PostDeclaration.groovy Outdated Show resolved Hide resolved

stchar reviewed Oct 16, 2020

View reviewed changes

src/main/groovy/com/lesfurets/jenkins/unit/declarative/StageDeclaration.groovy Outdated Show resolved Hide resolved

stchar reviewed Oct 16, 2020

View reviewed changes

src/test/jenkins/jenkinsfiles/Environment_Jenkinsfile Outdated Show resolved Hide resolved

Willem1987 force-pushed the implement-property-missing-method branch from a08f1d5 to 32dc9db Compare October 16, 2020 14:54

Willem1987 force-pushed the implement-property-missing-method branch from 6681845 to fd171b4 Compare June 7, 2021 18:30

Willem Borgesius and others added 3 commits June 7, 2021 20:39

Share initEnvironment and resetEnvironment logic to use in multiple D…

e8c7b86

…eclarations

Extract scoping testcase

3ec1c0a

Willem1987 force-pushed the implement-property-missing-method branch 5 times, most recently from 2e8d40d to e7360c8 Compare June 7, 2021 19:52

Simplify class interception

78b8154

Willem1987 force-pushed the implement-property-missing-method branch from e7360c8 to 78b8154 Compare June 7, 2021 19:53

Willem1987 mentioned this pull request Jun 7, 2021

Rehydrate components with thisObject pointing to the script #304

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement propertyMissing method#299

Implement propertyMissing method#299
Willem1987 wants to merge 4 commits intojenkinsci:masterfrom
Willem1987:implement-property-missing-method

Willem1987 commented Oct 1, 2020 •

edited

Loading

Uh oh!

stchar left a comment

Uh oh!

Uh oh!

stchar Oct 16, 2020

Uh oh!

stchar Oct 16, 2020 •

edited

Loading

Uh oh!

Willem1987 Oct 16, 2020

Uh oh!

Willem1987 Jun 7, 2021

Uh oh!

Uh oh!

Uh oh!

stchar Oct 16, 2020

Uh oh!

Willem1987 Oct 16, 2020

Uh oh!

Willem1987 Oct 16, 2020

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

Willem1987 commented Oct 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

stchar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

stchar Oct 16, 2020

Choose a reason for hiding this comment

Uh oh!

stchar Oct 16, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Willem1987 Oct 16, 2020

Choose a reason for hiding this comment

Uh oh!

Willem1987 Jun 7, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

stchar Oct 16, 2020

Choose a reason for hiding this comment

Uh oh!

Willem1987 Oct 16, 2020

Choose a reason for hiding this comment

Uh oh!

Willem1987 Oct 16, 2020

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Willem1987 commented Oct 1, 2020 •

edited

Loading

stchar Oct 16, 2020 •

edited

Loading