Skip to content

test: run patch tests for optional modifications #3793

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 75 additions & 0 deletions .github/workflows/optional-modifications.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
name: Optional Modifications

on:
workflow_dispatch:
schedule:
- cron: "0 0 */7 * *"
pull_request:
paths:
- "optional-modifications/**"
- ".github/workflows/optional-modifications.yml"

concurrency:
group: self-hosted-optional-modifications
cancel-in-progress: true

defaults:
run:
shell: bash

jobs:
test-apply-patch:
name: Test Apply Patch
runs-on: ubuntu-latest
permissions:
contents: read
issues: write
steps:
- name: Checkout
uses: actions/checkout@v4

- name: Apply patches
env:
GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
run: |
set -e

# For each directory in `optional-modifications/patches/`, we will try to execute:
# `patch < optional-modifications/patches/<dir>/<file>.patch`
# with `<file>` being the name of the file in the directory that we loop through.
#
# If the patch fails, we will open a GitHub issue with the failing patch file.
# To prevent so many issues created, we will check first whether there is already
# an issue open with the same patch `<dir>` (without the file name).

for patch_dir in optional-modifications/patches/*; do
echo "::group::Checking for existing issue for $patch_dir"

issue_title="[Optional Modification Failure] $patch_dir"
issue_body=("Failure during applying patches for $patch_dir:")

for patch_file in "$patch_dir"/*.patch; do
echo "Checking for existing issue for $patch_file"
Copy link
Member

Choose a reason for hiding this comment

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

Misleading message?

patch_base_file_name=$(basename "$patch_file" .patch)
if ! patch -p0 -s -f --dry-run < "$patch_file"; then
problem=$(patch -p0 -f --dry-run < "$patch_file")
issue_body+=($problem)
echo "::error::Patch $patch_file failed to apply"
fi
done

# If the length of `issue_body` array is greater than 1,
# then we have a problem and we need to open an issue.
if [ "${#issue_body[@]}" -gt 1 ]; then
issue_exists=$(gh issue list --search "$issue_title in:title" --state open --json title --jq 'if length == 0 then empty end')
if [ -z "$issue_exists" ]; then
echo "Creating issue for $patch_dir"
gh issue create --title "$issue_title" --body "${issue_body[*]}"
fi
fi
Comment on lines +61 to +69
Copy link
Member

Choose a reason for hiding this comment

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

I'd say check for the issue first and if there's an existing issue skip the apply/test step too.


echo "::debug::Reset the repository state"
git reset --hard
Copy link
Member

Choose a reason for hiding this comment

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

These seem like cheap tests, only testing if the patch would apply cleanly. I think we can run these on PR for information and on master to create a new issue.

The "heavy" tests would be the ones which apply the patches and test if they actually work. This might be tricky.

At this point I'm wondering if we should spin off these patches into their own repos (for externals too) and find a way to apply these at install step easily. This would also offload the maintenance burden to whoever created the patches.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These seem like cheap tests, only testing if the patch would apply cleanly. I think we can run these on PR for information and on master to create a new issue.

The "heavy" tests would be the ones which apply the patches and test if they actually work. This might be tricky.

It's not just tricky. If it's something like this external Kafka with auth + TLS support, you'd need to spawn an actual external Kafka with those configurations. Yet, I do think it is too heavy to do, and this minimal tests should work fine.

Copy link
Member

Choose a reason for hiding this comment

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

If it's something like this external Kafka with auth + TLS support, you'd need to spawn an actual external Kafka with those configurations.

That's also why I want these in their own repos so those repo owners would be thinking about these, not us 😅


echo "::endgroup::"
done