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

tests: change Travis to use Docker #133

Closed

Conversation

ntarocco
Copy link
Contributor

No description provided.

@ntarocco
Copy link
Contributor Author

ntarocco commented Sep 27, 2020

@ppanero I was thinking to use docker-services-cli, but I am not sure it was easy to test multiple versions... Maybe we can add it... anyway I have copied the docker-services.yml from there.
I think that in docker-services-cli we are missing the possibility of setting the version of MySQL: it looks like that invenio-db is not happy with MySQL 8.

@ntarocco
Copy link
Contributor Author

@lnielsen to use Postgres 12, we need to upgrade SQLAlchemy min version from 1.1 to 1.2.18

Copy link
Member

@ppanero ppanero left a comment

Choose a reason for hiding this comment

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

Okay, I made some comments and then I realized actually the problem was that docker-compose-cli was not easy to integrate. So most likely you can ignore my comments 😅 . Could we have a chat IRL so I fully understand the issues?

If there is no rush to integrate this I would like to try to integrate docker-services-cli and potentially fix the problems. Maybe is just a matter of improving the documentation (sorry about that).

Update: Made a PR with some required changes in the cli part inveniosoftware/docker-services-cli#10.

before_script:
- "docker --version"
- "docker-compose --version"
- if [ -n "${DOCKER_SERVICE}" ]; then
Copy link
Member

Choose a reason for hiding this comment

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

This should not be needed: Which case triggers this if? I am not able to understand it.


script:
- "./run-tests.sh"

after_script:
- docker-compose -f docker-services.yml down
Copy link
Member

Choose a reason for hiding this comment

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

Should not be needed either. Should use docker-services-cli down

- EXTRAS=mysql REQUIREMENTS=lowest SQLALCHEMY_DATABASE_URI="mysql+pymysql://travis@localhost:3306/invenio"
- EXTRAS=postgresql REQUIREMENTS=lowest SQLALCHEMY_DATABASE_URI="postgresql+psycopg2://postgres@localhost:5432/invenio" POSTGRESQL_VERSION="9.6"
- EXTRAS=postgresql REQUIREMENTS=lowest SQLALCHEMY_DATABASE_URI="postgresql+psycopg2://postgres@localhost:5432/invenio" POSTGRESQL_VERSION="10"
- EXTRAS=mysql REQUIREMENTS=lowest DOCKER_SERVICE=mysql SQLALCHEMY_DATABASE_URI="mysql+pymysql://testuser:testpsw@localhost:3306/invenio" MYSQL_VERSION="5.7"
Copy link
Member

Choose a reason for hiding this comment

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

We should be able to avoid all the DOCKER_SERVICE and URIs.

@@ -0,0 +1,28 @@
# -*- coding: utf-8 -*-
Copy link
Member

Choose a reason for hiding this comment

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

This file should not be needed.

@lnielsen
Copy link
Member

@lnielsen to use Postgres 12, we need to upgrade SQLAlchemy min version from 1.1 to 1.2.18

Ok from my side.

@ntarocco
Copy link
Contributor Author

closed in favor of: #134

@ntarocco ntarocco closed this Sep 28, 2020
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.

None yet

3 participants