search

okfn / docker-ckan

Docker images and Docker Compose setup for CKAN [Not Maintained]
GNU Affero General Public License v3.0
81 stars 91 forks source link

Test settings overridden by env vars #9

Closed gerbyzation closed 6 years ago

gerbyzation commented 6 years ago

When starting in development the paster tool sets the database url in test-core.ini to the urls provided in the test environment variables.

In my test-core.ini the sqlalchemy.url is set to postgres://ckan:ckan@db/ckan_test, but when checking the value in a test reads postgresql://ckan:ckan@db/ckan. This must come from the CKAN_SQLALCHEMY_URL which CKAN interprets itself as it's only provided in the .env file.

Besides modernising CKAN's config, I can only think of two solutions at the moment

  1. Ditch ckanext-envvars but keep the ENV vars with the same naming scheme (making sure they don't get picked up by CKAN itself) and write them to the config file with the config-tool.
  2. Write something that changes the ENV vars before and after tests a. as a wrapper script around the test command b. in setup/teardown methods (wouldn't work for CKAN core?)

Both seem sub-optimal to me, any thoughts?

amercader commented 6 years ago

Yeah, there's no elegant solution to this. What about adding TESTING=true env var, and if present start_ckan_development.sh would unset CKAN_SQLALCHEMY_URL? Or something along those lines.

gerbyzation commented 6 years ago

That would mean you can't run the dev server and tests in the same container though.

I've discovered env -i, which allows you to run a command with a clean environment (env -i env will print nothing). When running the tests with this it does use the test database, but raises on reset_db. Will look further into this later

gerbyzation commented 6 years ago

I've just been running the tests with a clean environment using env -i (something like env -i nosetests --nologcapture --with-pylons=test.ini), which works fine but is of course avoidance of the real problem. Any proper solution would have to happen in CKAN though I think, so I'll close this issue.