Skip to content

build: add extra property for docker image #335

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

Closed
line-o wants to merge 1 commit into from
Closed

build: add extra property for docker image #335

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 1 addition & 1 deletion .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,4 @@ jobs:
- name: Test
env:
CYPRESS_RECORD_KEY: ${{ secrets.CYPRESS_RECORD_KEY }}
run: mvn verify -P e2e-tests-with-recording -Dexist.processor.version=${{ matrix.exist }}
run: mvn verify -P e2e-tests-with-recording -Dexist.docker.tag=${{ matrix.exist }}
3 changes: 2 additions & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@

<exist.java-api.version>5.4.0</exist.java-api.version> <!-- The eXist-db XQuery Java Module API version -->
<exist.processor.version>6.1.0</exist.processor.version> <!-- The version of eXist-db needed by this EXPath Module -->
<existdb.docker.tag>6.1.0</existdb.docker.tag> <!-- The tag of the eXist-db Docker image used to run integration tests on -->

<jackson.version>2.19.1</jackson.version>
<websocket.api.version>1.1</websocket.api.version>
Expand Down Expand Up @@ -360,7 +361,7 @@
<images>
<image>
<alias>existdb-docs-tests</alias>
<name>existdb/existdb:${exist.processor.version}</name>
<name>existdb/existdb:${existdb.docker.tag}</name>
Copy link
Copy Markdown
Contributor

@duncdrum duncdrum Oct 30, 2025

Choose a reason for hiding this comment

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

if we want to run tests on Mac against the minimum supported version I would still

Suggested change
<name>existdb/existdb:${existdb.docker.tag}</name>
<name>duncdrum/existdb:${existdb.docker.tag}</name>

Copy link
Copy Markdown
Member Author

@line-o line-o Nov 2, 2025

Choose a reason for hiding this comment

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

@duncdrum I would not want to change the image this is tested on. As I wrote this only affects the release process on an arm machine. On CI however the tests will still be executed against all versions specified in the test matrix. As we will likely move the release to CI I don't see the benefit of testing on another base image.

Copy link
Copy Markdown
Contributor

@duncdrum duncdrum Nov 2, 2025

Choose a reason for hiding this comment

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

@line-o it affects all local build and test runs on macOS. What's the advantage of using an image we know won't run on macOS with the official min version?

Copy link
Copy Markdown
Member Author

@line-o line-o Feb 17, 2026

Choose a reason for hiding this comment

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

@duncdrum
a) this PR does not change the image being used to test
b) the ability to run the integration tests agains 6.1.0 on an arm Mac is still possible by running it on bare metal instead

Copy link
Copy Markdown
Contributor

@duncdrum duncdrum Feb 17, 2026

Choose a reason for hiding this comment

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

What bare metal. There is no Mac compatible image existdb/existdb:6.1.0 and since docker based integration tests are included in the build. You end up with a broken clone. Bare metal, in the cloud, or docker within docker

Copy link
Copy Markdown
Member Author

@line-o line-o Feb 25, 2026

Choose a reason for hiding this comment

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

I think this discussion is off-topic as the PR only attempts to decouple the test-instance version from the minimum required processor version.

<run>
<ports>
<port>8080:8080</port>
Expand Down