Add review notes (#164)

- remove keep - ajust jelly, avdanvedDocs, and tests
jenkinsci · Nov 15, 2024 · 0ec2e3f · 0ec2e3f
1 parent 9620818
commit 0ec2e3f
Show file tree

Hide file tree

Showing 9 changed files with 41 additions and 111 deletions.
diff --git a/docs/AdvancedUsage.md b/docs/AdvancedUsage.md
@@ -91,25 +91,18 @@ node {
 
 ## TestConfig
 
-| Properties                                   | Default Value | Description                                                                                                                                                                                                                 |
-|----------------------------------------------|---------------|-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
-| **tbcPath**: String                          | ''            | The relative path of the .tbc file in the Configurations directory to be started for this execution. Use "KEEP" to use the currently loaded test bench configuration. If empty, no test bench configuration will be loaded. |
-| **tcfPath**: String                          | ''            | The relative path of the .tcf file in the Configurations directory to be started for this execution. Use "KEEP" to use the currently loaded test configuration. If empty, no test configuration will be loaded.             |
-| **forceConfigurationReload**: boolean        | false         | If true, always reload the configuration even if the same one is still active. Hint: This flag is only required for ecu.test versions less than 2023.4!                                                                     |
-| **constants**:  List\<[Constant](#constant)> | []            | The configured global constants remain available throughout the entire test execution.                                                                                                                                      |
+| Properties                                   | Default Value | Description                                                                                                                                                  |
+|----------------------------------------------|---------------|--------------------------------------------------------------------------------------------------------------------------------------------------------------|
+| **tbcPath**: String                          | ''            | The relative path of the .tbc file in the Configurations directory to be started for this execution. If empty, no test bench configuration will be loaded.   |
+| **tcfPath**: String                          | ''            | The relative path of the .tcf file in the Configurations directory to be started for this execution. If empty, no test configuration will be loaded.         |
+| **forceConfigurationReload**: boolean        | false         | If true, always reload the configuration even if the same one is still active. Hint: This flag is only required for ecu.test versions less than 2023.4!      |
+| **constants**:  List\<[Constant](#constant)> | []            | The configured global constants remain available throughout the entire test execution.                                                                       |
 
 ### Configuration Change Options
 
-- **New Configurations**: Both `.tbc` and `.tcf` files must be explicitly set whenever a new configuration is needed.
-
-- **KEEP Option**: The `KEEP` option prevents reloading new configurations and keeps the currently active ones.
-  This feature is considered **advanced** and is not recommended for most use cases.
-  `KEEP` must always be set for both configurations and no constants may be set.
-
+- **Load Configuration**: Both `.tbc` and `.tcf` files must be explicitly set whenever a new configuration is needed.
+- **Keep Configuration**: In ecu.test versions prior to 2023.4, setting `forceConfigurationReload` to `true` forces a configuration reload, even if the same configuration is still active.
 - **Unload Configurations**: Keeping values empty, such as `[tbcPath: '', tcfPath: '']` will unload the configuration.
-  Providing an invalid data type for `testConfig` (e.g., an empty list) will result in an error and halt the pipeline execution.
-
-- **Force Reload**: In ecu.test versions prior to 2023.4, setting `forceConfigurationReload` to `true` forces a configuration reload, even if the same configuration is still active.
 
 ## PublishConfig
 

diff --git a/src/main/groovy/de/tracetronic/jenkins/plugins/ecutestexecution/configs/TestConfig.groovy b/src/main/groovy/de/tracetronic/jenkins/plugins/ecutestexecution/configs/TestConfig.groovy
@@ -12,7 +12,11 @@ import hudson.Extension
 import hudson.model.AbstractDescribableImpl
 import hudson.model.Descriptor
 import hudson.util.FormValidation
+import net.sf.json.JSONObject
 import org.apache.commons.lang.StringUtils
+import org.jinterop.dcom.test.Test
+import org.kohsuke.stapler.Stapler
+import org.kohsuke.stapler.StaplerRequest
 import org.kohsuke.stapler.DataBoundConstructor
 import org.kohsuke.stapler.DataBoundSetter
 import org.kohsuke.stapler.QueryParameter

diff --git a/src/main/groovy/de/tracetronic/jenkins/plugins/ecutestexecution/util/ValidationUtil.groovy b/src/main/groovy/de/tracetronic/jenkins/plugins/ecutestexecution/util/ValidationUtil.groovy
@@ -57,9 +57,9 @@ class ValidationUtil {
     static validateConfigFile(String configFilePath, String fileExtension) {
         FormValidation returnValue = validateParameterizedValue(configFilePath, false)
         if (returnValue == FormValidation.ok() && !StringUtils.isEmpty(configFilePath)) {
-            if (!configFilePath.endsWith(fileExtension) && (configFilePath != 'KEEP')) {
+            if (!configFilePath.endsWith(fileExtension)) {
                 returnValue = FormValidation.error(
-                        "${configFilePath} has to be empty, either of file type ${fileExtension} or \"KEEP\".")
+                        "${configFilePath} has to be empty or of file type ${fileExtension}.")
             }
         }
         return returnValue

diff --git a/...resources/de/tracetronic/jenkins/plugins/ecutestexecution/configs/TestConfig/config.jelly b/...resources/de/tracetronic/jenkins/plugins/ecutestexecution/configs/TestConfig/config.jelly
@@ -2,20 +2,24 @@
 <j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form">
     <f:section title="${%configSection.title}">
         <f:description>${%configSection.description}</f:description>
-        <f:entry title="${%tbcPath.title}" description="${%tbcPath.description}" field="tbcPath">
-            <f:textbox/>
-        </f:entry>
-        <f:entry title="${%tcfPath.title}" description="${%tcfPath.description}" field="tcfPath">
-            <f:textbox/>
-        </f:entry>
-        <f:entry title="${%forceConfigurationReload.title}" description="${%forceConfigurationReload.description}"
-                 field="forceConfigurationReload">
-            <f:checkbox/>
-        </f:entry>
-        <f:advanced title="${%constants.title}">
-            <f:entry title="${%constants.title}" description="${%constants.description}" field="constants">
-                <f:repeatableProperty field="constants" add="${%constants.add}" minimum="0"/>
+        <f:radioBlock name="configOption" title="Load Configuration" value="loadConfig" checked="${instance.configOption == 'loadConfig'}" inline="true">
+            <f:entry title="${%tbcPath.title}" description="${%tbcPath.description}" field="tbcPath">
+                <f:textbox />
             </f:entry>
-        </f:advanced>
+            <f:entry title="${%tcfPath.title}" description="${%tcfPath.description}" field="tcfPath">
+                <f:textbox />
+            </f:entry>
+            <f:advanced title="${%constants.title}">
+                <f:entry title="${%constants.title}" description="${%constants.description}" field="constants">
+                    <f:repeatableProperty field="constants" add="${%constants.add}" minimum="0"/>
+                </f:entry>
+            </f:advanced>
+        </f:radioBlock>
+        <f:radioBlock name="configOption" title="Keep Configuration" value="keepConfig" checked="${instance.configOption == 'keepConfig'}" inline="true">
+                <f:entry title="${%forceConfigurationReload.title}" description="${%forceConfigurationReload.description}"
+                         field="forceConfigurationReload">
+                    <f:checkbox />
+                </f:entry>
+        </f:radioBlock>
     </f:section>
 </j:jelly>
diff --git a/...rces/de/tracetronic/jenkins/plugins/ecutestexecution/configs/TestConfig/config.properties b/...rces/de/tracetronic/jenkins/plugins/ecutestexecution/configs/TestConfig/config.properties
@@ -1,6 +1,7 @@
 configSection.title=Test Configuration
-configSection.description=Both the Test Bench Configuration and Test Configuration must be provided to load new configurations. \
-  For invalid data types, such as <code>testConfig: []</code>, an Error will be thrown.
+configSection.description=It is recommended to choose one of the following options:<br/><br/> \
+<li><b>Load Configuration:</b> Both the Test Bench Configuration and Test Configuration must be provided to load new configurations.</li> \
+<li><b>Keep Configuration:</b> Enabling this option retains the existing configuration for continued use throughout the execution.</li>
 constants.add=Add Global Constant
 constants.description=The configured global constants remain available throughout the entire test execution.
 constants.title=Global Constants
@@ -10,14 +11,8 @@ packageParameters.add=Add Package Parameter
 packageParameters.description=Parameters for package execution.
 packageParameters.title=Package Parameters
 tbcPath.description=The relative path of the .tbc file in the Configurations directory to be started for this execution. \
-  Use "KEEP" to use the currently loaded test bench configuration. \
-  The "KEEP" option is considered advanced and not recommended for typical use cases. \
-  "KEEP" must always be set for both configurations. \
   If empty, no test bench configuration will be loaded.
 tbcPath.title=Test Bench Configuration
 tcfPath.description=The relative path of the .tcf file in the Configurations directory to be started for this execution. \
-  Use "KEEP" to use the currently loaded test configuration. \
-  The "KEEP" option is considered advanced and not recommended for typical use cases.  \
-  "KEEP" must always be set for both configurations. \
   If empty, no test configuration will be loaded.
 tcfPath.title=Test Configuration
diff --git a/...test/groovy/de/tracetronic/jenkins/plugins/ecutestexecution/steps/RunPackageStepIT.groovy b/...test/groovy/de/tracetronic/jenkins/plugins/ecutestexecution/steps/RunPackageStepIT.groovy
@@ -183,41 +183,20 @@ class RunPackageStepIT extends IntegrationTestBase {
             jenkins.assertLogContains("-> With TCF=test.tcf", run)
     }
 
-    def 'Run pipeline by declaring KEEP in testConfig'() {
+    def 'Run pipeline by forcing configuration to reload in testConfig'() {
         given:
             WorkflowJob job = jenkins.createProject(WorkflowJob.class, 'pipeline')
             job.setDefinition(new CpsFlowDefinition("node { ttRunPackage testCasePath: 'test.pkg', " +
-                    "testConfig: [tbcPath: 'KEEP', tcfPath: 'KEEP'] }", true))
+                    "testConfig: [forceConfigurationReload: true] }", true))
 
             GroovyMock(RestApiClientFactory, global: true)
             RestApiClientFactory.getRestApiClient() >> new MockRestApiClient()
 
         expect:
             WorkflowRun run = job.scheduleBuild2(0).get()
             jenkins.assertLogContains("Executing package 'test.pkg'...", run)
-            jenkins.assertLogContains("-> With TBC=KEEP", run)
-            jenkins.assertLogContains("-> With TCF=KEEP", run)
-
     }
 
-    def 'Run pipeline with invalid data type for testConfig'() {
-        given:
-            WorkflowJob job = jenkins.createProject(WorkflowJob.class, 'pipeline')
-            job.setDefinition(new CpsFlowDefinition("node { ttRunPackage testCasePath: 'test.pkg', " +
-                    "testConfig: [] }", true))
-
-            GroovyMock(RestApiClientFactory, global: true)
-            RestApiClientFactory.getRestApiClient() >> new MockRestApiClient()
-
-        expect:
-            WorkflowRun run = jenkins.assertBuildStatus(Result.FAILURE, job.scheduleBuild2(0).get())
-            jenkins.assertLogContains("java.lang.ClassCastException", run)
-            jenkins.assertLogContains("expects class " +
-                    "de.tracetronic.jenkins.plugins.ecutestexecution.configs.TestConfig", run)
-            jenkins.assertLogContains("but received class java.util.ArrayList", run)
-    }
-
-
     def 'Run pipeline with package check'(){
         given:
             GroovyMock(RestApiClientFactory, global: true)

diff --git a/...test/groovy/de/tracetronic/jenkins/plugins/ecutestexecution/steps/RunProjectStepIT.groovy b/...test/groovy/de/tracetronic/jenkins/plugins/ecutestexecution/steps/RunProjectStepIT.groovy
@@ -131,38 +131,18 @@ class RunProjectStepIT extends IntegrationTestBase {
             jenkins.assertLogContains("-> With TCF=test.tcf", run)
     }
 
-    def 'Run pipeline by declaring KEEP in testConfig'() {
+    def 'Run pipeline by forcing configuration to reload in testConfig'() {
         given:
             WorkflowJob job = jenkins.createProject(WorkflowJob.class, 'pipeline')
             job.setDefinition(new CpsFlowDefinition("node { ttRunProject testCasePath: 'test.prj', " +
-                    "testConfig: [tbcPath: 'KEEP', tcfPath: 'KEEP'] }", true))
+                    "testConfig: [forceConfigurationReload: true] }", true))
 
             GroovyMock(RestApiClientFactory, global: true)
             RestApiClientFactory.getRestApiClient() >> new MockRestApiClient()
 
         expect:
             WorkflowRun run = job.scheduleBuild2(0).get()
             jenkins.assertLogContains("Executing project 'test.prj'...", run)
-            jenkins.assertLogContains("-> With TBC=KEEP", run)
-            jenkins.assertLogContains("-> With TCF=KEEP", run)
-
-    }
-
-    def 'Run pipeline with invalid data type for testConfig'() {
-        given:
-            WorkflowJob job = jenkins.createProject(WorkflowJob.class, 'pipeline')
-            job.setDefinition(new CpsFlowDefinition("node { ttRunProject testCasePath: 'test.prj', " +
-                    "testConfig: [] }", true))
-
-            GroovyMock(RestApiClientFactory, global: true)
-            RestApiClientFactory.getRestApiClient() >> new MockRestApiClient()
-
-        expect:
-            WorkflowRun run = jenkins.assertBuildStatus(Result.FAILURE, job.scheduleBuild2(0).get())
-            jenkins.assertLogContains("java.lang.ClassCastException", run)
-            jenkins.assertLogContains("expects class " +
-                    "de.tracetronic.jenkins.plugins.ecutestexecution.configs.TestConfig", run)
-            jenkins.assertLogContains("but received class java.util.ArrayList", run)
     }
 
     def 'Run pipeline with package check'() {

diff --git a/...t/groovy/de/tracetronic/jenkins/plugins/ecutestexecution/steps/RunTestFolderStepIT.groovy b/...t/groovy/de/tracetronic/jenkins/plugins/ecutestexecution/steps/RunTestFolderStepIT.groovy
@@ -210,14 +210,14 @@ class RunTestFolderStepIT extends IntegrationTestBase {
             jenkins.assertLogContains("-> With TCF=test.tcf", run)
     }
 
-    def 'Run pipeline by declaring KEEP in testConfig'() {
+    def 'Run pipeline by forcing configuration to reload in testConfig'() {
         given:
             setupTestFolder()
             WorkflowJob job = jenkins.createProject(WorkflowJob.class, 'pipeline')
             job.setDefinition(new CpsFlowDefinition(
                     "node { ttRunTestFolder  recursiveScan: true, " +
                             "testCasePath: '${folder.getRoot().getAbsolutePath().replace('\\', '\\\\')}'" +
-                            ", testConfig: [tbcPath: 'KEEP', tcfPath: 'KEEP'] }",
+                            ", testConfig: [forceConfigurationReload: true] }",
                     true))
 
             GroovyMock(RestApiClientFactory, global: true)
@@ -228,29 +228,6 @@ class RunTestFolderStepIT extends IntegrationTestBase {
             jenkins.assertLogContains('Found 3 package(s)', run)
             jenkins.assertLogContains('Found 3 project(s)', run)
             jenkins.assertLogContains("Executing package '${subPackage.getAbsolutePath()}'", run)
-            jenkins.assertLogContains("-> With TBC=KEEP", run)
-            jenkins.assertLogContains("-> With TCF=KEEP", run)
-    }
-
-    def 'Run pipeline with invalid data type for testConfig'() {
-        given:
-            setupTestFolder()
-            WorkflowJob job = jenkins.createProject(WorkflowJob.class, 'pipeline')
-            job.setDefinition(new CpsFlowDefinition(
-                    "node { ttRunTestFolder  recursiveScan: true, " +
-                            "testCasePath: '${folder.getRoot().getAbsolutePath().replace('\\', '\\\\')}'" +
-                            ", testConfig: [] }",
-                    true))
-
-            GroovyMock(RestApiClientFactory, global: true)
-            RestApiClientFactory.getRestApiClient() >> new MockRestApiClient()
-
-        expect:
-            WorkflowRun run = jenkins.assertBuildStatus(Result.FAILURE, job.scheduleBuild2(0).get())
-            jenkins.assertLogContains("java.lang.ClassCastException", run)
-            jenkins.assertLogContains("expects class " +
-                    "de.tracetronic.jenkins.plugins.ecutestexecution.configs.TestConfig", run)
-            jenkins.assertLogContains("but received class java.util.ArrayList", run)
     }
 
     def 'Run recursive scan pipeline'() {

diff --git a/...est/groovy/de/tracetronic/jenkins/plugins/ecutestexecution/util/ValidationUtilTest.groovy b/...est/groovy/de/tracetronic/jenkins/plugins/ecutestexecution/util/ValidationUtilTest.groovy
@@ -73,8 +73,6 @@ class ValidationUtilTest extends Specification {
             configFilePath | fileExtension | expectedKind
             'test.tcf'     | '.tcf'        | FormValidation.Kind.OK
             'test.tcf'     | '.tbc'        | FormValidation.Kind.ERROR
-            'KEEP'         | ''            | FormValidation.Kind.OK
-            'KEEP'         | '.tcf'        | FormValidation.Kind.OK
             '${CONFIG}'    | ''            | FormValidation.Kind.WARNING
             ''             | ''            | FormValidation.Kind.OK
             null           | null          | FormValidation.Kind.OK