Add invalid job case & improve error handling in check-for-build #1590
Conversation
Signed-off-by: Tyler Ohlsen <[email protected]>
Codecov Report
@@ Coverage Diff @@
## main #1590 +/- ##
=========================================
Coverage 94.63% 94.63%
Complexity 13 13
=========================================
Files 153 153
Lines 3281 3281
Branches 21 21
=========================================
Hits 3105 3105
Misses 173 173
Partials 3 3 Continue to review full report at Codecov.
|
jenkins/check-for-build.jenkinsfile
Outdated
| @@ -81,6 +82,8 @@ pipeline { | |||
| build job: "${TARGET_JOB_NAME}", parameters: [ | |||
| string(name: 'INPUT_MANIFEST', value: "${INPUT_MANIFEST}") | |||
| ], wait: true | |||
| } else { | |||
| throw new Exception("Job ${TARGET_JOB_NAME} is not a valid option") | |||
There was a problem hiding this comment.
Use error doc to make sure the build is marked a failure. I am not a fan of exceptions as standard flow of control mechanism.
There was a problem hiding this comment.
Fixed in latest commit by checking the job name matches the 2 currently-supported jobs (distribution-build-opensearch, distribution-build-opensearch-dashboards
jenkins/check-for-build.jenkinsfile
Outdated
| @@ -89,6 +92,7 @@ pipeline { | |||
| ) | |||
| } catch (err) { | |||
There was a problem hiding this comment.
Can we remove this catch block, its just swallowing errors that should be failing the build
There was a problem hiding this comment.
+1 here. I think the exceptions threw above will be caught here and won't fail the job.
There was a problem hiding this comment.
Good catch - I was adding on to existing code but see this would silently fail. Removed in latest commit.
jenkins/check-for-build.jenkinsfile
Outdated
| // TEST_MANIFEST param isn't applicable for distribution-build-opensearch-dashboards job yet, | ||
| // so we don't pass it to that job to prevent confusion from Jenkins UI |
There was a problem hiding this comment.
Could we just add the param (and not use it) to that job and avoid the error entirely?
There was a problem hiding this comment.
Removed if/else logic, and passing the same params regardless of job.
Signed-off-by: Tyler Ohlsen <[email protected]>
ohltyler
changed the title
…nsearch-project#1590) * Add invalid job case in check-for-build * Remove catch block; pass TEST_MANIFEST for all jobs Signed-off-by: Tyler Ohlsen <[email protected]>
Signed-off-by: Tyler Ohlsen [email protected]
Description
Adds a check for handling invalid jobs passed. Properly fails the job if there is any failure seen in the build or uploading steps. Follow-up to #1585
Sanity test with job name
distribution-build-invalid(Jenkins jobPlayground/ohltyler-check-for-build-2#3):Issues Resolved
Check List
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.