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

Replace vagrant-based testing with docker-based testing #7

Merged
merged 22 commits into from
Feb 24, 2025

Conversation

RaghavaAlajangi
Copy link
Contributor

@RaghavaAlajangi RaghavaAlajangi commented Feb 19, 2025

This PR aims to replace vagrant-based testing with docker-based testing. Docker-based testing is useful because it is faster to execute, easier to manage dependencies, and better integrated with CI/CD.

To do:

  • Make changes in the CICD file
  • Add docker-run-test.sh and docker-compose.ci.yml files
  • Tests pass locally using docker
  • Update CHANGELOG
  • Passed CICD

@RaghavaAlajangi
Copy link
Contributor Author

Hi @paulmueller
please review my changes and let me know if they need any changes.

Copy link
Member

@paulmueller paulmueller left a comment

Choose a reason for hiding this comment

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

Congratulations, this is great!

The only thing that I would like to change is the environment file name.

.env.example Outdated
@@ -0,0 +1,75 @@
# Host Ports
Copy link
Member

Choose a reason for hiding this comment

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

I would propose to name this file to docker-env.ci and consequently use it in the entire workflow, i.e.

  • remove the "Create env file" step in check.yml
  • use that file in docker-compose.ci.yml's env_file key.

The reason is that .env is too generic which might lead to ambiguities.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I will change it.

@RaghavaAlajangi
Copy link
Contributor Author

Aha, this approach has failed. Maybe it is compulsory to have a .env extension!

@RaghavaAlajangi
Copy link
Contributor Author

RaghavaAlajangi commented Feb 20, 2025

Ok, I tested this file (docker-env.ci) manually. The variables are loaded when I run docker compose up but some of them are still set to empty strings. I don't know why.

@RaghavaAlajangi
Copy link
Contributor Author

RaghavaAlajangi commented Feb 20, 2025

We need both .env and env_file variables for the pipeline to be passed. It is explained here. We need .env file for docker-compose and env_file to send variables to the ckan container. The name of the file doesn't matter.

@paulmueller
Copy link
Member

You can specify the env file via docker command line: docker-compose --env-file .my-env up -d

From the thread you linked, I read that there is a difference between .env and env_file. Does it mean you have to split the docker configuration from the ckan/postgresql/etc configuration? In that case I would name them docker-compose.ci.env and docker-dcor.env.

@RaghavaAlajangi
Copy link
Contributor Author

RaghavaAlajangi commented Feb 21, 2025

If I understand correctly, you want me to split the docker-ci.env that I have into two files?

But, we don't have to split this file. We have to keep the Create env file step in check.yml. that's the solution because .env is used to set the environment variables for all the container used in compose file. Whereas, env_file is used to pass the variables into the dcor-ckan container. So, basically, we use docker-ci.env file to set both .env and env_file

@paulmueller
Copy link
Member

paulmueller commented Feb 21, 2025

If I understand correctly, you want me to split the docker-ci.env that I have into two files?

This is the question that I asked you 😸

But, we don't have to split this file. We have to keep the Create env file step in check.yml. that's the solution because .env is used to set the environment variables for all the container used in compose file. Whereas, env_file is used to pass the variables into the dcor-ckan container. So, basically, we use docker-ci.env file to set both .env and env_file

You could remove the "Create env file" step and instead specify the env file in docker compose (docker-compose --env-file docker-ci.env up ...), correct?

@RaghavaAlajangi
Copy link
Contributor Author

RaghavaAlajangi commented Feb 21, 2025

sorry for the confusion. Yes, we can set the global variables with docker-compose --env-file docker-ci.env up and remove the "Create env file" step. And, we can use the same file (docker-ci.env) to set the variables for dcor-ckan container as well. Is that ok, or do you still want me to create two different files?

@paulmueller
Copy link
Member

Sounds good to me 👍

@RaghavaAlajangi
Copy link
Contributor Author

Hi @paulmueller ,
I made the changes. Can you have a look?

@paulmueller paulmueller merged commit 3dfcd85 into master Feb 24, 2025
2 checks passed
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