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

Run commons checks and tests on Travis #3189

Merged
merged 3 commits into from
Oct 27, 2017
Merged

Run commons checks and tests on Travis #3189

merged 3 commits into from
Oct 27, 2017

Conversation

arnaud-morvan
Copy link
Member

No description provided.

.travis.yml Outdated
- psql -d c2cgeoportal_tests -c "CREATE EXTENSION postgis;" -U postgres
script:
- cd commons
- make check test
Copy link
Member

Choose a reason for hiding this comment

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

This is a good first point but is should use the same test infra as for geoportal, is it OK for you to merge like it and I will uniformise this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, I'd like to merge this ASAP.

Copy link
Member Author

Choose a reason for hiding this comment

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

Note we are not yet ready for docker and apache.

@@ -1,6 +1,12 @@
"""c2cgeoportal_commons package."""

from c2cgeoportal_commons.models import get_session_factory, get_engine, get_tm_session, generate_mappers, init_dbsessions
from c2cgeoportal_commons.models import ( # noqa
Copy link
Member

Choose a reason for hiding this comment

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

Why noqa?

Copy link
Member Author

@arnaud-morvan arnaud-morvan Oct 24, 2017

Choose a reason for hiding this comment

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

Why do you add this here ? it is unused, so it do not pass checks without noqa

Copy link
Member Author

Choose a reason for hiding this comment

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

I've supposed you add this imports here for use in c2cgeoportal, as I do not want to break c2cgeoportal, I've prefered to put noqa than to break something in c2cgeoportal.

Copy link
Member

Choose a reason for hiding this comment

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

No, they should be removed :-)

@arnaud-morvan arnaud-morvan force-pushed the common_travis branch 2 times, most recently from 1eb141e to 800b99f Compare October 24, 2017 12:06
@@ -14,7 +14,7 @@ make install
```
sudo -u postgres psql -c "CREATE USER \"www-data\" WITH PASSWORD 'www-data';"

DATABASE=c2cgeoportal_tests
DATABASE=c2cgeoportal_test
Copy link
Member

Choose a reason for hiding this comment

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

Just one test?

Copy link
Member Author

Choose a reason for hiding this comment

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

To be consistent with admin part, is this really relevant ?

Copy link
Member

Choose a reason for hiding this comment

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

no :-)

@arnaud-morvan arnaud-morvan force-pushed the common_travis branch 2 times, most recently from fed55a7 to 63a6e7c Compare October 25, 2017 14:41
@@ -24,7 +24,7 @@ flake8: ${BUILD_DIR}/requirements-dev.timestamp

${BUILD_DIR}/venv.timestamp:
#Create a Python virtual environment.
python3 -m venv ${VENV}
virtualenv -p python3 ${VENV}
Copy link
Member

Choose a reason for hiding this comment

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

Why this change?

@arnaud-morvan
Copy link
Member Author

Because the old version works locally but not on travis, I do not know exactly why.

@sbrunner sbrunner added this to the 2.3 milestone Nov 14, 2017
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