hackoregon / transportation-system-backend

2018 repo for the transportation api backend
MIT License
8 stars 2 forks source link

Testing Fixes for Unmanaged DB #41

Closed kiniadit closed 6 years ago

kiniadit commented 6 years ago

I'm submitting this PR since it appends to the previous PR and doesn't create any conflicts. However, if this is not the process we want, I can wait until the previous PR is approved and merged into staging.

Fixed the testing issue for the unmanaged model. Now tests can be created without issues.

bhgrant8 commented 6 years ago

I went ahead and merged the other commit so would not hang up progress, if we need to change configuration we can later.

so tests generally look good but one thing that is happening is that the test_db_1 container is hanging and not being stopped or removed. One idea I had was to add the docker commands to the test-entrypoint file to stop and remove the test_db container after the tests are run.

It seems to add a sleep cycle to the test startup waiting for the postgres container to startup, but leaves a clean environment once the test is complete:

while getopts ":lst" opt; do
    case "$opt" in
        l)
          docker-compose -p tests run --entrypoint /code/bin/test-entrypoint.sh  -p 8000 --rm api
          echo "Stopping test db container"
          docker stop tests_db_1
          echo "Removing test db container"
          docker rm tests_db_1
           ;;
        s)
...

Thoughts?

kiniadit commented 6 years ago

I retried the script and you are right. This behavior doesn't seem to affect the tests but your edit is probably a safer/better practice going ahead.

Not sure why the container closes during dev but not testing. Maybe it's got something to do with the keyboard interrupt ctrl-c to stop the server during development vs what Django does to end the test db.

On a separate note, though the test suite is ready now, I don't want to add tests until we have nailed down what the underlying database schema is going to be. This is what I understood will be happening during the previous meeting. No point creating tests for fields that will not be used right?

kiniadit commented 6 years ago

Go ahead and commit the change. It's a good fix for now.