-
Notifications
You must be signed in to change notification settings - Fork 542
feat(.github/workflows): add container testing workflow #12352
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
Changes from all commits
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,265 @@ | ||||||||
| name: Container Integration Tests Workflow | ||||||||
|
|
||||||||
| on: | ||||||||
| push: | ||||||||
| branches: | ||||||||
| - develop | ||||||||
| paths-ignore: | ||||||||
| - "doc/**" | ||||||||
| - "**/*.md" | ||||||||
| - "**/*.txt" | ||||||||
| - ".github/ISSUE_TEMPLATE/**" | ||||||||
| - ".github/*.md" | ||||||||
| pull_request: | ||||||||
| branches: | ||||||||
| - develop | ||||||||
| paths-ignore: | ||||||||
| - "doc/**" | ||||||||
| - "**/*.md" | ||||||||
| - "**/*.txt" | ||||||||
| - ".github/ISSUE_TEMPLATE/**" | ||||||||
| - ".github/*.md" | ||||||||
|
|
||||||||
| concurrency: | ||||||||
| group: ${{ github.workflow }}-${{ github.ref }} | ||||||||
| cancel-in-progress: true | ||||||||
|
|
||||||||
| jobs: | ||||||||
| main-integration-tests-workflow: | ||||||||
| runs-on: ubuntu-latest | ||||||||
| if: vars.ENABLE_DOCKER_TESTS == 'true' | ||||||||
|
||||||||
| if: vars.ENABLE_DOCKER_TESTS == 'true' | |
| if: ${{ vars.ENABLE_DOCKER_TESTS == 'true' && github.repository_owner == 'IQSS' }} |
Copilot
AI
Apr 22, 2026
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.
The docker exec ... curl calls don't use --fail/--fail-with-body (and don't check HTTP status), so a 4xx/5xx response can still return exit code 0 and silently leave the instance misconfigured, causing harder-to-diagnose test failures later. Consider adding --fail-with-body (or --fail) and logging/validating the response for each setting update.
| docker exec dev_dataverse curl -s -X PUT -d "${settings[$key]}" "http://localhost:8080/api/admin/settings/$key" | |
| response="$(docker exec dev_dataverse curl --silent --show-error --fail-with-body -X PUT -d "${settings[$key]}" "http://localhost:8080/api/admin/settings/$key")" | |
| echo "$response" |
Copilot
AI
Apr 22, 2026
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.
This step downloads a test fixture from raw.githubusercontent.com without pinning to a commit SHA and without curl --fail, which hurts CI reproducibility and can silently succeed on HTTP errors (e.g., 404 HTML saved as JSON). Prefer using a fixture checked into the repo, or pin to a specific commit + add -fL/--fail-with-body so failures are explicit.
| docker exec dev_dataverse sh -c 'curl https://raw.githubusercontent.com/IQSS/dataverse/develop/src/test/java/edu/harvard/iq/dataverse/makedatacount/sushi_sample_logs.json > /tmp/sushi_sample_logs.json && head /tmp/sushi_sample_logs.json' | |
| docker cp src/test/java/edu/harvard/iq/dataverse/makedatacount/sushi_sample_logs.json dev_dataverse:/tmp/sushi_sample_logs.json | |
| docker exec dev_dataverse sh -c 'head /tmp/sushi_sample_logs.json' |
Copilot
AI
Apr 22, 2026
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.
The Maven invocation mixes lifecycle phases (verify and later package) in the same command. Because verify already runs all prior phases including package, this can cause redundant work (and in some setups can re-run parts of the build). Consider dropping the trailing package and only invoking verify (with the desired profile and system properties).
| -P all-unit-tests package | |
| -P all-unit-tests |
Copilot
AI
Apr 22, 2026
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.
This workflow uses actions/upload-artifact@v4 while the rest of the repo has standardized on newer upload-artifact versions (e.g. .github/workflows/maven_unit_test.yml:65 uses @v7). Consider aligning to the same version to avoid inconsistencies and potential deprecation issues.
| uses: actions/upload-artifact@v4 | |
| uses: actions/upload-artifact@v7 |
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.
paths-ignoreexcludesdoc/**, but later this workflow reads and editsdoc/sphinx-guides/source/_static/util/createsequence.sql. As a result, PRs that only change that SQL file (or nearby fixtures underdoc/) will not trigger this workflow, even though the runtime behavior depends on it. Consider removingdoc/**from the ignore list or switching to apaths:allowlist that includes this SQL file.