Skip to content

Commit

Permalink
Introduce a separate GHA workflow for Pull Request Validation to vali…
Browse files Browse the repository at this point in the history
…date the correct commit hashes (#269)

## Summary

This PR adds a new workflow which largely reuses the existing workflow
for building/creating images, except that it runs on pull requests but
also uses the correct hashes. The PR workflow also has some
optimizations to cancel the previous test run on the same PR if a new
commit is pushed since that test result is outdated.

<!--- HINT: Replace #nnn with corresponding Issue number, if you are
fixing an existing issue -->

[Issue](https://github.com/linkedin/openhouse/issues/#nnn)] Briefly
discuss the summary of the changes made in this
pull request in 2-3 lines.

For all the boxes checked, please include additional details of the
changes made in this pull request.

The workflow on the main branch appears to be using the merge commit SHA
for tagging purposes: https://github.com/anothrNick/github-tag-action

However this causes issues where it can use outdated hashes of pull
requests due to using `github.event.pull_request.merge_commit_sha` which
does not seem consistent with the Pull Request HEAD SHA.

Documentation on the default behavior here that should fix it
https://github.com/actions/checkout/blob/main/README.md

This can cause PRs to have breaking commits but due to this workflow
file can pass the CI tests, making it difficult to work in the
repository. Another side effect of the PR is that committers branches
that have multiple commits may have difficulty debugging their failing
workflows locally since the CI tests are running on outdated files.

See testing below for replication of the issue.

## Changes

- [ ] Client-facing API Changes
- [ ] Internal API Changes
- [x] Bug Fixes
- [ ] New Features
- [ ] Performance Improvements
- [ ] Code Style
- [ ] Refactoring
- [ ] Documentation
- [ ] Tests


## Testing Done
<!--- Check any relevant boxes with "x" -->

- [ ] Manually Tested on local docker setup. Please include commands
ran, and their output.
- [ ] Added new tests for the changes made.
- [ ] Updated existing tests to reflect the changes made.
- [ ] No tests added or updated. Please explain why. If unsure, please
feel free to ask for help.
- [x] Some other form of testing like staging or soak time in
production. Please explain.

For all the boxes checked, include a detailed description of the testing
done for the changes made in this pull request.

Tested locally on my repo
Will-Lo#1 shows a PR with the old
behavior which has a clearly breaking change, but is passing tests since
the commit that opened the PR passed tests

Will-Lo#3 shows a PR that used to pass
with the commit that opened it, but now fails with the same breaking
change.

# Additional Information

- [ ] Breaking Changes
- [ ] Deprecations
- [ ] Large PR broken into smaller PRs, and PR plan linked in the
description.

For all the boxes checked, include additional details of the changes
made in this pull request.
  • Loading branch information
Will-Lo authored Dec 17, 2024
1 parent 7b341c8 commit eaf36db
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 36 deletions.
44 changes: 44 additions & 0 deletions .github/workflows/build-run-tests.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
name: Gradle Build and Run Tests

on:
workflow_call: # Allow to be run by other workflows

jobs:
build-and-run-tests:
name: Build and Run Tests
runs-on: ubuntu-latest
steps:
- name: Checkout project sources
uses: actions/checkout@v4

- name: Set up JDK 17
uses: actions/setup-java@v1
with:
distribution: 'microsoft'
java-version: '17'

- name: Validate Gradle Wrapper
uses: gradle/wrapper-validation-action@v1

- name: Build with Gradle
run: ./gradlew clean build

- name: Start Docker Containers
run: docker compose -f infra/recipes/docker-compose/oh-only/docker-compose.yml up -d --build

- name: Wait for Docker Containers to start
run: sleep 30

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.x'

- name: Install dependencies
run: pip install -r scripts/python/requirements.txt

- name: Run Integration Tests
run: python scripts/python/integration_test.py ./tables-test-fixtures/tables-test-fixtures-iceberg-1.2/src/main/resources/dummy.token

- name: Stop Docker Containers
run: docker compose -f infra/recipes/docker-compose/oh-only/docker-compose.yml down
40 changes: 4 additions & 36 deletions .github/workflows/build-tag-publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,52 +4,20 @@ on:
push:
branches:
- main
pull_request:
types: [opened, synchronize, reopened]

jobs:
build-gradle-project:
build-and-run-tests:
uses: ./.github/workflows/build-run-tests.yml

tag-publish-gradle:
name: Build tagged commit
runs-on: ubuntu-latest
steps:
- name: Checkout project sources
uses: actions/checkout@v4
with:
ref: ${{ github.event.pull_request.merge_commit_sha }}
fetch-depth: 0

- name: Set up JDK 17
uses: actions/setup-java@v1
with:
distribution: 'microsoft'
java-version: '17'

- name: Validate Gradle Wrapper
uses: gradle/wrapper-validation-action@v1

- name: Build with Gradle
run: ./gradlew clean build

- name: Start Docker Containers
run: docker compose -f infra/recipes/docker-compose/oh-only/docker-compose.yml up -d --build

- name: Wait for Docker Containers to start
run: sleep 30

- name: Set up Python
uses: actions/setup-python@v5
with:
python-version: '3.x'

- name: Install dependencies
run: pip install -r scripts/python/requirements.txt

- name: Run Integration Tests
run: python scripts/python/integration_test.py ./tables-test-fixtures/tables-test-fixtures-iceberg-1.2/src/main/resources/dummy.token

- name: Stop Docker Containers
run: docker compose -f infra/recipes/docker-compose/oh-only/docker-compose.yml down

- name: Bump version and push tag
if: ${{ success() && github.ref == 'refs/heads/main' && github.repository == 'linkedin/openhouse' }}
uses: anothrNick/github-tag-action@1.69.0
Expand Down
15 changes: 15 additions & 0 deletions .github/workflows/pr-validations.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
name: Pull Request Validations

on:
pull_request:
types: [opened, synchronize, reopened]
branches:
- main

concurrency:
group: ci-${{ github.event.pull_request.number || github.ref }}
cancel-in-progress: true

jobs:
build-run-tests:
uses: ./.github/workflows/build-run-tests.yml

0 comments on commit eaf36db

Please sign in to comment.