Skip to content

Use a wrapper function for github status in jenkins#4847

Open
pfultz2 wants to merge 9 commits into
developfrom
jenkins-git-status-wrapper
Open

Use a wrapper function for github status in jenkins#4847
pfultz2 wants to merge 9 commits into
developfrom
jenkins-git-status-wrapper

Conversation

@pfultz2
Copy link
Copy Markdown
Collaborator

@pfultz2 pfultz2 commented May 6, 2026

Motivation

This works similar to how it did before.

Technical Details

Changelog Category

Add a CHANGELOG.md entry for any option other than Not Applicable

    • Added: New functionality.
    • Changed: Changes to existing functionality.
    • Removed: Functionality or support that has been removed. (Compared to a previous release)
    • Optimized: Component performance that has been optimized or improved.
    • Resolved Issues: Known issues from a previous version that have been resolved.
    • Not Applicable: This PR is not to be included in the changelog.

Copilot AI review requested due to automatic review settings May 6, 2026 14:47
@pfultz2 pfultz2 requested a review from causten as a code owner May 6, 2026 14:47
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR introduces a Jenkins Pipeline wrapper to automatically set GitHub commit statuses (pending/failure/success) around a build closure, and refactors the rocmtest helper to use this wrapper instead of inline status-handling logic.

Changes:

  • Add autoSetGitStatus closure to centralize commit status updates for a build block.
  • Update rocmtest to rely on autoSetGitStatus and remove the previous try/catch/finally status logic.

Comment thread Jenkinsfile Outdated
Comment thread Jenkinsfile
Comment on lines +123 to +135
def autoSetGitStatus = { Map conf = [:], Closure body ->
def statusContext = conf.get("context", "Unknown")
def commitSha = ${env.GIT_COMMIT};
try {
setCommitStatus(commitSha, 'pending', statusContext, 'Building')
body()
}
catch (Exception ex) {
setCommitStatus(commitSha, 'failure', statusContext, 'Build failed')
throw ex
}
setCommitStatus(commitSha, 'success', statusContext, 'Build succeeded')
}
Comment thread Jenkinsfile Outdated
Comment on lines +149 to +153
@@ -136,13 +150,11 @@ def rocmtest = { Map conf = [:], Closure body ->
def statusContext = "Jenkins - ${variant}"
def buildResult = 'failure'

try {
autoSetGitStatus(context: "Jenkins - ${variant}") {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot apply changes based on this feedback

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the unused commitSha, statusContext, and buildResult variables from rocmtest in commit 7c14204.

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Copilot finished work on behalf of pfultz2 May 6, 2026 17:07
@codecov
Copy link
Copy Markdown

codecov Bot commented May 8, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #4847   +/-   ##
========================================
  Coverage    92.85%   92.85%           
========================================
  Files          584      584           
  Lines        30147    30147           
========================================
  Hits         27992    27992           
  Misses        2155     2155           
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@causten
Copy link
Copy Markdown
Collaborator

causten commented May 12, 2026

Rekicking the CI since the previous run could have been a CI internal error

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants