ci: make the new GitHub workflow logic more defensive
cover ci: make the new GitHub workflow logic more defensive
Phil just reported that the check I added failed miserably
[https://github.com/phil-blain/git/runs/1238660958] in their fork, and the entire run would be abandoned. That's totally unintended, and was caused by a different metadata format for older recorded runs.
Let's work around this particular issue, and then make the entire check a lot more robust.
This patch series is based on js/ci-ghwf-dedup-tests.
Johannes Schindelin (2):
ci: work around old records of GitHub runs
ci: make the "skip-if-redundant" check more defensive
.github/workflows/main.yml | 58 ++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 27 deletions(-)
base-commit: 4463ce75b7eea47f9b484b05957def655d3f46d5
Published-As: https://github.com/gitgitgadget/git/releases/tag/pr-748%2Fdscho%2Fci-ghwf-dedup-tests-defensive-v1
Fetch-It-Via: git fetch https://github.com/gitgitgadget/git pr-748/dscho/ci-ghwf-dedup-tests-defensive-v1
Pull-Request: https://github.com/gitgitgadget/git/pull/748
Signature · 1 line
gitgitgadget
PATCH 1/2 ci: work around old records of GitHub runs
From: Johannes Schindelin <[email protected]>
Apparently older GitHub runs at least _sometimes_ lack information about the head_commit (and therefore the ci-config check will fail with "TypeError: Cannot read property 'tree_id' of null") in the check added in 7d78d5fc1a9 (ci: skip GitHub workflow runs for already-tested commits/trees, 2020-10-08).
Let's work around this by adding a defensive condition.
--- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
.github/workflows/main.yml +1 -1
@@ -65,7 +65,7 @@ jobs: core.setOutput('enabled', ' but skip'); break; } - if (tree_id === run.head_commit.tree_id) { + if (run.head_commit && tree_id === run.head_commit.tree_id) { core.warning(`Successful run for the tree ${tree_id}: ${run.html_url}`); core.setOutput('enabled', ' but skip'); break;
Signature · 1 line
gitgitgadget
PATCH 2/2 ci: make the "skip-if-redundant" check more defensive
In reply to Johannes Schindelin via GitGitGadgetFrom: Johannes Schindelin <[email protected]>
In 7d78d5fc1a9 (ci: skip GitHub workflow runs for already-tested commits/trees, 2020-10-08), we added a check that determines whether there is already a workflow run for the given commit (or at least tree), and if found, skips the current run.
We just worked around an issue with this check where older runs might unexpectedly miss the head_commit attribute.
Let's be even more defensive by catching all kinds of exceptions, logging them as warnings, and continue the run without skipping it (after all, if the check fails, we _want_ to continue with the run).
This commit is best viewed with the diff option -w because it increases the indentation level of the GitHub Action script by two spaces, surrounding it by a try ... catch construct.
--- .github/workflows/main.yml | 58 ++++++++++++++++++++------------------ 1 file changed, 31 insertions(+), 27 deletions(-)
.github/workflows/main.yml +31 -27
@@ -41,35 +41,39 @@ jobs: with: github-token: ${{secrets.GITHUB_TOKEN}} script: | - // Figure out workflow ID, commit and tree - const { data: run } = await github.actions.getWorkflowRun({ - owner: context.repo.owner, - repo: context.repo.repo, - run_id: context.runId, - }); - const workflow_id = run.workflow_id; - const head_sha = run.head_sha; - const tree_id = run.head_commit.tree_id; + try { + // Figure out workflow ID, commit and tree + const { data: run } = await github.actions.getWorkflowRun({ + owner: context.repo.owner, + repo: context.repo.repo, + run_id: context.runId, + }); + const workflow_id = run.workflow_id; + const head_sha = run.head_sha; + const tree_id = run.head_commit.tree_id; - // See whether there is a successful run for that commit or tree - const { data: runs } = await github.actions.listWorkflowRuns({ - owner: context.repo.owner, - repo: context.repo.repo, - per_page: 500, - status: 'success', - workflow_id, - }); - for (const run of runs.workflow_runs) { - if (head_sha === run.head_sha) { - core.warning(`Successful run for the commit ${head_sha}: ${run.html_url}`); - core.setOutput('enabled', ' but skip'); - break; - } - if (run.head_commit && tree_id === run.head_commit.tree_id) { - core.warning(`Successful run for the tree ${tree_id}: ${run.html_url}`); - core.setOutput('enabled', ' but skip'); - break; + // See whether there is a successful run for that commit or tree + const { data: runs } = await github.actions.listWorkflowRuns({ + owner: context.repo.owner, + repo: context.repo.repo, + per_page: 500, + status: 'success', + workflow_id, + }); + for (const run of runs.workflow_runs) { + if (head_sha === run.head_sha) { + core.warning(`Successful run for the commit ${head_sha}: ${run.html_url}`); + core.setOutput('enabled', ' but skip'); + break; + } + if (run.head_commit && tree_id === run.head_commit.tree_id) { + core.warning(`Successful run for the tree ${tree_id}: ${run.html_url}`); + core.setOutput('enabled', ' but skip'); + break; + } } + } catch (e) { + core.warning(e); } windows-build:
Signature · 1 line
gitgitgadget
Re: [PATCH 2/2] ci: make the "skip-if-redundant" check more defensive
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 3 lines · writes:
Let's be even more defensive by catching all kinds of exceptions, logging them as warnings, and continue the run without skipping it (after all, if the check fails, we _want_ to continue with the run).
Yeah, looks sensible.
"Johannes Schindelin via GitGitGadget" <[email protected]>
Quoted 4 lines · writes:
Phil just reported that the check I added failed miserably
[https://github.com/phil-blain/git/runs/1238660958] in their fork, and the entire run would be abandoned. That's totally unintended, and was caused by a different metadata format for older recorded runs.
Thanks for a quick response to breakage.
Quoted 4 lines
Let's work around this particular issue, and then make the entire check a lot more robust.
This patch series is based on js/ci-ghwf-dedup-tests.
Will queue.