search

mrkschan / nginxbeat

Superseded by https://github.com/elastic/beats/tree/master/metricbeat
Apache License 2.0
58 stars 7 forks source link

Start integration test #39

Closed mrkschan closed 8 years ago

mrkschan commented 8 years ago

35

mrkschan commented 8 years ago

@ruflin Have a look :)

A few questions:

  1. Shall we separate unit-test and integration-test in make testsuite?
  2. Shall I "teardown" the Docker containers after the test instead of "setup" the containers by stopping them before the test? If we take "teardown" approach, would that affect the CI result?
ruflin commented 8 years ago
  1. Yes we should separate them in the future. With the current -short flag this was not possible.
  2. The reason we have it implemented like this because with the teardown thing we had issues several time. Theoretically I would agree the teardown option is the correct one and the clean one, but it fully depends on that in a previous build everything went as expected. In any case, it should not affect the CI results.
mrkschan commented 8 years ago

Do you mean sometimes the docker-compose stop would fail which results in false negative CI test result?

mrkschan commented 8 years ago

Indeed, the docker-compose stop here is to reset the Nginx stats. Shall I just add a frozen stats (e.g. /fake/status) for testing instead of using real Nginx stats?

ruflin commented 8 years ago

It could happen in the past, that a build started to hang so docker-compose stop was never called. Be aware that these are edge cases, but it is what lead to our current implementation.

I really like to use "real" nginx to test the stats as it makes it possible to also test different versions. Be aware that docker-compose stop does not remove the volumes, docker-compose rm is needed in addition to make sure you always start "clean".

mrkschan commented 8 years ago

Yea, I'm aware of that but Nginx keeps stats in memory so we don't have to remove the container. Asking Nginx to reload would also do the trick.

mrkschan commented 8 years ago

Perhaps, we could also use docker-compose up --force-recreate?

ruflin commented 8 years ago

Good to know about nginx. I was thinking in a more general way how to handle it. --force-recreate would work for nginx but not for services that store data on volumes: https://github.com/docker/compose/issues/2127