Cleanup of config/fixtures in services/web/server/tests

[pcrespov]

What do these changes do?

Refactoring services/web/server/tests fixtures and tests:

• Optimizes re-usability: conftest hierarchies and helper functions
• Simplified client-type of fixture
• Cleanup naming, doc and remove deprecated fixtures

Related issue number

None

How to test

  $ make devenv
  $ cd services/web/server
  $ make install 
  $ make tests

Checklist

• I think the code is well written
• Unit tests for the changes exist
• Documentation reflects the changes
• If you design a new module, add your user to .github/CODEOWNERS

[codecov-io]

Codecov Report

Merging #1158 into master will increase coverage by 2.89%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1158      +/-   ##
==========================================
+ Coverage    69.9%   72.79%   +2.89%     
==========================================
  Files         140      110      -30     
  Lines        6190     4418    -1772     
==========================================
- Hits         4327     3216    -1111     
+ Misses       1863     1202     -661

Impacted Files	Coverage Δ
...storage/src/simcore_service_storage/rest_config.py
...s/storage/src/simcore_service_storage/db_tokens.py
...orage/src/simcore_service_storage/config_schema.py
api/tests/conftest.py
...es/storage/src/simcore_service_storage/__init__.py
...ervices/storage/src/simcore_service_storage/cli.py
...ices/storage/src/simcore_service_storage/models.py
...es/storage/src/simcore_service_storage/settings.py
...es/storage/src/simcore_service_storage/__main__.py
api/tests/test_full_openapis.py
... and 21 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0513c82...2d95e87. Read the comment docs.

[sanderegg]

this looks great and much simpler!

[sanderegg]

I guess the client fixture you also have something in your pocket to put it in a common fixtures file?

[pcrespov]

I guess the client fixture you also have something in your pocket to put it in a common fixtures file?

I would like this PR to be kind of a cleanup. I can implement #1142 in a separate PR

[sanderegg]

very nice refactoring!

[ignapas]

Following the testing instructions I found some minor issues/confusing stuff:

• make devenv and make .env do exactly the same?
• if .venv already exists, then .venv and devenv targets don't run (normal make behavior). maybe asking if user wants it to be recreated would be useful?
• If node is already part of the swarm, then the testing fails. I had to manually leave the swarm and run the tests again
• When running the tests for the first time I got this error, and afterwards it went fine

================================================================================================ ERRORS ================================================================================================
_____________________________________________________________________________ ERROR at setup of test_check_health[pyloop] ______________________________________________________________________________

loop = <_UnixSelectorEventLoop running=False closed=False debug=False>, aiohttp_client = <function aiohttp_client.<locals>.go at 0x1132ad488>
app_config = {'application_proxy': {'enabled': True}, 'db': {'enabled': True, 'init_tables': True, 'postgres': {'database': 'simcor...login': {'enabled': True, 'registration_confirmation_required': False, 'registration_invitation_required': False}, ...}

    @pytest.fixture
    def client(loop, aiohttp_client,
            app_config,    ## waits until swarm with *_services are up
        ):
        assert app_config["rest"]["version"] == API_VERSION
        assert API_VERSION in app_config["rest"]["location"]
    
        app_config['storage']['enabled'] = False
        app_config["db"]["init_tables"] = True # inits postgres_service
    
        pprint(app_config)
    
        # fake config
        app = create_safe_application()
        app[APP_CONFIG_KEY] = app_config
    
        pprint(app_config)
    
        setup_db(app)
        setup_session(app)
        setup_security(app)
        setup_rest(app)
        setup_login(app)
        setup_projects(app)
        setup_computation(app)
    
        yield loop.run_until_complete(aiohttp_client(app, server_kwargs={
            'port': app_config["main"]["port"],
>           'host': app_config['main']['host']
        }))

computation/test_computation.py:83: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/usr/local/Cellar/python/3.6.5_1/Frameworks/Python.framework/Versions/3.6/lib/python3.6/asyncio/base_events.py:468: in run_until_complete
    return future.result()
../../../../../.venv/lib/python3.6/site-packages/aiohttp/pytest_plugin.py:335: in go
    await client.start_server()
../../../../../.venv/lib/python3.6/site-packages/aiohttp/test_utils.py:257: in start_server
    await self._server.start_server(loop=self._loop)
../../../../../.venv/lib/python3.6/site-packages/aiohttp/test_utils.py:105: in start_server
    await self.runner.setup()
../../../../../.venv/lib/python3.6/site-packages/aiohttp/web_runner.py:232: in setup
    self._server = await self._make_server()
../../../../../.venv/lib/python3.6/site-packages/aiohttp/web_runner.py:331: in _make_server
    await self._app.startup()
../../../../../.venv/lib/python3.6/site-packages/aiohttp/web_app.py:389: in startup
    await self.on_startup.send(self)
../../../../../.venv/lib/python3.6/site-packages/aiohttp/signals.py:34: in send
    await receiver(*args, **kwargs)  # type: ignore
../../src/simcore_service_webserver/computation_subscribe.py:40: in subscribe
    connection = await aio_pika.connect(rabbit_broker, connection_attempts=100)
../../../../../.venv/lib/python3.6/site-packages/aio_pika/connection.py:420: in connect
    yield from connection.connect()
../../../../../.venv/lib/python3.6/site-packages/aio_pika/connection.py:222: in connect
    result = yield from f
../../../../../.venv/lib/python3.6/site-packages/aio_pika/adapter.py:157: in _handle_disconnect
    super()._handle_disconnect()
../../../../../.venv/lib/python3.6/site-packages/pika/adapters/base_connection.py:288: in _handle_disconnect
    self._adapter_disconnect()
../../../../../.venv/lib/python3.6/site-packages/aio_pika/adapter.py:153: in _adapter_disconnect
    super()._adapter_disconnect()
../../../../../.venv/lib/python3.6/site-packages/pika/adapters/base_connection.py:154: in _adapter_disconnect
    self._check_state_on_disconnect()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <aio_pika.adapter.AsyncioConnection object at 0x1150be780>

    def _check_state_on_disconnect(self):
        """Checks to see if we were in opening a connection with RabbitMQ when
        we were disconnected and raises exceptions for the anticipated
        exception types.
    
        """
        if self.connection_state == self.CONNECTION_PROTOCOL:
            LOGGER.error('Incompatible Protocol Versions')
>           raise exceptions.IncompatibleProtocolError
E           pika.exceptions.IncompatibleProtocolError

../../../../../.venv/lib/python3.6/site-packages/pika/adapters/base_connection.py:169: IncompatibleProtocolError

[ignapas]

very nice, do you think we can work together on adding some tests for the activity manager? please 😄 @sanderegg or @pcrespov

[pcrespov]

• make devenv and make .env do exactly the same?

Do you mean .venv, right? because .env is a file while .venv is a directory.
devenv is an alias to build the real target .**v**env

• if .venv already exists, then .venv and devenv targets don't run (normal make behavior). maybe asking if user wants it to be recreated would be useful?

This is what Makefile does. The recipes do not run if the target is up-to-date. To recreate .venv just make clean devenv. Read about Makefile.

• If node is already part of the swarm, then the testing fails. I had to manually leave the swarm and run the tests again

Yes, I am aware of that. At the time I thought this is NOT responsibility of the test mainly because the test cannot recover the original state that the system has before starting. It is easy to live the swarm but cannot recover the state when the swarm was on ...
I still think this is minor

[pcrespov]

very nice, do you think we can work together on adding some tests for the activity manager? please @sanderegg or @pcrespov

this comes after this PR is in

[ignapas]

✅