Unable to create a test database when DATABASE_URL is present

[John-Hayes-Reed]

Description

When trying to create a _test database using a docker-compose service based project that is supplying a DATABASE_URL environment variable to the service, it does not create a test database, and shows an xxxxxxx_development database already exists message.

Steps to Reproduce

1. Create a new Amber project
2. Ensure the generated docker-compose.yml file is supplying an _development DATABASE_URL to the main app service, like:

services:
  app:
  build: .
  image: example_app
  command: amber watch
  environment:
    DATABASE_URL: postgres://admin:password@db:5432/xxxxxxxx_development
  ports:
    - 3000:3000
  links:
    - db
  volumes:
  - .:/app
  - nodes:/app/node_modules
  - shards:/app/lib

3. Enter into the container with the bash command: docker-compose run app bash
4. Run CLI command to create test database: AMBER_ENV=test amber db create

Expected behavior: [What you expect to happen]

A xxxxxxxx_test database to be created.

Actual behavior: [What actually happens]

A message appears with: xxxxxxxx_development database already exists and no test database is created.

Reproduces how often: [What percentage of the time does it reproduce?]

Every time so far for my one project, have not tried this after generating a second project though.

Versions

• Amber CLI (amberframework.org) - v0.32.0
• Crystal 0.33.0 [612825a53] (2020-02-14)
• Image: amberframework/amber:0.33.0

On MacOS 10.14.6, Running Docker Desktop 2.2.0.4 with Docker Engine 19.03.8 and Compose 1.25.4

[damianham]

IMHO this is correct behaviour as environment variables should always take precedence over static configuration. Setting the env variable AMBER_ENV=test will use the test database URL in the amber test environment config unless a DATABASE_URL environment variable specifies another database URL. I think this is the best option in that you can setup env variables to override static config. This is not a bug with Amber - this is mis-configuration of the environment in the docker-compose.yml file.

[John-Hayes-Reed]

I can see that and would tend to agree I think. In my case that env variable setup in the compose file is part of the auto generation of the new project, so perhaps to avoid people like myself coming to the framework and wanting to get learning / up and running with it with the least amount of hiccups, have the docker-compose.yml file generate without that environment variable set in it and passed to the service? Or have it in an override compose file for development instead of the main one?

[drujensen]

@damianham This is a bug since amber generated the docker-compose.yml file and its pointing to the development database instead of the test one when running specs.

There has been several debates if we should support the DATABASE_URL environment variable. I personally fall on the side of less magic, but I will leave that for others to argue.

[damianham]

what we should probably do is have an extra test service in the docker-compose file that sets the AMBER_ENV variable to test and the DATABASE_URL variable to the test db then in src/amber/cli/helpers/sentry.cr(Line 66) if AMBER_ENV == 'test' run the spec: section(task) of .amber.yml only rather than joining it with the run: section then

docker-compose up test

to run tests and it would be nice if it kept running and retested any changed files.

[ZimbiX]

I would recommend for Amber to support different environment variables for each piece of database configuration, e.g.:

DB_TYPE=postgres
DB_HOST=db
DB_USERNAME=postgres
DB_PASSWORD=password
DB_DATABASE=pet_tracker_development

And internally generating the connection URL, if required.

This way, when running in Docker Compose, we only need the host to be customised, and Amber's defaults for the others can be used, e.g. having just:

environment:
  DB_HOST: db

Leaving DB_DATABASE unset could allow the test database to be selected automatically when AMBER_ENV=test.

To avoid confusion around precedence of these environment variables vs the existing DATABASE_URL variable, an error could be shown if both are used. Or the DATABASE_URL could take precedence, but show a deprecation warning.