-
Notifications
You must be signed in to change notification settings - Fork 1
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
ci: added docker compose config for running tests #75
base: main
Are you sure you want to change the base?
ci: added docker compose config for running tests #75
Conversation
Reviewer's Guide by SourceryThis pull request adds Docker Compose configuration for running tests, making it easier to set up and test the harvester extension. The changes include adding a Dockerfile and docker-compose.yml file, updating the README with new test instructions, and adding a shell script for running tests within the Docker environment. File-Level Changes
Tips
|
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.
Hey @Markus92 - I've reviewed your changes and they look great!
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟡 Security: 1 issue found
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟡 Documentation: 2 issues found
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
image: ckan/ckan-postgres-dev:2.11 | ||
environment: | ||
POSTGRES_USER: postgres | ||
POSTGRES_PASSWORD: postgres |
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.
🚨 suggestion (security): Consider using environment variables or Docker secrets for sensitive information
Hardcoding passwords in the Docker Compose file poses a security risk, especially if this file is committed to version control. Use environment variables or Docker secrets to manage sensitive information more securely.
POSTGRES_PASSWORD: ${POSTGRES_PASSWORD}
@@ -116,8 +112,7 @@ pip install -r https://raw.githubusercontent.com/ckan/ckanext-dcat/v1.5.1/requir | |||
|
|||
## Tests | |||
|
|||
To run the tests go to [GDI harvester test information](https://genomicdatainfrastructure.github.io/gdi-userportal-docs/docs/ckan/extension-local-setup-and-testing/) | |||
|
|||
To run the tests, run `docker compose build && docker-compose -f docker-compose.yml up --abort-on-container-exit --exit-code-from ckan-test && docker-compose rm -fsv` |
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.
suggestion (documentation): Updated test instructions with specific command
Consider keeping the link to the GDI harvester test information as well for more detailed guidance.
To run the tests, run `docker compose build && docker-compose -f docker-compose.yml up --abort-on-container-exit --exit-code-from ckan-test && docker-compose rm -fsv` | |
To run the tests: | |
1. Run the following command: | |
`docker compose build && docker-compose -f docker-compose.yml up --abort-on-container-exit --exit-code-from ckan-test && docker-compose rm -fsv` | |
2. For more detailed guidance on GDI harvester testing, refer to [GDI Harvester Test Information](link-to-gdi-harvester-test-info). |
# The minimum number of hours to wait before re-checking a resource | ||
# (optional, default: 24). | ||
ckanext.fairdatapoint.some_setting = some_default_value |
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.
question (documentation): Changed formatting of config settings comments
Is this change in indentation intentional? It might affect readability.
# | ||
# SPDX-License-Identifier: AGPL-3.0-only | ||
|
||
FROM ckan/ckan-dev:2.10 |
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.
important: can you please check these changes, they may conflict with dcat-ap 3 support and ckanext-dcat 2.0.
This PR adds a Dockerfile and docker-compose setup for running the test suite. It's based on the github actions workflow.
The standard setup is quite difficult to run - I never managed to install it successfully. So this way it should be easier to test the harvester extension.
Summary by Sourcery
Introduce Docker Compose setup for running tests, including a Dockerfile and configuration for services like Solr, Postgres, and Redis. Update documentation to guide users on using Docker Compose for testing.
CI:
Documentation: