Skip to content

Remove the unmaintained Batect in favor of using a GitHub action container #7853

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

Merged
merged 2 commits into from
Jan 26, 2024
Merged

Remove the unmaintained Batect in favor of using a GitHub action container #7853

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ea7fbc8
refactor(github): Run functional tests against the snapshot Docker image
sschuberth Nov 13, 2023
dbec174
chore: Remove Batect as it has become unmaintained
sschuberth Nov 13, 2023
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
1 change: 0 additions & 1 deletion .dockerignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
# Ignore Docker-related files to avoid the cache being invalidated if the files are changed during development.
.batect/
.dockerignore
Dockerfile*
scripts/docker_*.sh
Expand Down
1 change: 0 additions & 1 deletion .gitattributes
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ gradlew text eol=lf

# Always use Windows line endings for Windows batch scripts.
*.bat text eol=crlf
*.cmd text eol=crlf

# Use Unix line endings for expected test results for consistency across platforms.
**/src/*/assets/**/*expected*.txt text eol=lf
Expand Down
14 changes: 5 additions & 9 deletions .github/workflows/build-and-test.yml
View file
Open in desktop
Copy link
Member

@mnonnenmacher mnonnenmacher Nov 17, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Commit message: "Docherfile" -> "Dockerfile"

The downside is that Dockerfile changes and e.g. package manager changes depending on it cannot be performed in the same commit / PR as the image first needs to get published.

Doesn't this introduce a serious issue if e.g. a tool version in Docker is upgraded and any test failures caused by this only appear in a random other PR later on? This probably also creates the risk that we make broken releases.

I thought with the new setup building the Docker image is a lot faster, as most of the images can be reused from the registry, and that only Batect was blocking us from using the registry. E.g. if the Python version is upgraded only the Python image needs to be rebuilt, and if nothing is changed in Docker the build should be very fast?

Copy link
Member Author

@sschuberth sschuberth Nov 17, 2023

Choose a reason for hiding this comment

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

Doesn't this introduce a serious issue if e.g. a tool version in Docker is upgraded and any test failures caused by this only appear in a random other PR later on?

Yes, that's a valid point. To solve this, we should probably do something like this in the PR:

  1. Pull the current snapshot image.
  2. (Re-)build the Docker image from source on the worker node, which should be fast due to stage image / layer caching, if not a lot has changed in the Dockerfile itself.
  3. Run functional tests against the local Docker image.
  4. Push the Docker image as the new snapshot if everything succeeded.

Copy link
Member Author

@sschuberth sschuberth Nov 17, 2023

Choose a reason for hiding this comment

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

Batect was blocking us from using the registry.

Not that it matters now anymore as Batect became unmaintained, but Batect was not blocking us from using the registry. Batect just made it hard / impossible to benefit from stage image / layer caching in conjunction with the docker/build-push-action.

E.g. if the Python version is upgraded only the Python image needs to be rebuilt, and if nothing is changed in Docker the build should be very fast?

That's my understanding as well, but I haven't verified it.

Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,9 @@ jobs:
flags: funTest-non-docker
funTest-docker:
runs-on: ubuntu-22.04
container:
image: ghcr.io/oss-review-toolkit/ort:snapshot
options: --user 1001
steps:
- name: Checkout Repository
uses: actions/checkout@v4
Expand All @@ -149,18 +152,11 @@ jobs:
with:
distribution: temurin
java-version: 17
- name: Setup Gradle
- name: Run functional tests that do require external tools
uses: gradle/gradle-build-action@v2
with:
gradle-home-cache-cleanup: true
- name: Run functional tests that do require external tools
run: |
echo "Running as $(id)."
BATECT_QUIET_DOWNLOAD=true ./batect --enable-buildkit \
--config-var docker_build_user_id=1001 \
--config-var gradle_build_scan=true \
--config-var gradle_console=plain \
runFunTest
arguments: --scan -Ptests.include=org.ossreviewtoolkit.plugins.packagemanagers.* funTest jacocoFunTestReport
- name: Upload code coverage data
uses: codecov/codecov-action@v3
with:
Expand Down
7 changes: 0 additions & 7 deletions .github/workflows/wrapper-validation.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,6 @@ on:
- main

jobs:
batect-wrapper:
runs-on: ubuntu-22.04
steps:
- name: Checkout Repository
uses: actions/checkout@v4
- name: Validate Wrapper
uses: batect/[email protected]
gradle-wrapper:
runs-on: ubuntu-22.04
steps:
Expand Down
1 change: 0 additions & 1 deletion .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
.batect/
.gradle/
.idea/
!.idea/icon*.png
Expand Down
4 changes: 0 additions & 4 deletions .reuse/dep5
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,6 @@ License: Apache-2.0

# Third-party files.

Files: batect*
Copyright: 2017-2021 Charles Korn <[email protected]>
License: Apache-2.0

Files: gradlew*
Copyright: 2007-2020 The original author or authors <[email protected]>
License: Apache-2.0
Expand Down
205 changes: 0 additions & 205 deletions batect
View file
Open in desktop

This file was deleted.

Loading