From 9889f6cdb8c81bcfcfad583892cd15276378c7ea Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Thu, 16 Jul 2020 20:26:38 -0400 Subject: [PATCH 01/17] WIP in-progress pr comments --- vars/githubPr.groovy | 58 +++++++++++++++++++++++++++++++++----------- 1 file changed, 44 insertions(+), 14 deletions(-) diff --git a/vars/githubPr.groovy b/vars/githubPr.groovy index 9dad2f88de2ec..e8cae1a28e79f 100644 --- a/vars/githubPr.groovy +++ b/vars/githubPr.groovy @@ -15,25 +15,38 @@ */ def withDefaultPrComments(closure) { catchErrors { + buildState.set('PR_COMMENTS_ENABLED', true) catchErrors { closure() } + sendComment() + } +} - if (!params.ENABLE_GITHUB_PR_COMMENTS || !isPr()) { - return - } +def sendComment() { + if (!params.ENABLE_GITHUB_PR_COMMENTS || !isPr() || !buildState.get('PR_COMMENTS_ENABLED')) { + return + } - def status = buildUtils.getBuildStatus() - if (status == "ABORTED") { - return; - } + def status = buildUtils.getBuildStatus() + if (status == "ABORTED") { + return; + } + + def lastComment = getLatestBuildComment() + def info = getLatestBuildInfo(lastComment) ?: [:] + info.builds = (info.builds ?: []).takeRight(5) // Rotate out old builds + + def shouldUpdateComment = !!info.builds.find { it.number == env.BUILD_NUMBER } - def lastComment = getLatestBuildComment() - def info = getLatestBuildInfo(lastComment) ?: [:] - info.builds = (info.builds ?: []).takeRight(5) // Rotate out old builds + // Need to re-factor above code to get the full comment objects back, instead of just the text - def message = getNextCommentMessage(info) - postComment(message) + def message = getNextCommentMessage(info) + + if (shouldUpdateComment) { + updateComment(lastComment.id, message) + } else { + createComment(message) if (lastComment && lastComment.user.login == 'kibanamachine') { deleteComment(lastComment.id) @@ -66,7 +79,7 @@ def getLatestBuildInfo() { } def getLatestBuildInfo(comment) { - return comment ? getBuildInfoFromComment(comment) : null + return comment ? getBuildInfoFromComment(comment.body) : null } def createBuildInfo() { @@ -141,6 +154,9 @@ def getNextCommentMessage(previousCommentInfo = [:]) { def info = previousCommentInfo ?: [:] info.builds = previousCommentInfo.builds ?: [] + // When we update an in-progress comment, we need to remove the old version from the history + info.builds = info.builds.filter { it.number != env.BUILD_NUMBER } + def messages = [] def status = buildUtils.getBuildStatus() @@ -208,7 +224,7 @@ def getNextCommentMessage(previousCommentInfo = [:]) { .join("\n\n") } -def postComment(message) { +def createComment(message) { if (!isPr()) { error "Trying to post a GitHub PR comment on a non-PR or non-elastic PR build" } @@ -224,6 +240,20 @@ def getComments() { } } +def updateComment(commentId, message) { + if (!isPr()) { + error "Trying to post a GitHub PR comment on a non-PR or non-elastic PR build" + } + + withGithubCredentials { + def path = "repos/elastic/kibana/issues/comments/${commentId}" + def json = toJSON([ body: message ]).toString() + + def resp = githubApi([ path: path ], [ method: "PATCH", data: json ]) + return toJSON(resp) + } +} + def deleteComment(commentId) { withGithubCredentials { def path = "repos/elastic/kibana/issues/comments/${commentId}" From 718e300c79bd156f299441f7dd9c8b4ecc9feae5 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Thu, 16 Jul 2020 20:28:00 -0400 Subject: [PATCH 02/17] Test Jenkinsfile --- Jenkinsfile | 51 ++++----------------------------------------------- 1 file changed, 4 insertions(+), 47 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index 69c61b5bfa988..afc4e451e759d 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -5,54 +5,11 @@ kibanaLibrary.load() kibanaPipeline(timeoutMinutes: 155, checkPrChanges: true, setCommitStatus: true) { githubPr.withDefaultPrComments { - ciStats.trackBuild { - catchError { - retryable.enable() - parallel([ - 'kibana-intake-agent': workers.intake('kibana-intake', './test/scripts/jenkins_unit.sh'), - 'x-pack-intake-agent': workers.intake('x-pack-intake', './test/scripts/jenkins_xpack.sh'), - 'kibana-oss-agent': workers.functional('kibana-oss-tests', { kibanaPipeline.buildOss() }, [ - 'oss-firefoxSmoke': kibanaPipeline.functionalTestProcess('kibana-firefoxSmoke', './test/scripts/jenkins_firefox_smoke.sh'), - 'oss-ciGroup1': kibanaPipeline.ossCiGroupProcess(1), - 'oss-ciGroup2': kibanaPipeline.ossCiGroupProcess(2), - 'oss-ciGroup3': kibanaPipeline.ossCiGroupProcess(3), - 'oss-ciGroup4': kibanaPipeline.ossCiGroupProcess(4), - 'oss-ciGroup5': kibanaPipeline.ossCiGroupProcess(5), - 'oss-ciGroup6': kibanaPipeline.ossCiGroupProcess(6), - 'oss-ciGroup7': kibanaPipeline.ossCiGroupProcess(7), - 'oss-ciGroup8': kibanaPipeline.ossCiGroupProcess(8), - 'oss-ciGroup9': kibanaPipeline.ossCiGroupProcess(9), - 'oss-ciGroup10': kibanaPipeline.ossCiGroupProcess(10), - 'oss-ciGroup11': kibanaPipeline.ossCiGroupProcess(11), - 'oss-ciGroup12': kibanaPipeline.ossCiGroupProcess(12), - 'oss-accessibility': kibanaPipeline.functionalTestProcess('kibana-accessibility', './test/scripts/jenkins_accessibility.sh'), - // 'oss-visualRegression': kibanaPipeline.functionalTestProcess('visualRegression', './test/scripts/jenkins_visual_regression.sh'), - ]), - 'kibana-xpack-agent': workers.functional('kibana-xpack-tests', { kibanaPipeline.buildXpack() }, [ - 'xpack-firefoxSmoke': kibanaPipeline.functionalTestProcess('xpack-firefoxSmoke', './test/scripts/jenkins_xpack_firefox_smoke.sh'), - 'xpack-ciGroup1': kibanaPipeline.xpackCiGroupProcess(1), - 'xpack-ciGroup2': kibanaPipeline.xpackCiGroupProcess(2), - 'xpack-ciGroup3': kibanaPipeline.xpackCiGroupProcess(3), - 'xpack-ciGroup4': kibanaPipeline.xpackCiGroupProcess(4), - 'xpack-ciGroup5': kibanaPipeline.xpackCiGroupProcess(5), - 'xpack-ciGroup6': kibanaPipeline.xpackCiGroupProcess(6), - 'xpack-ciGroup7': kibanaPipeline.xpackCiGroupProcess(7), - 'xpack-ciGroup8': kibanaPipeline.xpackCiGroupProcess(8), - 'xpack-ciGroup9': kibanaPipeline.xpackCiGroupProcess(9), - 'xpack-ciGroup10': kibanaPipeline.xpackCiGroupProcess(10), - 'xpack-accessibility': kibanaPipeline.functionalTestProcess('xpack-accessibility', './test/scripts/jenkins_xpack_accessibility.sh'), - 'xpack-savedObjectsFieldMetrics': kibanaPipeline.functionalTestProcess('xpack-savedObjectsFieldMetrics', './test/scripts/jenkins_xpack_saved_objects_field_metrics.sh'), - 'xpack-securitySolutionCypress': { processNumber -> - whenChanged(['x-pack/plugins/security_solution/', 'x-pack/test/security_solution_cypress/']) { - kibanaPipeline.functionalTestProcess('xpack-securitySolutionCypress', './test/scripts/jenkins_security_solution_cypress.sh')(processNumber) - } - }, - - // 'xpack-visualRegression': kibanaPipeline.functionalTestProcess('xpack-visualRegression', './test/scripts/jenkins_xpack_visual_regression.sh'), - ]), - ]) - } + catchErrors { + error "Test Error" } + githubPr.sendComment() + sleep 30 } if (params.NOTIFY_ON_FAILURE) { From 086ae7cda132ff758ec1212f318303a577f01961 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Thu, 16 Jul 2020 20:33:46 -0400 Subject: [PATCH 03/17] Fix method call --- Jenkinsfile | 2 +- vars/githubPr.groovy | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index afc4e451e759d..b154f9c071d0d 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -3,7 +3,7 @@ library 'kibana-pipeline-library' kibanaLibrary.load() -kibanaPipeline(timeoutMinutes: 155, checkPrChanges: true, setCommitStatus: true) { +kibanaPipeline(timeoutMinutes: 155, checkPrChanges: false, setCommitStatus: true) { githubPr.withDefaultPrComments { catchErrors { error "Test Error" diff --git a/vars/githubPr.groovy b/vars/githubPr.groovy index e8cae1a28e79f..81a34ceee0dcd 100644 --- a/vars/githubPr.groovy +++ b/vars/githubPr.groovy @@ -23,7 +23,7 @@ def withDefaultPrComments(closure) { } } -def sendComment() { +def sendComment(isFinal = false) { // TODO if (!params.ENABLE_GITHUB_PR_COMMENTS || !isPr() || !buildState.get('PR_COMMENTS_ENABLED')) { return } @@ -155,7 +155,7 @@ def getNextCommentMessage(previousCommentInfo = [:]) { info.builds = previousCommentInfo.builds ?: [] // When we update an in-progress comment, we need to remove the old version from the history - info.builds = info.builds.filter { it.number != env.BUILD_NUMBER } + info.builds = info.builds.findAll { it.number != env.BUILD_NUMBER } def messages = [] def status = buildUtils.getBuildStatus() From 522dcad2cc59979debcf6ce7c45ff5641f5ac8ba Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Thu, 16 Jul 2020 20:42:07 -0400 Subject: [PATCH 04/17] Testing workaround --- Jenkinsfile | 2 ++ vars/githubPr.groovy | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/Jenkinsfile b/Jenkinsfile index b154f9c071d0d..d4801e8c04840 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -9,6 +9,8 @@ kibanaPipeline(timeoutMinutes: 155, checkPrChanges: false, setCommitStatus: true error "Test Error" } githubPr.sendComment() + sleeep 3 + githubPr.sendComment() sleep 30 } diff --git a/vars/githubPr.groovy b/vars/githubPr.groovy index 81a34ceee0dcd..a3b7f8c3fae33 100644 --- a/vars/githubPr.groovy +++ b/vars/githubPr.groovy @@ -249,7 +249,7 @@ def updateComment(commentId, message) { def path = "repos/elastic/kibana/issues/comments/${commentId}" def json = toJSON([ body: message ]).toString() - def resp = githubApi([ path: path ], [ method: "PATCH", data: json ]) + def resp = githubApi([ path: path ], [ method: "POST", data: json, headers: [ "X-HTTP-Method-Override": "PATCH" ] ]) return toJSON(resp) } } From 5bec1cb354027fe76744557e068ce7db4d03b68b Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Thu, 16 Jul 2020 20:53:36 -0400 Subject: [PATCH 05/17] Add in-progress indicator --- Jenkinsfile | 2 +- vars/githubPr.groovy | 21 +++++++++++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index d4801e8c04840..d76948c003168 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -9,7 +9,7 @@ kibanaPipeline(timeoutMinutes: 155, checkPrChanges: false, setCommitStatus: true error "Test Error" } githubPr.sendComment() - sleeep 3 + sleep 3 githubPr.sendComment() sleep 30 } diff --git a/vars/githubPr.groovy b/vars/githubPr.groovy index a3b7f8c3fae33..da0131f841d42 100644 --- a/vars/githubPr.groovy +++ b/vars/githubPr.groovy @@ -41,7 +41,7 @@ def sendComment(isFinal = false) { // TODO // Need to re-factor above code to get the full comment objects back, instead of just the text - def message = getNextCommentMessage(info) + def message = getNextCommentMessage(info, isFinal) if (shouldUpdateComment) { updateComment(lastComment.id, message) @@ -150,7 +150,7 @@ def getTestFailuresMessage() { return messages.join("\n") } -def getNextCommentMessage(previousCommentInfo = [:]) { +def getNextCommentMessage(previousCommentInfo = [:], isFinal = false) { def info = previousCommentInfo ?: [:] info.builds = previousCommentInfo.builds ?: [] @@ -160,7 +160,15 @@ def getNextCommentMessage(previousCommentInfo = [:]) { def messages = [] def status = buildUtils.getBuildStatus() - if (status == 'SUCCESS') { + if (!isFinal) { + def failuresPart = status != 'SUCCESS' ? ', with failures' : '' + messages << """ + ## :hourglass_flowing_sand: Build in-progress${failuresPart} + * [continuous-integration/kibana-ci/pull-request](${env.BUILD_URL}) + * Commit: ${getCommitHash()} + * This comment will update when the build is complete + """ + } else if (status == 'SUCCESS') { messages << """ ## :green_heart: Build Succeeded * [continuous-integration/kibana-ci/pull-request](${env.BUILD_URL}) @@ -188,7 +196,9 @@ def getNextCommentMessage(previousCommentInfo = [:]) { * [Pipeline Steps](${env.BUILD_URL}flowGraphTable) (look for red circles / failed steps) * [Interpreting CI Failures](https://www.elastic.co/guide/en/kibana/current/interpreting-ci-failures.html) """ + } + if (status != 'SUCCESS' && status != 'UNSTABLE') { try { def steps = getFailedSteps() if (steps?.size() > 0) { @@ -202,7 +212,10 @@ def getNextCommentMessage(previousCommentInfo = [:]) { } messages << getTestFailuresMessage() - messages << ciStats.getMetricsReport() + + if (!isFinal) { + messages << ciStats.getMetricsReport() + } if (info.builds && info.builds.size() > 0) { messages << getHistoryText(info.builds) From ef8b2d530b8f28223607bde52afcc1b5991cffc4 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Thu, 16 Jul 2020 20:55:28 -0400 Subject: [PATCH 06/17] Fixes --- vars/githubPr.groovy | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/vars/githubPr.groovy b/vars/githubPr.groovy index da0131f841d42..5d14bf0dd82c9 100644 --- a/vars/githubPr.groovy +++ b/vars/githubPr.groovy @@ -19,11 +19,11 @@ def withDefaultPrComments(closure) { catchErrors { closure() } - sendComment() + sendComment(true) } } -def sendComment(isFinal = false) { // TODO +def sendComment(isFinal = false) { if (!params.ENABLE_GITHUB_PR_COMMENTS || !isPr() || !buildState.get('PR_COMMENTS_ENABLED')) { return } @@ -213,7 +213,7 @@ def getNextCommentMessage(previousCommentInfo = [:], isFinal = false) { messages << getTestFailuresMessage() - if (!isFinal) { + if (isFinal) { messages << ciStats.getMetricsReport() } From c2a9305a049fb3328063ce9378c06bee80ed0f9d Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Thu, 16 Jul 2020 21:14:26 -0400 Subject: [PATCH 07/17] Add sendCommentOnError where needed --- vars/githubPr.groovy | 13 +++++++++++++ vars/kibanaPipeline.groovy | 30 +++++++++++++++++++----------- vars/workers.groovy | 4 +++- 3 files changed, 35 insertions(+), 12 deletions(-) diff --git a/vars/githubPr.groovy b/vars/githubPr.groovy index 5d14bf0dd82c9..444a8d62caaf7 100644 --- a/vars/githubPr.groovy +++ b/vars/githubPr.groovy @@ -54,6 +54,19 @@ def sendComment(isFinal = false) { } } +def sendCommentOnError(Closure closure) { + try { + closure() + } catch (ex) { + catchErrors { + if (isPR()) { + sendComment(false) + } + } + throw ex + } +} + // Checks whether or not this currently executing build was triggered via a PR in the elastic/kibana repo def isPr() { return !!(env.ghprbPullId && env.ghprbPullLink && env.ghprbPullLink =~ /\/elastic\/kibana\//) diff --git a/vars/kibanaPipeline.groovy b/vars/kibanaPipeline.groovy index f43fe9f96c3ef..410578886a01d 100644 --- a/vars/kibanaPipeline.groovy +++ b/vars/kibanaPipeline.groovy @@ -35,7 +35,9 @@ def functionalTestProcess(String name, Closure closure) { "JOB=${name}", "KBN_NP_PLUGINS_BUILT=true", ]) { - closure() + githubPr.sendCommentOnError { + closure() + } } } } @@ -180,26 +182,32 @@ def bash(script, label) { } def doSetup() { - retryWithDelay(2, 15) { - try { - runbld("./test/scripts/jenkins_setup.sh", "Setup Build Environment and Dependencies") - } catch (ex) { + githubPr.sendCommentOnError { + retryWithDelay(2, 15) { try { - // Setup expects this directory to be missing, so we need to remove it before we do a retry - bash("rm -rf ../elasticsearch", "Remove elasticsearch sibling directory, if it exists") - } finally { - throw ex + runbld("./test/scripts/jenkins_setup.sh", "Setup Build Environment and Dependencies") + } catch (ex) { + try { + // Setup expects this directory to be missing, so we need to remove it before we do a retry + bash("rm -rf ../elasticsearch", "Remove elasticsearch sibling directory, if it exists") + } finally { + throw ex + } } } } } def buildOss() { - runbld("./test/scripts/jenkins_build_kibana.sh", "Build OSS/Default Kibana") + githubPr.sendCommentOnError { + runbld("./test/scripts/jenkins_build_kibana.sh", "Build OSS/Default Kibana") + } } def buildXpack() { - runbld("./test/scripts/jenkins_xpack_build_kibana.sh", "Build X-Pack Kibana") + githubPr.sendCommentOnError { + runbld("./test/scripts/jenkins_xpack_build_kibana.sh", "Build X-Pack Kibana") + } } def runErrorReporter() { diff --git a/vars/workers.groovy b/vars/workers.groovy index 8b7e8525a7ce3..74ce86516e863 100644 --- a/vars/workers.groovy +++ b/vars/workers.groovy @@ -126,7 +126,9 @@ def intake(jobName, String script) { return { ci(name: jobName, size: 's-highmem', ramDisk: true) { withEnv(["JOB=${jobName}"]) { - runbld(script, "Execute ${jobName}") + githubPr.sendCommentOnError { + runbld(script, "Execute ${jobName}") + } } } } From 1311bab05a1d96cc937fddb545067019edafc584 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Thu, 16 Jul 2020 21:21:55 -0400 Subject: [PATCH 08/17] Restore Jenkinsfile --- Jenkinsfile | 55 ++++++++++++++++++++++++++++++++++++++------ vars/githubPr.groovy | 6 +---- 2 files changed, 49 insertions(+), 12 deletions(-) diff --git a/Jenkinsfile b/Jenkinsfile index d76948c003168..69c61b5bfa988 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -3,15 +3,56 @@ library 'kibana-pipeline-library' kibanaLibrary.load() -kibanaPipeline(timeoutMinutes: 155, checkPrChanges: false, setCommitStatus: true) { +kibanaPipeline(timeoutMinutes: 155, checkPrChanges: true, setCommitStatus: true) { githubPr.withDefaultPrComments { - catchErrors { - error "Test Error" + ciStats.trackBuild { + catchError { + retryable.enable() + parallel([ + 'kibana-intake-agent': workers.intake('kibana-intake', './test/scripts/jenkins_unit.sh'), + 'x-pack-intake-agent': workers.intake('x-pack-intake', './test/scripts/jenkins_xpack.sh'), + 'kibana-oss-agent': workers.functional('kibana-oss-tests', { kibanaPipeline.buildOss() }, [ + 'oss-firefoxSmoke': kibanaPipeline.functionalTestProcess('kibana-firefoxSmoke', './test/scripts/jenkins_firefox_smoke.sh'), + 'oss-ciGroup1': kibanaPipeline.ossCiGroupProcess(1), + 'oss-ciGroup2': kibanaPipeline.ossCiGroupProcess(2), + 'oss-ciGroup3': kibanaPipeline.ossCiGroupProcess(3), + 'oss-ciGroup4': kibanaPipeline.ossCiGroupProcess(4), + 'oss-ciGroup5': kibanaPipeline.ossCiGroupProcess(5), + 'oss-ciGroup6': kibanaPipeline.ossCiGroupProcess(6), + 'oss-ciGroup7': kibanaPipeline.ossCiGroupProcess(7), + 'oss-ciGroup8': kibanaPipeline.ossCiGroupProcess(8), + 'oss-ciGroup9': kibanaPipeline.ossCiGroupProcess(9), + 'oss-ciGroup10': kibanaPipeline.ossCiGroupProcess(10), + 'oss-ciGroup11': kibanaPipeline.ossCiGroupProcess(11), + 'oss-ciGroup12': kibanaPipeline.ossCiGroupProcess(12), + 'oss-accessibility': kibanaPipeline.functionalTestProcess('kibana-accessibility', './test/scripts/jenkins_accessibility.sh'), + // 'oss-visualRegression': kibanaPipeline.functionalTestProcess('visualRegression', './test/scripts/jenkins_visual_regression.sh'), + ]), + 'kibana-xpack-agent': workers.functional('kibana-xpack-tests', { kibanaPipeline.buildXpack() }, [ + 'xpack-firefoxSmoke': kibanaPipeline.functionalTestProcess('xpack-firefoxSmoke', './test/scripts/jenkins_xpack_firefox_smoke.sh'), + 'xpack-ciGroup1': kibanaPipeline.xpackCiGroupProcess(1), + 'xpack-ciGroup2': kibanaPipeline.xpackCiGroupProcess(2), + 'xpack-ciGroup3': kibanaPipeline.xpackCiGroupProcess(3), + 'xpack-ciGroup4': kibanaPipeline.xpackCiGroupProcess(4), + 'xpack-ciGroup5': kibanaPipeline.xpackCiGroupProcess(5), + 'xpack-ciGroup6': kibanaPipeline.xpackCiGroupProcess(6), + 'xpack-ciGroup7': kibanaPipeline.xpackCiGroupProcess(7), + 'xpack-ciGroup8': kibanaPipeline.xpackCiGroupProcess(8), + 'xpack-ciGroup9': kibanaPipeline.xpackCiGroupProcess(9), + 'xpack-ciGroup10': kibanaPipeline.xpackCiGroupProcess(10), + 'xpack-accessibility': kibanaPipeline.functionalTestProcess('xpack-accessibility', './test/scripts/jenkins_xpack_accessibility.sh'), + 'xpack-savedObjectsFieldMetrics': kibanaPipeline.functionalTestProcess('xpack-savedObjectsFieldMetrics', './test/scripts/jenkins_xpack_saved_objects_field_metrics.sh'), + 'xpack-securitySolutionCypress': { processNumber -> + whenChanged(['x-pack/plugins/security_solution/', 'x-pack/test/security_solution_cypress/']) { + kibanaPipeline.functionalTestProcess('xpack-securitySolutionCypress', './test/scripts/jenkins_security_solution_cypress.sh')(processNumber) + } + }, + + // 'xpack-visualRegression': kibanaPipeline.functionalTestProcess('xpack-visualRegression', './test/scripts/jenkins_xpack_visual_regression.sh'), + ]), + ]) + } } - githubPr.sendComment() - sleep 3 - githubPr.sendComment() - sleep 30 } if (params.NOTIFY_ON_FAILURE) { diff --git a/vars/githubPr.groovy b/vars/githubPr.groovy index 444a8d62caaf7..e748f12a53af9 100644 --- a/vars/githubPr.groovy +++ b/vars/githubPr.groovy @@ -39,8 +39,6 @@ def sendComment(isFinal = false) { def shouldUpdateComment = !!info.builds.find { it.number == env.BUILD_NUMBER } - // Need to re-factor above code to get the full comment objects back, instead of just the text - def message = getNextCommentMessage(info, isFinal) if (shouldUpdateComment) { @@ -59,9 +57,7 @@ def sendCommentOnError(Closure closure) { closure() } catch (ex) { catchErrors { - if (isPR()) { - sendComment(false) - } + sendComment(false) } throw ex } From 43b1905c45d70bbaef0a504d9dc20b819c3b648e Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Fri, 17 Jul 2020 11:30:04 -0400 Subject: [PATCH 09/17] Add some temporary failures for testing --- test/scripts/jenkins_ci_group.sh | 4 ++++ test/scripts/jenkins_unit.sh | 1 + 2 files changed, 5 insertions(+) diff --git a/test/scripts/jenkins_ci_group.sh b/test/scripts/jenkins_ci_group.sh index 60d7f0406f4c9..177fcaac2ed84 100755 --- a/test/scripts/jenkins_ci_group.sh +++ b/test/scripts/jenkins_ci_group.sh @@ -3,6 +3,10 @@ source test/scripts/jenkins_test_setup_oss.sh if [[ -z "$CODE_COVERAGE" ]]; then + if [ "$CI_GROUP" == "1" ]; then + exit 1 + fi + checks-reporter-with-killswitch "Functional tests / Group ${CI_GROUP}" yarn run grunt "run:functionalTests_ciGroup${CI_GROUP}"; if [ "$CI_GROUP" == "1" ]; then diff --git a/test/scripts/jenkins_unit.sh b/test/scripts/jenkins_unit.sh index a9751003e8425..491f712d62edb 100755 --- a/test/scripts/jenkins_unit.sh +++ b/test/scripts/jenkins_unit.sh @@ -3,6 +3,7 @@ source test/scripts/jenkins_test_setup.sh if [[ -z "$CODE_COVERAGE" ]] ; then + exit 1 "$(FORCE_COLOR=0 yarn bin)/grunt" jenkins:unit --dev; else echo " -> Running jest tests with coverage" From 77c6d80c19e7eb65550b5e71431c7c8e0f102466 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Mon, 20 Jul 2020 10:53:52 -0400 Subject: [PATCH 10/17] Include failed steps that Jenkins thinks are still in-progress --- vars/jenkinsApi.groovy | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/vars/jenkinsApi.groovy b/vars/jenkinsApi.groovy index 1ea4c3dd76b8d..57818593ffeb2 100644 --- a/vars/jenkinsApi.groovy +++ b/vars/jenkinsApi.groovy @@ -10,7 +10,7 @@ def getSteps() { def getFailedSteps() { def steps = getSteps() - def failedSteps = steps?.findAll { it.iconColor == "red" && it._class == "org.jenkinsci.plugins.workflow.cps.nodes.StepAtomNode" } + def failedSteps = steps?.findAll { (it.iconColor == "red" || it.iconColor == "red_anime") && it._class == "org.jenkinsci.plugins.workflow.cps.nodes.StepAtomNode" } failedSteps.each { step -> step.logs = "${env.BUILD_URL}execution/node/${step.id}/log".toString() } From 7c40a4c009fa411376e1b6ea3b041738a2f88658 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Mon, 20 Jul 2020 10:58:37 -0400 Subject: [PATCH 11/17] Don't post after later builds --- vars/githubPr.groovy | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/vars/githubPr.groovy b/vars/githubPr.groovy index e748f12a53af9..467dd6cd73930 100644 --- a/vars/githubPr.groovy +++ b/vars/githubPr.groovy @@ -30,13 +30,18 @@ def sendComment(isFinal = false) { def status = buildUtils.getBuildStatus() if (status == "ABORTED") { - return; + return } def lastComment = getLatestBuildComment() def info = getLatestBuildInfo(lastComment) ?: [:] info.builds = (info.builds ?: []).takeRight(5) // Rotate out old builds + // If two builds are running at the same time, the first one should not post a comment after the second one + if (info.number && info.number.toInteger() > env.BUILD_NUMBER.toInteger()) { + return + } + def shouldUpdateComment = !!info.builds.find { it.number == env.BUILD_NUMBER } def message = getNextCommentMessage(info, isFinal) From f70e49cedeca72d486ddeca3862f9c043714572f Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Mon, 20 Jul 2020 11:05:39 -0400 Subject: [PATCH 12/17] Add temporary debug --- vars/jenkinsApi.groovy | 1 + 1 file changed, 1 insertion(+) diff --git a/vars/jenkinsApi.groovy b/vars/jenkinsApi.groovy index 57818593ffeb2..180ac0964146d 100644 --- a/vars/jenkinsApi.groovy +++ b/vars/jenkinsApi.groovy @@ -10,6 +10,7 @@ def getSteps() { def getFailedSteps() { def steps = getSteps() + print steps // TODO remove def failedSteps = steps?.findAll { (it.iconColor == "red" || it.iconColor == "red_anime") && it._class == "org.jenkinsci.plugins.workflow.cps.nodes.StepAtomNode" } failedSteps.each { step -> step.logs = "${env.BUILD_URL}execution/node/${step.id}/log".toString() From 3ac42c8ad6032fb1003358b9ec193fc4256eca63 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Mon, 20 Jul 2020 11:23:54 -0400 Subject: [PATCH 13/17] Some more debug --- vars/githubPr.groovy | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/vars/githubPr.groovy b/vars/githubPr.groovy index 467dd6cd73930..562589461c5c6 100644 --- a/vars/githubPr.groovy +++ b/vars/githubPr.groovy @@ -212,6 +212,10 @@ def getNextCommentMessage(previousCommentInfo = [:], isFinal = false) { """ } + // TODO remove + print "Status: ${status}" + getFailedSteps() + if (status != 'SUCCESS' && status != 'UNSTABLE') { try { def steps = getFailedSteps() From cff336a8300f7cc7ddfc61533e1c2a94728575ae Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Mon, 20 Jul 2020 11:34:49 -0400 Subject: [PATCH 14/17] Set build failure --- vars/githubPr.groovy | 2 ++ 1 file changed, 2 insertions(+) diff --git a/vars/githubPr.groovy b/vars/githubPr.groovy index 562589461c5c6..4fd709023f228 100644 --- a/vars/githubPr.groovy +++ b/vars/githubPr.groovy @@ -61,6 +61,8 @@ def sendCommentOnError(Closure closure) { try { closure() } catch (ex) { + // If this is the first failed step, it's likely that the error hasn't propagated up far enough to mark the build as a failure + currentBuild.result = 'FAILURE' catchErrors { sendComment(false) } From 3be02a7e7d566d7d55a2168fa80700eca4192bed Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Mon, 20 Jul 2020 11:49:54 -0400 Subject: [PATCH 15/17] Remove debug stuff --- vars/githubPr.groovy | 4 ---- vars/jenkinsApi.groovy | 1 - 2 files changed, 5 deletions(-) diff --git a/vars/githubPr.groovy b/vars/githubPr.groovy index 4fd709023f228..d176d97c3c1d4 100644 --- a/vars/githubPr.groovy +++ b/vars/githubPr.groovy @@ -214,10 +214,6 @@ def getNextCommentMessage(previousCommentInfo = [:], isFinal = false) { """ } - // TODO remove - print "Status: ${status}" - getFailedSteps() - if (status != 'SUCCESS' && status != 'UNSTABLE') { try { def steps = getFailedSteps() diff --git a/vars/jenkinsApi.groovy b/vars/jenkinsApi.groovy index 180ac0964146d..57818593ffeb2 100644 --- a/vars/jenkinsApi.groovy +++ b/vars/jenkinsApi.groovy @@ -10,7 +10,6 @@ def getSteps() { def getFailedSteps() { def steps = getSteps() - print steps // TODO remove def failedSteps = steps?.findAll { (it.iconColor == "red" || it.iconColor == "red_anime") && it._class == "org.jenkinsci.plugins.workflow.cps.nodes.StepAtomNode" } failedSteps.each { step -> step.logs = "${env.BUILD_URL}execution/node/${step.id}/log".toString() From ac94d28dad4882e590a3a5a72f368378f67f4785 Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Mon, 20 Jul 2020 14:13:00 -0400 Subject: [PATCH 16/17] Remove forced errors --- test/scripts/jenkins_ci_group.sh | 4 ---- test/scripts/jenkins_unit.sh | 1 - 2 files changed, 5 deletions(-) diff --git a/test/scripts/jenkins_ci_group.sh b/test/scripts/jenkins_ci_group.sh index 177fcaac2ed84..60d7f0406f4c9 100755 --- a/test/scripts/jenkins_ci_group.sh +++ b/test/scripts/jenkins_ci_group.sh @@ -3,10 +3,6 @@ source test/scripts/jenkins_test_setup_oss.sh if [[ -z "$CODE_COVERAGE" ]]; then - if [ "$CI_GROUP" == "1" ]; then - exit 1 - fi - checks-reporter-with-killswitch "Functional tests / Group ${CI_GROUP}" yarn run grunt "run:functionalTests_ciGroup${CI_GROUP}"; if [ "$CI_GROUP" == "1" ]; then diff --git a/test/scripts/jenkins_unit.sh b/test/scripts/jenkins_unit.sh index 491f712d62edb..a9751003e8425 100755 --- a/test/scripts/jenkins_unit.sh +++ b/test/scripts/jenkins_unit.sh @@ -3,7 +3,6 @@ source test/scripts/jenkins_test_setup.sh if [[ -z "$CODE_COVERAGE" ]] ; then - exit 1 "$(FORCE_COLOR=0 yarn bin)/grunt" jenkins:unit --dev; else echo " -> Running jest tests with coverage" From 20230ca8336835c862b4fc84497e47a6ad2e160a Mon Sep 17 00:00:00 2001 From: Brian Seeders Date: Mon, 20 Jul 2020 16:30:00 -0400 Subject: [PATCH 17/17] A little refactor based on PR comment --- vars/githubPr.groovy | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/vars/githubPr.groovy b/vars/githubPr.groovy index d176d97c3c1d4..da5348749f668 100644 --- a/vars/githubPr.groovy +++ b/vars/githubPr.groovy @@ -15,7 +15,9 @@ */ def withDefaultPrComments(closure) { catchErrors { - buildState.set('PR_COMMENTS_ENABLED', true) + // sendCommentOnError() needs to know if comments are enabled, so lets track it with a global + // isPr() just ensures this functionality is skipped for non-PR builds + buildState.set('PR_COMMENTS_ENABLED', isPr()) catchErrors { closure() } @@ -24,7 +26,7 @@ def withDefaultPrComments(closure) { } def sendComment(isFinal = false) { - if (!params.ENABLE_GITHUB_PR_COMMENTS || !isPr() || !buildState.get('PR_COMMENTS_ENABLED')) { + if (!buildState.get('PR_COMMENTS_ENABLED')) { return }