ENH/CI/TST: test with Olog docker containers

NSLS-II / olog

Python client to the Olog

BSD 3-Clause "New" or "Revised" License

2 stars 4 forks source link

ENH/CI/TST: test with Olog docker containers #5

Closed mrakitin closed 4 years ago

mrakitin commented 5 years ago

This PR is based on #4, but employs a more reproducible Olog service via Docker containers. See .travis.yml for the details how to start the containers. In short:

docker pull mrakitin/olog-mysql-db:latest
docker pull mrakitin/olog-server:latest
run https://github.com/mrakitin/docker-olog-server/blob/nsls2/run-all.sh

I had to trim the tests since there were no predefined records in the database. @shroffk may have an idea how to get it.

mrakitin commented 5 years ago

Recycle to trigger a Travis build.

danielballan commented 5 years ago

This is awesome.

I believe we should need to delete the VCR code, however. The path should be:

Delete the cassettes.
Run the tests against these docker containers (locally) to generate fresh files.
Commit the new cassettes and upload.

By keeping the VCR code, we enable potential contributors to make contributions without setting up the Docker images. The Dockers are super valuable for development but should only be needed when contributing a change that alters the bytes sent by the client during a given test.

danielballan commented 5 years ago

On second though, if the marginal value of VCR here seems low, given how easy it is to get the Dockers running, I would be OK with removing it to make the codebase simpler.

mrakitin commented 5 years ago

@danielballan, I agree with your second thought. It's especially convenient since we already have it running on Travis, so the non-*nix developers can use it as a test environment.

danielballan commented 5 years ago

The README should be updated to remove the distinction of running the tests with and without VCR, and the VCR dependency should be removed. After that, I'm happy to merge this and continue adding functionality/tests in follow-on PRs.

mrakitin commented 5 years ago

Sounds good, I'll update it soon.

tacaswell commented 5 years ago

see https://github.com/mrakitin/pyolog2/pull/1 For some reason github is not letting me push to @mrakitin 's branch from this PR...

danielballan commented 5 years ago

Thanks @tacaswell. I'll go ahead and merge manually.

danielballan commented 5 years ago

I notice that the CI passed on baac564 but failed on 4e3a077, which changed nothing of consequence. I also cannot get it to pass locally. I wonder if the docker images changed?

mrakitin commented 5 years ago

I was playing with the images to have a predefined olog database. It might have broken something. Working on the fix.

danielballan commented 5 years ago

This seems close! Would be nice to push it across the finish line once this deployment is behind us.

danielballan commented 4 years ago

Closing, presuming that this work is not relevant since we have pivoted to olog-es.