Code review comment for ~peter-sabaini/charm-mongodb:upgrade-functests

Thanks Paul.

On 15.04.20 23:12, Paul Goins wrote:
> Review: Approve
>
> I haven't actually tested this myself, but it looks good. It doesn't look like this provides parity with the Amulet suite (correct me if I'm wrong), but it looks like it does port us over to Python 3 and provides a start of a new Zaza-based test suite.
>
> I'm +1 for this, but would like one more pair of eyes given the size of this.
>
> Diff comments:
>
>> diff --git a/Makefile b/Makefile
>> index c23418b..68db95f 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -31,14 +32,14 @@ lint: .venv
>> .venv/bin/flake8 --exclude hooks/charmhelpers actions $(ACTIONS) hooks tests unit_tests
>> .venv/bin/charm-proof
>>
>> -test: .venv
>> +unit:
>> @echo Starting unit tests...
>> - .venv/bin/nosetests -s --nologcapture --with-coverage $(EXTRA) unit_tests/
>> - .venv/bin/nosetests -s --nologcapture --with-coverage $(EXTRA) actions/
>> + @tox -e unit
>>
>> -functional_test:
>> - @echo Starting amulet tests...
>> - @juju test -v -p AMULET_HTTP_PROXY --timeout 900
>> +functional:
>> + @echo Starting functional tests...
>> + rm -rf .venv
>
> .venv seems tangential to the functional target at this point because of moving to tox. We can remove it, but I'm not sure it's necessary. (Feel free to ignore.)

I've added it as the Zaza deploy uploads the charm incl. the .venv, and balks at the symlink to /usr/bin/python
I will add a comment to that effect

>> + @tox -e functional
>>
>> sync:
>> @mkdir -p bin
>> diff --git a/tox.ini b/tox.ini
>> new file mode 100644
>> index 0000000..013abf0
>> --- /dev/null
>> +++ b/tox.ini
>> @@ -0,0 +1,34 @@
>> +[tox]
>> +skipsdist=True
>> +envlist = unit, functional, lint
>> +skip_missing_interpreters = True
>> +
>> +[testenv]
>> +setenv =
>> + PYTHONPATH = .
>> +passenv =
>> + HOME
>> + JUJU_REPOSITORY
>> + MODEL_SETTINGS
>> +
>> +[testenv:unit]
>> +basepython = python2
>> +commands =
>> + nosetests -s --nologcapture --with-coverage unit_tests/ actions/
>> +deps = -r{toxinidir}/test_requirements.txt
>> +
>> +[testenv:functional]
>> +basepython = python3
>> +commands =
>> + functest-run-suite --keep-model
>> +deps =
>> + git+https://github.com/openstack-charmers/zaza.git#egg=zaza
>> + pymongo
>> +
>> +[testenv:func-smoke]
>> +basepython = python3
>> +commands =
>> + functest-run-suite --keep-model --smoke
>> +deps =
>> + git+https://github.com/openstack-charmers/zaza.git#egg=zaza
>> + pymongo
>
> Since we have this list of dependencies in two different places, I feel like it may be better to put it in a tests/test_requirements.txt, in case it needs to be modified later. Seems this may be easy for someone to update one copy and not the other by mistake.
Fair point, will do
>
>
>