-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
base: master
Are you sure you want to change the base?
Changes from all commits
51cf17e
dc9ea6d
9a4cd1f
6792bfc
b2ed6b4
f7236de
1bfe113
b2ba39f
711dd84
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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" | ||
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
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Misleading message?