Skip to content
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.

Sign up for GitHub

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

Correctly fetch same-named homeserver branches #1129

Merged
merged 8 commits into from
Sep 8, 2021
Merged

Correctly fetch same-named homeserver branches #1129

Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
812e68c
Dummy to run tests
Sep 6, 2021
749c21d
Another test
Sep 6, 2021
82dc637
Try to fix CI fetching synapse branch
Sep 6, 2021
0a2545f
Same fix for dendrite
Sep 6, 2021
7956b51
Revert the dummy test commits
Sep 6, 2021
7af373c
Kill typo
Sep 6, 2021
20f92da
Fix other typo (and trigger a CI run)
Sep 6, 2021
eaac430
Merge branch 'develop' of github.com:matrix-org/sytest into dmr/test-gha
Sep 6, 2021
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
48 changes: 42 additions & 6 deletions .github/workflows/pipeline.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,30 @@ jobs:
with:
path: sytest

# TODO the shell script below is nicked from complement. We use this pattern
# in a few places. Can we make this an Action so it's easier to reuse?
Comment on lines +63 to +64
Copy link
Member

Choose a reason for hiding this comment

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

+1

- name: Fetch corresponding synapse branch
shell: bash
run: |
BRANCH=${GITHUB_REF#refs/heads/}
(wget -O - https://github.com/matrix-org/synapse/archive/$BRANCH.tar.gz || wget -O - https://github.com/matrix-org/synapse/archive/develop.tar.gz) \
| tar -xz --strip-components=1 -C /src/
# Attempt to use the version of synapse which best matches the current
# build. Depending on whether this is a PR or release, etc. we need to
# use different fallbacks.
#
# 1. First check if there's a similarly named branch (GITHUB_HEAD_REF
# for pull requests, otherwise GITHUB_REF).
# 2. Attempt to use the base branch, e.g. when merging into release-vX.Y
# (GITHUB_BASE_REF for pull requests).
# 3. Use the default synapse branch ("develop").
for BRANCH_NAME in "$GITHUB_HEAD_REF" "$GITHUB_BASE_REF" "${GITHUB_REF#refs/heads/}" "develop"; do
Copy link
Member

Choose a reason for hiding this comment

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

Since we had an issue with "${GITHUB_REF#refs/heads/}" due to PR numbers, is it even worth including it on this list?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

GITHUB_HEAD_REF and GITHUB_BASE_REF only exist when the action is triggered by a pull request. https://docs.github.com/en/actions/reference/environment-variables#default-environment-variables. This action runs on specific branches too, outside of the context of pull requests:

name: Run sytest
on:
  push:
    branches: ["develop", "release-*"]
  pull_request

(but note that's only there because I copied it from synapse's CI file)

Assuming it's meaningful to run sytest on develop and release branches, I think we do still want to read GITHUB_REF.

Copy link
Member

Choose a reason for hiding this comment

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

Aha, I see. So in the case of a pull request, it's unlikely that we'll end up using "${GITHUB_REF#refs/heads/}". But it's useful on develop / release-*.

Yes, I think it's worthwhile to run Sytest on those branches.

# Skip empty branch names and merge commits.
if [[ -z "$BRANCH_NAME" || $BRANCH_NAME =~ ^refs/pull/.* ]]; then
continue
fi
(wget -O - "https://github.com/matrix-org/synapse/archive/$BRANCH_NAME.tar.gz" \
| tar -xz --strip-components=1 -C /src/) \
&& echo "Successfully downloaded and extracted $BRANCH_NAME.tar.gz" \
&& break
done

- name: Prepare blacklist file for running with workers
if: ${{ matrix.workers }}
Expand Down Expand Up @@ -129,12 +147,30 @@ jobs:
with:
path: sytest

# TODO the shell script below is nicked from complement. We use this pattern
# in a few places. Can we make this an Action so it's easier to reuse?
- name: Fetch corresponding dendrite branch
shell: bash
run: |
BRANCH=${GITHUB_REF#refs/heads/}
(wget -O - https://github.com/matrix-org/dendrite/archive/$BRANCH.tar.gz || wget -O - https://github.com/matrix-org/dendrite/archive/master.tar.gz) \
| tar -xz --strip-components=1 -C /src/
# Attempt to use the version of dendrite which best matches the current
# build. Depending on whether this is a PR or release, etc. we need to
# use different fallbacks.
#
# 1. First check if there's a similarly named branch (GITHUB_HEAD_REF
# for pull requests, otherwise GITHUB_REF).
# 2. Attempt to use the base branch, e.g. when merging into release-vX.Y
# (GITHUB_BASE_REF for pull requests).
# 3. Use the default dendrite branch ("master").
for BRANCH_NAME in "$GITHUB_HEAD_REF" "$GITHUB_BASE_REF" "${GITHUB_REF#refs/heads/}" "master"; do
# Skip empty branch names and merge commits.
if [[ -z "$BRANCH_NAME" || $BRANCH_NAME =~ ^refs/pull/.* ]]; then
continue
fi
(wget -O - "https://github.com/matrix-org/dendrite/archive/$BRANCH_NAME.tar.gz" \
| tar -xz --strip-components=1 -C /src/) \
&& echo "Successfully downloaded and extracted $BRANCH_NAME.tar.gz" \
&& break
done

- name: Run sytest
run: |
Expand Down