Skip to content
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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Markus92
Copy link

@Markus92 Markus92 commented Aug 16, 2024

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:

  • Add Docker Compose configuration for running tests, simplifying the test setup process.

Documentation:

  • Update README.md to include instructions for running tests using Docker Compose.

Copy link

sourcery-ai bot commented Aug 16, 2024

Reviewer's Guide by Sourcery

This 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

Files Changes
docker-compose.yml Added a docker-compose.yml file to set up the testing environment with Solr, PostgreSQL, Redis, and a CKAN test container
ckanext/fairdatapoint/tests/run_tests.sh Added a shell script to initialize the database, run tests, and generate a coverage report within the Docker environment
README.md Updated the README.md file with new instructions for running tests using Docker Compose
README.md Removed empty lines and made minor formatting adjustments throughout the README

Tips
  • Trigger a new Sourcery review by commenting @sourcery-ai review on the pull request.
  • Continue your discussion with Sourcery by replying directly to review comments.
  • You can change your review settings at any time by accessing your dashboard:
    • Enable or disable the Sourcery-generated pull request summary or reviewer's guide;
    • Change the review language;
  • You can always contact us if you have any questions or feedback.

Copy link

@sourcery-ai sourcery-ai bot left a 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

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
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
Copy link

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`
Copy link

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.

Suggested change
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).

Comment on lines +89 to +91
# The minimum number of hours to wait before re-checking a resource
# (optional, default: 24).
ckanext.fairdatapoint.some_setting = some_default_value
Copy link

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
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants