Skip to content

Commit 8cd8f57

Browse files
authored
Say what is wrong with a pull request number, and test the head ref (#307) (#327)
Asking the job for 16360 spent minutes cloning and resetting the OpenModelica repository before failing with "Could not fetch refs/pull/16360/merge: either there is no such pull request, or GitHub cannot merge it into its base branch". Both of those were wrong about what had happened: 16360 is an *issue*. Issues and pull requests share one numbering on GitHub, so an issue number reaches the job looking exactly like a pull request number, and the answer to that is "there is no such pull request", which the message buried as one of two possibilities. The stage now asks GitHub what the number is - one git ls-remote, before the clone, the reset and the build - and says so: OpenModelica/OpenModelica has no pull request 16360. Issues and pull requests share one numbering there, so check that 16360 is not the number of an issue. The checkout separates the two cases as well. GitHub only has refs/pull/<N>/merge while it can merge the pull request into its base branch, so a pull request that conflicts, or one already closed, has only refs/pull/<N>/head. That is still something to test - the pull request on its own rather than as it would land - so it is used, with a warning saying which of the two was tested and what the head ref does not have. Nothing at all for that number is still an error. While here: a parameter is only recorded on a job once a build has run with its definition, so the build that first sees this file has the new ones as null, and every stage that reads them to decide whether to run would fail on it, not only a pull request one. They now fall back to what the definition says the default is. --- Generated by Claude Code.
1 parent 86f08e4 commit 8cd8f57

1 file changed

Lines changed: 48 additions & 12 deletions

File tree

‎.CI/Jenkinsfile‎

Lines changed: 48 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -124,25 +124,34 @@ pipeline {
124124
stage('pull request') {
125125
agent {
126126
node {
127-
label "${params.pull_request_node}"
127+
label "${params.pull_request_node ?: 'ryzen-5950x-1'}"
128128
customWorkspace 'ws/OpenModelicaLibraryTestingWork'
129129
}
130130
}
131131
options { skipDefaultCheckout() }
132132
when {
133133
beforeAgent true
134-
expression { params.pull_request.trim() }
134+
expression { pullRequest() }
135135
}
136136
steps {
137137
script {
138-
if (!(params.pull_request.trim() ==~ /[0-9]+/)) {
138+
if (!(pullRequest() ==~ /[0-9]+/)) {
139139
error "pull_request is a pull request number; got '${params.pull_request}'"
140140
}
141141
}
142+
// Before the clone, the reset and the build: a number that is not a
143+
// pull request costs minutes to find out about otherwise. Issues and
144+
// pull requests share one numbering, so an issue number gets this far.
145+
sh """
146+
if ! git ls-remote --exit-code https://github.com/OpenModelica/OpenModelica.git 'refs/pull/${pullRequest()}/*' > /dev/null; then
147+
echo "OpenModelica/OpenModelica has no pull request ${pullRequest()}. Issues and pull requests share one numbering there, so check that ${pullRequest()} is not the number of an issue."
148+
exit 1
149+
fi
150+
"""
142151
// One build of omc per pull request is kept, as for a branch, and they
143152
// accumulate: a pull request is tested once and never again.
144153
sh 'find "$HOME/saved_omc" -maxdepth 1 -name "pr-*" -type d -mtime +14 -exec rm -rf {} ";" || true'
145-
runRegressiontest("pr-${params.pull_request.trim()}", "pr-${params.pull_request.trim()}", '', '', false, '', '', false, false, 0, params.pull_request_config)
154+
runRegressiontest("pr-${pullRequest()}", "pr-${pullRequest()}", '', '', false, '', '', false, false, 0, params.pull_request_config ?: 'configs/conf.json')
146155
}
147156
}
148157

@@ -588,7 +597,7 @@ pipeline {
588597
}
589598
when {
590599
beforeAgent true
591-
expression { params.drop_stale_pull_request_tables }
600+
expression { params.drop_stale_pull_request_tables ?: false }
592601
}
593602
environment {
594603
PGPASSFILE = credentials('omdb-pgpass')
@@ -612,20 +621,20 @@ pipeline {
612621
}
613622
when {
614623
beforeAgent true
615-
expression { params.pull_request.trim() }
624+
expression { pullRequest() }
616625
}
617626
environment {
618627
PYTHONIOENCODING = 'utf-8'
619628
PGPASSFILE = credentials('omdb-pgpass')
620629
}
621630
steps {
622631
script {
623-
if (!(params.pull_request.trim() ==~ /[0-9]+/)) {
632+
if (!(pullRequest() ==~ /[0-9]+/)) {
624633
error "pull_request is a pull request number; got '${params.pull_request}'"
625634
}
626635
}
627636
sh 'rm -rf history'
628-
sh "./pr-report.py '${params.pull_request.trim()}' --baseline='${params.pull_request_baseline.trim()}'"
637+
sh "./pr-report.py '${pullRequest()}' --baseline='${(params.pull_request_baseline ?: 'master').trim()}'"
629638
// The summary to comment on the pull request with, in the build log
630639
// until there is a token to post it with.
631640
sh 'cat history/pr-*/00_comment.md'
@@ -634,6 +643,20 @@ pipeline {
634643
}
635644
}
636645
}
646+
/**
647+
* The pull request this job is testing, or "" when it is testing branches.
648+
*
649+
* A job only learns of a parameter that has been added to it once a build has
650+
* run with the definition, so the first build after this file changes sees the
651+
* new ones as null - which is every build of the pipeline, not only one asking
652+
* for a pull request, because the stages that ignore them still have to decide
653+
* whether to run. Everything that reads them therefore falls back to what the
654+
* definition says the default is.
655+
*/
656+
def pullRequest() {
657+
return (params.pull_request ?: '').trim()
658+
}
659+
637660
def omsimulatorHash() {
638661
return 'master'
639662
}
@@ -878,10 +901,23 @@ def runRegressiontest(branch, name, extraFlags, omsHash, omcompiler, extrasimfla
878901
// same workspace to trip over.
879902
def pullRequest = branch.startsWith('pr-') && branch.substring(3).isInteger() ? branch.substring(3) : ''
880903
def checkoutRef = pullRequest ? """
881-
if ! git fetch --force https://github.com/OpenModelica/OpenModelica.git refs/pull/${pullRequest}/merge; then
882-
echo "Could not fetch refs/pull/${pullRequest}/merge: either there is no such pull request, or GitHub cannot merge it into its base branch."
883-
exit 1
884-
fi
904+
REFS=`git ls-remote https://github.com/OpenModelica/OpenModelica.git "refs/pull/${pullRequest}/head" "refs/pull/${pullRequest}/merge"` || exit 1
905+
case "\$REFS" in
906+
*"refs/pull/${pullRequest}/merge"*)
907+
PRREF="refs/pull/${pullRequest}/merge" ;;
908+
*"refs/pull/${pullRequest}/head"*)
909+
# GitHub only has a merge ref while it can merge the pull request into
910+
# its base branch. Without one there is still something to test, only it
911+
# is the pull request on its own rather than as it would land.
912+
echo "WARNING: pull request ${pullRequest} has no merge ref: it conflicts with its base branch, or it is closed."
913+
echo "WARNING: testing refs/pull/${pullRequest}/head, which does not have what was merged into the base branch since it was branched."
914+
PRREF="refs/pull/${pullRequest}/head" ;;
915+
*)
916+
echo "OpenModelica/OpenModelica has no pull request ${pullRequest}."
917+
exit 1 ;;
918+
esac
919+
echo "Testing \$PRREF"
920+
git fetch --force https://github.com/OpenModelica/OpenModelica.git "\$PRREF" || exit 1
885921
git checkout -f --detach FETCH_HEAD || exit 1
886922
git fetch --tags --force || exit 1
887923
""" : """

0 commit comments

Comments
 (0)