🐛 maintenance/flaky tests

.github/CODEOWNERS

@@ -4,33 +4,33 @@
 
 
 # files and folders recursively
-.codeclimate.yml                          @sanderegg, @pcrespov
+.codeclimate.yml                          @sanderegg @pcrespov
 .travis.yml                               @sanderegg
-Makefile                                  @pcrespov, @sanderegg
+Makefile                                  @pcrespov @sanderegg
 
 
 # NOTE: '/' denotes the root of the repository
-/api/                                     @sanderegg, @pcrespov
-/ci/                                      @sanderegg, @pcrespov
+/api/                                     @sanderegg @pcrespov
+/ci/                                      @sanderegg @pcrespov
 /docs/                                    @pcrespov
-/packages/models-library/                 @sanderegg, @pcrespov
-/packages/pytest-simcore/                 @pcrespov, @sanderegg
-/packages/service-integration/            @pcrespov, @sanderegg, @KZzizzle
+/packages/models-library/                 @sanderegg @pcrespov
+/packages/pytest-simcore/                 @pcrespov @sanderegg
+/packages/service-integration/            @pcrespov @sanderegg @KZzizzle
 /packages/service-library/                @pcrespov
-/packages/settings-library/               @pcrespov, @sanderegg
+/packages/settings-library/               @pcrespov @sanderegg
 /requirements/                            @pcrespov
-/scripts/demo/                            @odeimaiz, @pcrespov
+/scripts/demo/                            @odeimaiz @pcrespov
 /scripts/json-schema-to-openapi-schema    @sanderegg
-/scripts/template-projects/               @odeimaiz, @pcrespov
+/scripts/template-projects/               @odeimaiz @pcrespov
 /services/api-server/                     @pcrespov
-/services/catalog/                        @pcrespov, @sanderegg
-/services/director*/                      @sanderegg, @pcrespov
+/services/catalog/                        @pcrespov @sanderegg
+/services/director*/                      @sanderegg @pcrespov
 /services/dynamic-sidecar/                @GitHK
 /services/migration/                      @pcrespov
-/services/storage/                        @mguidon, @pcrespov
+/services/storage/                        @mguidon @pcrespov
 /services/static-webserver                @GitHK
-/services/web/client/                     @odeimaiz, @ignapas
-/services/web/server/                     @pcrespov, @sanderegg, @GitHK
+/services/web/client/                     @odeimaiz @ignapas
+/services/web/server/                     @pcrespov @sanderegg @GitHK
 /tests/environment-setup/                 @pcrespov
 /tests/performance/                       @pcrespov
 /tests/public-api/                        @pcrespov

.github/PULL_REQUEST_TEMPLATE.md

@@ -11,6 +11,9 @@ Consider prefix your PR message with an emoticon
   ⚗️ experimental
   ⬆️ upgrades dependencies
   📝 documentation
+  🗑️ deprecated
+  ⚰️ remove dead code
+  🔥 remove code or files
 or from https://gitmoji.dev/
 
 and append (⚠️ devops) if changes in devops configuration required before deploying
@@ -25,16 +28,15 @@ and append (⚠️ devops) if changes in devops configuration required before de
 
 <!-- Enumerate REVIEWERS other issues
 
-e.g.
-
+- ITISFoundation/osparc-issues#428
 - #26 : node_ports should have retry policies when upload/download fails  (FIXED)
-- ITISFoundation/osparc-issues#304: (Part 2) Prep2Go: creating features to support complex S4L scripts (IMPLEMENTED)
 
 -->
 
 
 ## How to test
 
+- Covered by CI
 <!-- Give REVIEWERS some hits or code snippets on how could this be tested -->
 
 

.pre-commit-config.yaml

@@ -13,10 +13,10 @@ repos:
       - id: check-merge-conflict
   # NOTE: Keep order as pycln (remove unused imports), then isort (sort them) and black (final formatting)
   - repo: https://github.com/hadialqattan/pycln
-    rev: v1.1.0
+    rev: v1.2.4
     hooks:
       - id: pycln
-        args: [--all]
+        args: [--expand-stars]
   - repo: https://github.com/PyCQA/isort
     rev: 5.6.4
     hooks:

packages/postgres-database/src/simcore_postgres_database/models/users.py

@@ -4,15 +4,24 @@
     - Users they have a role within the framework that provides
     them different access levels to it
 """
-import itertools
 from enum import Enum
+from functools import total_ordering
 
 import sqlalchemy as sa
 from sqlalchemy.sql import func
 
 from .base import metadata
 
+_USER_ROLE_TO_LEVEL = {
+    "ANONYMOUS": 0,
+    "GUEST": 10,
+    "USER": 20,
+    "TESTER": 30,
+    "ADMIN": 100,
+}
 
+
+@total_ordering
 class UserRole(Enum):
     """SORTED enumeration of user roles
 
@@ -36,11 +45,14 @@ class UserRole(Enum):
     TESTER = "TESTER"
     ADMIN = "ADMIN"
 
-    @classmethod
-    def super_users(cls):
-        return list(itertools.takewhile(lambda e: e != cls.USER, cls))
+    @property
+    def privilege_level(self) -> int:
+        return _USER_ROLE_TO_LEVEL[self.name]
 
-    # TODO: add comparison https://portingguide.readthedocs.io/en/latest/comparisons.html
+    def __lt__(self, other) -> bool:
+        if self.__class__ is other.__class__:
+            return self.privilege_level < other.privilege_level
+        return NotImplemented
 
 
 class UserStatus(Enum):

packages/postgres-database/tests/test_users.py

@@ -0,0 +1,50 @@
+from simcore_postgres_database.models.users import _USER_ROLE_TO_LEVEL, UserRole
+
+
+def test_user_role_to_level_map_in_sync():
+    # If fails, then update _USER_ROLE_TO_LEVEL map
+    assert set(_USER_ROLE_TO_LEVEL.keys()) == set(UserRole.__members__.keys())
+
+
+def test_user_role_comparison():
+
+    assert UserRole.ANONYMOUS < UserRole.ADMIN
+    assert UserRole.GUEST < UserRole.ADMIN
+    assert UserRole.USER < UserRole.ADMIN
+    assert UserRole.TESTER < UserRole.ADMIN
+    assert UserRole.ADMIN == UserRole.ADMIN
+
+    assert UserRole.ANONYMOUS < UserRole.TESTER
+    assert UserRole.GUEST < UserRole.TESTER
+    assert UserRole.USER < UserRole.TESTER
+    assert UserRole.TESTER == UserRole.TESTER
+    assert UserRole.ADMIN > UserRole.TESTER
+
+    assert UserRole.ANONYMOUS < UserRole.USER
+    assert UserRole.GUEST < UserRole.USER
+    assert UserRole.USER == UserRole.USER
+    assert UserRole.TESTER > UserRole.USER
+    assert UserRole.ADMIN > UserRole.USER
+
+    assert UserRole.ANONYMOUS < UserRole.GUEST
+    assert UserRole.GUEST == UserRole.GUEST
+    assert UserRole.USER > UserRole.GUEST
+    assert UserRole.TESTER > UserRole.GUEST
+    assert UserRole.ADMIN > UserRole.GUEST
+
+    assert UserRole.ANONYMOUS == UserRole.ANONYMOUS
+    assert UserRole.GUEST > UserRole.ANONYMOUS
+    assert UserRole.USER > UserRole.ANONYMOUS
+    assert UserRole.TESTER > UserRole.ANONYMOUS
+    assert UserRole.ADMIN > UserRole.ANONYMOUS
+
+    # < and >
+    assert UserRole.TESTER < UserRole.ADMIN
+    assert UserRole.ADMIN > UserRole.TESTER
+
+    # >=, == and <=
+    assert UserRole.TESTER <= UserRole.ADMIN
+    assert UserRole.ADMIN >= UserRole.TESTER
+
+    assert UserRole.ADMIN <= UserRole.ADMIN
+    assert UserRole.ADMIN == UserRole.ADMIN

packages/pytest-simcore/src/pytest_simcore/helpers/utils_login.py

@@ -1,5 +1,5 @@
 import re
-from typing import Any, Dict
+from typing import TypedDict
 
 from aiohttp import web
 from aiohttp.test_utils import TestClient
@@ -12,9 +12,22 @@
 
 from .utils_assert import assert_status
 
-# WARNING: UserDict is already in https://docs.python.org/3/library/collections.html#collections.UserDict
-# TODO: move this to future simcore_service_webserver.users_models.py
-AUserDict = Dict[str, Any]
+
+# WARNING: DO NOT use UserDict is already in https://docs.python.org/3/library/collections.html#collections.UserDictclass UserRowDict(TypedDict):
+# NOTE: this is modified dict version of packages/postgres-database/src/simcore_postgres_database/models/users.py for testing purposes
+class _UserInfoDictRequired(TypedDict, total=True):
+    id: int
+    name: str
+    email: str
+    raw_password: str
+    status: UserStatus
+    role: UserRole
+
+
+class UserInfoDict(_UserInfoDictRequired, total=False):
+    created_ip: int
+    password_hash: str
+
 
 TEST_MARKS = re.compile(r"TEST (\w+):(.*)")
 
@@ -37,7 +50,7 @@ def parse_link(text):
     return URL(link).path
 
 
-async def create_user(db: AsyncpgStorage, data=None) -> AUserDict:
+async def create_user(db: AsyncpgStorage, data=None) -> UserInfoDict:
     data = data or {}
     password = get_random_string(10)
     params = {
@@ -56,7 +69,7 @@ async def create_user(db: AsyncpgStorage, data=None) -> AUserDict:
 
 async def log_client_in(
     client: TestClient, user_data=None, *, enable_check=True
-) -> AUserDict:
+) -> UserInfoDict:
     # creates user directly in db
     db: AsyncpgStorage = get_plugin_storage(client.app)
     cfg: LoginOptions = get_plugin_options(client.app)
@@ -99,7 +112,7 @@ def __init__(self, client, params=None, *, check_if_succeeds=True):
         self.client = client
         self.enable_check = check_if_succeeds
 
-    async def __aenter__(self) -> AUserDict:
+    async def __aenter__(self) -> UserInfoDict:
         self.user = await log_client_in(
             self.client, self.params, enable_check=self.enable_check
         )

services/web/server/tests/unit/with_dbs/_helpers.py → packages/pytest-simcore/src/pytest_simcore/helpers/utils_webserver_projects.py

@@ -34,6 +34,7 @@ class ExpectedResponse(NamedTuple):
     ]
 
     def __str__(self) -> str:
+        # pylint: disable=no-member
         items = ",".join(f"{k}={v.__name__}" for k, v in self._asdict().items())
         return f"{self.__class__.__name__}({items})"
 

packages/pytest-simcore/src/pytest_simcore/services_api_mocks_for_aiohttp_clients.py

@@ -8,10 +8,10 @@
 
 import pytest
 from aiohttp import web
-from aioresponses import aioresponses as AioResponsesMock
 from aioresponses.core import CallbackResult
 from models_library.api_schemas_storage import FileMetaData
 from models_library.projects_state import RunningState
+from pytest_simcore.aioresponses_mocker import AioResponsesMock
 from yarl import URL
 
 pytest_plugins = [
@@ -130,7 +130,7 @@ def get_computation_cb(url, **kwargs) -> CallbackResult:
 
 
 @pytest.fixture
-async def director_v2_service_mock(
+async def director_v2_service_responses_mock(
     aioresponses_mocker: AioResponsesMock,
 ) -> AioResponsesMock:
     """mocks responses of director-v2"""

packages/service-library/src/servicelib/utils.py

@@ -61,17 +61,17 @@ def search_osparc_repo_dir(start: Union[str, Path], max_iterations=8) -> Optiona
 
 
 # FUTURES
+def log_exception_callback(fut: asyncio.Future):
+    try:
+        fut.result()
+    except Exception:  # pylint: disable=broad-except
+        logger.exception("Error occured while running task!")
+
+
 def fire_and_forget_task(
     obj: Union[Coroutine, asyncio.Future, Awaitable]
 ) -> asyncio.Future:
     future = asyncio.ensure_future(obj)
-
-    def log_exception_callback(fut: asyncio.Future):
-        try:
-            fut.result()
-        except Exception:  # pylint: disable=broad-except
-            logger.exception("Error occured while running task!")
-
     future.add_done_callback(log_exception_callback)
     return future
 

packages/settings-library/src/settings_library/email.py

@@ -1,6 +1,6 @@
 from typing import Optional
-from pydantic import root_validator
 
+from pydantic import root_validator
 from pydantic.fields import Field
 from pydantic.types import SecretStr
 
@@ -11,14 +11,15 @@
 class SMTPSettings(BaseCustomSettings):
     """Simple Mail Transfer Protocol"""
 
+    # TODO: NameEmail or EmailStr
     SMTP_SENDER: str = "@".join(["O2SPARC support <support", "osparc.io>"])
 
     SMTP_HOST: str
     SMTP_PORT: PortInt
 
     SMTP_TLS_ENABLED: bool = Field(False, description="Enables Secure Mode")
-    SMTP_USERNAME: Optional[str]
-    SMTP_PASSWORD: Optional[SecretStr]
+    SMTP_USERNAME: Optional[str] = Field(None, min_length=1)
+    SMTP_PASSWORD: Optional[SecretStr] = Field(None, min_length=1)
 
     @root_validator
     @classmethod

packages/settings-library/tests/test_email.py

@@ -1,7 +1,7 @@
 from typing import Any, Dict
-from pydantic import ValidationError
 
 import pytest
+from pydantic import ValidationError
 from settings_library.email import SMTPSettings
 
 
@@ -53,6 +53,13 @@ def test_smtp_configuration_ok(cfg: Dict[str, Any]):
             "SMTP_TLS_ENABLED": True,
             "SMTP_USERNAME": "test",
         },
+        {
+            "SMTP_HOST": "test",
+            "SMTP_PORT": 113,
+            "SMTP_USERNAME": "",
+            "SMTP_PASSWORD": "test",
+            "SMTP_TLS_ENABLED": True,
+        },
     ],
 )
 def test_smtp_configuration_fails(cfg: Dict[str, Any]):