From 0741e2bfe1e01f8ba3f47adb23c7d57339ab9802 Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Fri, 8 Nov 2024 12:20:53 +0100 Subject: [PATCH 1/2] github: fix the comments in test-osbuild-composer-integration The comments were referring to the merge-base between the PR and main being used to test the base case when in fact the base is the HEAD of main at the time the PR was created or updated. This commit fixes the comments to reflect reality. --- .../test-osbuild-composer-integration.yml | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/.github/workflows/test-osbuild-composer-integration.yml b/.github/workflows/test-osbuild-composer-integration.yml index 48b937b588..9e3d0348cd 100644 --- a/.github/workflows/test-osbuild-composer-integration.yml +++ b/.github/workflows/test-osbuild-composer-integration.yml @@ -8,19 +8,20 @@ name: "osbuild-composer integration" # author and reviewers of this change. The workflow works as follows: # # 1. Checks out osbuild/osbuild-composer -# 2. Replaces the osbuild/images dependency with the *merge base* of the open -# PR with main and runs osbuild-composer's unit tests. -# 3. If the unit tests on the merge base (step 2) succeed, replaces the +# 2. Replaces the osbuild/images dependency with the base of the PR (this is +# the HEAD of main at the time the PR was opened or updated) and runs +# osbuild-composer's unit tests. +# 3. If the unit tests on the base (step 2) succeed, replaces the # osbuild/images dependency with the *HEAD* of the open PR and runs -# osbuild-composer's unit tests. If the tests on the merge base failed, no -# further action is taken. +# osbuild-composer's unit tests. If the tests on the base failed, no further +# action is taken. # 4. At most one of two messages is posted: -# 4.1 Posts a message on the open PR only if the unit tests with the merge base -# (step 2) succeed and the unit tests with the PR HEAD (step 3) fail. This +# 4.1 Posts a message on the open PR only if the unit tests with the base (step +# 2) succeed and the unit tests with the PR HEAD (step 3) fail. This # combination of outcomes indicates that the PR is the one responsible for # the breakage. # 4.2 Updates the existing message on the open PR only if the unit tests with -# the merge base (step2) succeed, the unit tests with the PR HEAD (step 3) +# the base (step2) succeed, the unit tests with the PR HEAD (step 3) # succeed, and there is already a message posted by this workflow. This is # meant for cases where a PR initially breaks compatibility and then it gets # fixed. No message should be posted on a PR that doesn't affect the @@ -32,8 +33,8 @@ name: "osbuild-composer integration" # PR. Running on pull_request_target is needed to have access to repository # secrets (Schutzbot's GitHub token). # 2. If the unit tests in this repository fail, the integration will fail and -# the message will be posted (if the integration is not failing on the merge -# base). This will happen even if there's no actual integration issue, so +# the message will be posted (if the integration is not also failing on +# main). This will happen even if there's no actual integration issue, so # the message can be misleading. However, subsequent runs that fix the issue # will update the message accordingly. @@ -55,8 +56,7 @@ jobs: outputs: # Define job outputs # (see https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/passing-information-between-jobs#example-defining-outputs-for-a-job) - # One output for the test with the merge base and one for the test against - # the PR + # One output for the test with the base and one for the test against the PR base_test: ${{ steps.tests-base.outputs.base_test }} pr_test: ${{ steps.tests-pr.outputs.pr_test }} @@ -83,15 +83,15 @@ jobs: - name: Mark the working directory as safe for git run: git config --global --add safe.directory "$(pwd)" - - name: Update the osbuild/images reference to the merge base with main + - name: Update the osbuild/images reference to the base (main) env: - merge_base_sha: ${{ github.event.pull_request.base.sha }} + base_sha: ${{ github.event.pull_request.base.sha }} run: | cd osbuild-composer - go mod edit -replace github.com/osbuild/images=github.com/osbuild/images@$merge_base_sha + go mod edit -replace github.com/osbuild/images=github.com/osbuild/images@$base_sha ./tools/prepare-source.sh - - name: Run unit tests (merge base) + - name: Run unit tests (main) working-directory: osbuild-composer id: tests-base # This step will not fail if the test fails, but it will write the From 0e45bda16a048bd389b35de70f9458c160069eea Mon Sep 17 00:00:00 2001 From: Achilleas Koutsou Date: Fri, 8 Nov 2024 14:02:31 +0100 Subject: [PATCH 2/2] github: fix the repo-token option for mshick/add-pr-comment --- .github/workflows/test-osbuild-composer-integration.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test-osbuild-composer-integration.yml b/.github/workflows/test-osbuild-composer-integration.yml index 9e3d0348cd..71442e252d 100644 --- a/.github/workflows/test-osbuild-composer-integration.yml +++ b/.github/workflows/test-osbuild-composer-integration.yml @@ -140,9 +140,9 @@ jobs: steps: - name: Add comment (breakage) uses: mshick/add-pr-comment@v2 - repo-token: ${{ secrets.SCHUTZBOT_GITHUB_ACCESS_TOKEN }} if: needs.unit-tests.outputs.base_test == 1 && needs.unit-tests.outputs.pr_test == 0 with: + repo-token: ${{ secrets.SCHUTZBOT_GITHUB_ACCESS_TOKEN }} issue: ${{ github.event.pull_request.number }} message: | This PR changes the images API or behaviour causing integration failures with osbuild-composer. The next update of the images dependency in osbuild-composer will need work to adapt to these changes. @@ -151,9 +151,9 @@ jobs: - name: Update comment (fixed) uses: mshick/add-pr-comment@v2 - repo-token: ${{ secrets.SCHUTZBOT_GITHUB_ACCESS_TOKEN }} if: needs.unit-tests.outputs.pr_test == 1 with: + repo-token: ${{ secrets.SCHUTZBOT_GITHUB_ACCESS_TOKEN }} update-only: true # don't write a message if there isn't one already issue: ${{ github.event.pull_request.number }} message: |