[JENKINS-51489] Use ATH pre configured plugins mode from essentialsTest #65

raul-arabaolaza · 2018-05-23T14:57:04Z

JENKINS-51489

Downstream of:

Modify essentialsTest so it uses the new pre configured plugins mode introduced in ATH#433

@reviewbybees @oleg-nenashev @jglick

oleg-nenashev

Just a UX concern. The implementation LGTM

oleg-nenashev · 2018-05-24T09:41:49Z

vars/essentialsTest.groovy

@@ -2,6 +2,7 @@ def call(Map params = [:]) {
    def baseDir = params.containsKey('baseDir') ? params.baseDir : "."
    def metadataFile = params.containsKey('metadataFile') ? params.metadataFile : "essentials.yml"
    def labels = params.containsKey('labels') ? params.labels : "docker && highmem"
+    def pluginEvaluationOutcome = params.get("pluginEvaluationOutcome") ?: "failOnInvalid"


The argument name cannot be understood bu a user without reading docs.
I would rather prefe...

metadata: testPluginResolution: skipOnUnmetDependencies: true

with defaulting to failOnInvalid

Absolutely, that makes it much more nicer, will update the PR

raul-arabaolaza · 2018-05-24T10:04:56Z

vars/essentialsTest.groovy

@@ -10,7 +11,9 @@ def call(Map params = [:]) {

        dir(baseDir) {
            def metadataPath = "${pwd()}/${metadataFile}"
-            metadata = readYaml(file: metadataPath)
+            def configData = readYaml(file: metadataPath)


The variable was lacking a def and I have changed the name of it to avoid having a metadata.metadata in the code which may be misleading, and I like the idea of a metadatasection in the yml file

raul-arabaolaza · 2018-05-24T10:06:22Z

vars/essentialsTest.groovy

@@ -10,7 +11,9 @@ def call(Map params = [:]) {

        dir(baseDir) {
            def metadataPath = "${pwd()}/${metadataFile}"
-            metadata = readYaml(file: metadataPath)
+            def configData = readYaml(file: metadataPath)
+            testPluginResolution = configData.metadata?.testPluginResolution?.skipOnUnmetDependencies ?  "skipOnInvalid" : "failOnInvalid"


Basically this is allways failOnInvalid unless you have:

metadata: testPluginResolution: skipOnUnmetDependencies: true

raul-arabaolaza · 2018-05-25T10:40:22Z

Some reasonable changes requested in the upstream PR, I am going to modify this a little bit to accommodate to the new code

oleg-nenashev · 2018-05-26T21:10:48Z

Regarding metadata entry, it was rather a mistake from my side. I was about proposing

ath:
  testPluginResolution:
    skipOnUnmetDependencies: true

Having a separate section sounds reasonable anyway, because the same setting may be required for runATH() at some point. I just doubt that metadata is a best name since it's still a source of confusion with the "metadata file" term.

Not sure what would be a better name tho

raul-arabaolaza · 2018-05-28T16:10:46Z

@oleg-nenashev Yeah, I am all for a separate section because this is not intended to work on ath step alone, this setting makes sense in a concrete flow (essentials in this case) and the runATH already provides support for this kind of customization via the custom javaOptions parameter.

I am thinking on a flow section or something on that lines that can be used by shared libraries or Jenkinsfiles to get some metadata in order to customize a high level flow. essentialsTest is one of those. In that section you can add some semantic metadata like the testPluginResolution or something more generic like:

flow: 
  javaOptions:
    pct:
      - "-D..."
    ath: 
     - "-Djfjefjf"
  ath:
    testPluginResolution:
      skipOnUnmetDependencies: true

…H run Needed to be able to set the preconfigured plugins mode Also some rationalization of the final run params to make code more readable

raul-arabaolaza · 2018-06-11T08:38:07Z

@oleg-nenashev I had to fix some conflicts, all needed PR are already merged, can you re-bee this if you feel is worthwile?

oleg-nenashev

After splitting the flow section to "ath" and "pct" I see no particular reason to keep it outside the main "ath" and "pct" sections. But whatever works

raul-arabaolaza mentioned this pull request May 24, 2018

[JENKINS-51183] - Introduce Integration testing flow with BOM jenkinsci/artifact-manager-s3-plugin#20

Closed

raul-arabaolaza requested a review from oleg-nenashev May 24, 2018 08:09

oleg-nenashev approved these changes May 24, 2018

View reviewed changes

raul-arabaolaza commented May 24, 2018

View reviewed changes

oleg-nenashev approved these changes May 24, 2018

View reviewed changes

raul-arabaolaza added 6 commits June 11, 2018 10:23

[JENKINS-51489] Use ATH pre configured plugins mode from essentialsTest

9d05d87

[JENKINS-51489] Pass the outcome as string

75326e6

[JENKINS-51489] Better naming and move the resolution to metadata file

dfc69da

[JENKINS-51489] Add the ability to pass custom java options to the AT…

2b44b44

…H run Needed to be able to set the preconfigured plugins mode Also some rationalization of the final run params to make code more readable

[JENKINS-51489] pre configured plugin mode is passed now by java opts

6ea0103

[JENKINS-51489] Use a more meaningfull section in metadata file

a435806

raul-arabaolaza force-pushed the JENKINS-51489 branch from bf4ab2f to a435806 Compare June 11, 2018 08:35

raul-arabaolaza requested a review from oleg-nenashev June 11, 2018 08:38

oleg-nenashev approved these changes Jun 11, 2018

View reviewed changes

raul-arabaolaza merged commit 1034fc7 into jenkins-infra:master Jun 11, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[JENKINS-51489] Use ATH pre configured plugins mode from essentialsTest #65

[JENKINS-51489] Use ATH pre configured plugins mode from essentialsTest #65

raul-arabaolaza commented May 23, 2018 •

edited

Loading

oleg-nenashev left a comment

oleg-nenashev May 24, 2018

raul-arabaolaza May 24, 2018

raul-arabaolaza May 24, 2018

raul-arabaolaza May 24, 2018

raul-arabaolaza commented May 25, 2018

oleg-nenashev commented May 26, 2018

raul-arabaolaza commented May 28, 2018 •

edited

Loading

raul-arabaolaza commented Jun 11, 2018

oleg-nenashev left a comment

[JENKINS-51489] Use ATH pre configured plugins mode from essentialsTest #65

[JENKINS-51489] Use ATH pre configured plugins mode from essentialsTest #65

Conversation

raul-arabaolaza commented May 23, 2018 • edited Loading

oleg-nenashev left a comment

Choose a reason for hiding this comment

oleg-nenashev May 24, 2018

Choose a reason for hiding this comment

raul-arabaolaza May 24, 2018

Choose a reason for hiding this comment

raul-arabaolaza May 24, 2018

Choose a reason for hiding this comment

raul-arabaolaza May 24, 2018

Choose a reason for hiding this comment

raul-arabaolaza commented May 25, 2018

oleg-nenashev commented May 26, 2018

raul-arabaolaza commented May 28, 2018 • edited Loading

raul-arabaolaza commented Jun 11, 2018

oleg-nenashev left a comment

Choose a reason for hiding this comment

raul-arabaolaza commented May 23, 2018 •

edited

Loading

raul-arabaolaza commented May 28, 2018 •

edited

Loading