From 0fafa2227237f0f278752a4fad39c5b99692487d Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 15 Dec 2023 17:57:48 +0100 Subject: [PATCH] cleanup (#18) - remove demo runner - improve docs - rename all tests to test_* (previously, also setup_* and cleanup_* existed) to improve stability as it is not guaranteed that pytest.ini is loaded. - improve logging formatting - improve full integration test Reviewed-on: https://git.local-it.org/local-it-infrastructure/e2e_tests/pulls/18 Co-authored-by: Daniel Co-committed-by: Daniel --- docs/documentation.md | 1 + pyproject.toml | 1 - pytest_abra/cli.py | 8 +++-- pytest_abra/custom_fixtures.py | 2 +- pytest_abra/runner.py | 1 + .../tests_authentik/cleanup_authentik.py | 2 +- .../tests_authentik/setup_authentik.py | 7 +++-- recipes/demo/tests_demo/__init__.py | 0 recipes/demo/tests_demo/fixtures_demo.py | 26 ---------------- recipes/demo/tests_demo/runner_demo.py | 24 -------------- recipes/demo/tests_demo/setup_demo.py | 3 -- .../tests_nextcloud/runner_nextcloud.py | 2 +- .../tests_nextcloud/setup_nextcloud.py | 2 +- .../tests_wordpress/setup_wordpress.py | 2 +- .../setup_wordpress_trigger_email.py | 2 +- tests/test_cli_full_integration.py | 31 +++++++++++++------ tests/test_coordinator.py | 12 ++++++- tests/test_dir_manager.py | 13 ++++---- 18 files changed, 58 insertions(+), 81 deletions(-) delete mode 100644 recipes/demo/tests_demo/__init__.py delete mode 100644 recipes/demo/tests_demo/fixtures_demo.py delete mode 100644 recipes/demo/tests_demo/runner_demo.py delete mode 100644 recipes/demo/tests_demo/setup_demo.py diff --git a/docs/documentation.md b/docs/documentation.md index b84c7da..421dcb2 100644 --- a/docs/documentation.md +++ b/docs/documentation.md @@ -222,6 +222,7 @@ To understand how a test suite is built, let's have a look at the files runner_authentik.py -> required, defines the Runner subclass (see below) conftest.py -> not required. special file for pytest. is automatically discovered and loaded. convenient place to define fixtures and functions to be used in more than one test routine setup_authentik.py -> not required. can hold setup routine for authentik, has to be registered in runner_authentik.py +fixtures_authentik.py -> not required. holds fixtures that are meant to be imported by other test modules that depend on authentik. # Create a custom Runner diff --git a/pyproject.toml b/pyproject.toml index 6708c04..0a2ed00 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -59,7 +59,6 @@ line-length = 120 target-version = "py311" [tool.pytest.ini_options] -python_functions = "setup_* test_* cleanup_*" norecursedirs = ".* previous-work recipes" testpaths = "tests" markers = [ diff --git a/pytest_abra/cli.py b/pytest_abra/cli.py index 680ecd1..ab4d791 100644 --- a/pytest_abra/cli.py +++ b/pytest_abra/cli.py @@ -1,5 +1,6 @@ import argparse import os +import sys from pathlib import Path from loguru import logger @@ -19,10 +20,11 @@ def run(): parser.add_argument("--env_paths", type=str, help="List of loaded env files separated with ;", required=True) parser.add_argument("--recipes_dir", type=Path, help="Dir of abra recipes and respective runners", required=True) parser.add_argument("--output_dir", type=Path, help="Dir of test outputs", required=True) - parser.add_argument("--timeout", type=int, help="Set Playwright timeout in ms", default=20_000) + parser.add_argument("--timeout", type=int, help="Set Playwright timeout in ms", default=30_000) parser.add_argument("--debug", action="store_true", help="Enable Playwright debug mode") parser.add_argument("--resume", action="store_true", help="Re-run the most recent test, skipping passed tests") parser.add_argument("--session_id", help="Session dir name (inside output_dir). Overwrites --resume") + parser.add_argument("--cleanup", help="Force test cleanup. Should not be necessary") args = parser.parse_args() env_paths = [Path(s) for s in args.env_paths.split(";")] @@ -41,7 +43,9 @@ def run(): # todo: move to Coordinator DIR = DirManager(output_dir=args.output_dir, session_id=session_id) log_file = DIR.RESULTS / "coordinator.log" - logger.add(log_file) + logger.remove() + logger.add(log_file, format="{time:YYYY-MM-DD at HH:mm:ss} | {level} | {message}") + logger.add(sys.stdout, colorize=True, format="{time:YYYY-MM-DD HH:mm:ss} {message}") # ---------------------------- initialize and run ---------------------------- # diff --git a/pytest_abra/custom_fixtures.py b/pytest_abra/custom_fixtures.py index 0f0a5f0..4c07127 100644 --- a/pytest_abra/custom_fixtures.py +++ b/pytest_abra/custom_fixtures.py @@ -22,7 +22,7 @@ def pytest_addoption(parser: Parser): parser.addoption("--runner_index", action="store", type=int) parser.addoption("--output_dir", action="store", type=Path) parser.addoption("--session_id", action="store", type=str) - parser.addoption("--timeout", action="store", type=int, default=20_000) + parser.addoption("--timeout", action="store", type=int, default=30_000) @pytest.fixture(autouse=True) diff --git a/pytest_abra/runner.py b/pytest_abra/runner.py index c9eb650..6676560 100644 --- a/pytest_abra/runner.py +++ b/pytest_abra/runner.py @@ -149,6 +149,7 @@ class Runner: # command_arguments.append("--traceconfig") command_arguments.append("-v") + command_arguments.append(str(full_test_path)) command_arguments.append("--runner_index") diff --git a/recipes/authentik/tests_authentik/cleanup_authentik.py b/recipes/authentik/tests_authentik/cleanup_authentik.py index b556ea4..3f65dfd 100644 --- a/recipes/authentik/tests_authentik/cleanup_authentik.py +++ b/recipes/authentik/tests_authentik/cleanup_authentik.py @@ -27,7 +27,7 @@ def remove_user(admin_context: BrowserContext, URL: BaseUrl): page.get_by_role("dialog").get_by_role("button", name=re.compile(r"Löschen|Delete")).click() -def cleanup_delete_user( +def test_cleanup_delete_user( context: BrowserContext, env_config: dict[str, str], DIR: DirManager, URL: BaseUrl, check_if_user_exists ): # load admin cookies to context diff --git a/recipes/authentik/tests_authentik/setup_authentik.py b/recipes/authentik/tests_authentik/setup_authentik.py index b66b41d..9dd6aee 100644 --- a/recipes/authentik/tests_authentik/setup_authentik.py +++ b/recipes/authentik/tests_authentik/setup_authentik.py @@ -12,7 +12,7 @@ TEST_USER = os.environ["TEST_USER"] TEST_PASS = os.environ["TEST_PASS"] -def setup_admin_state(context: BrowserContext, env_config: dict[str, str], DIR: DirManager, URL: BaseUrl): +def test_setup_admin_state(context: BrowserContext, env_config: dict[str, str], DIR: DirManager, URL: BaseUrl): # go to page page = context.new_page() page.goto(URL.get()) @@ -50,7 +50,8 @@ def create_invite_link(admin_context: BrowserContext, env_config: dict[str, str] page.locator('input[name="name"]').click() linkname = "test_link_123" page.locator('input[name="name"]').fill(linkname) - page.get_by_placeholder("Wählen Sie ein Objekt aus.").click() + placeholder_pattern = re.compile(r"Wählen Sie ein|Select an") + page.get_by_placeholder(placeholder_pattern).click() page.get_by_role("option", name=re.compile(r"invitation-enrollment-flow")).click() # force, because else we get "intercepts pointer events" @@ -82,7 +83,7 @@ def create_user(user_context: BrowserContext, invitelink): expect(page.locator("ak-library")).to_be_visible() -def setup_user_state( +def test_setup_user_state( context: BrowserContext, env_config: dict[str, str], DIR: DirManager, URL: BaseUrl, check_if_user_exists ): # load admin cookies to context diff --git a/recipes/demo/tests_demo/__init__.py b/recipes/demo/tests_demo/__init__.py deleted file mode 100644 index e69de29..0000000 diff --git a/recipes/demo/tests_demo/fixtures_demo.py b/recipes/demo/tests_demo/fixtures_demo.py deleted file mode 100644 index c650202..0000000 --- a/recipes/demo/tests_demo/fixtures_demo.py +++ /dev/null @@ -1,26 +0,0 @@ -""" -This file can be used to define fixtures thate are then used by other tests which -depend on [demo]. For this to work - -1. the Runner class of the other test needs to define the depencency as seen - by referencing RunnerDemo in the dependencies list: - -from pytest_abra.tests_demo.runner_demo import RunnerDemo - -class RunnerOther(Runner): - dependencies = [RunnerDemo] - - -2. the specific tests that rely on these fixtures need to import the fixtures. - To globally import for all tests in 'other', the import should be done in conftest: - -in 'conftest.py' in 'test_other' dir: -from pytest_abra.tests_demo.fixtures_demo import demo_fixture -""" - -import pytest - - -@pytest.fixture -def demo_fixture(): - return "" diff --git a/recipes/demo/tests_demo/runner_demo.py b/recipes/demo/tests_demo/runner_demo.py deleted file mode 100644 index e735e5f..0000000 --- a/recipes/demo/tests_demo/runner_demo.py +++ /dev/null @@ -1,24 +0,0 @@ -from pytest_abra.runner import Runner, Test - - -class RunnerDemo(Runner): - """Every env file has a corresponding runner class""" - - env_type = "demo" # name of the test, used for logging / output naming - - # this indicates that tests from RunnerDemo depend on the setup from RunnerAuthentik. - # RunnerDemo will only execute, when setup_authentik.py has finished successfully. - # For example, setup_authentik.py generates session states, that can be used as fixtures - # that can be loaded from fixtures_authentik.py - dependencies: list[str] = ["authentik"] - - # todo: update these comments - # Filename of Demo setup. If defined, it will run 1st by executing pytest - # Filename of Demo test. This file contains unconditional tests that will be run in any - # case. If defined, it will run 2nd by executing pytest - # this list can hold many more tests from RunnerDemo that run conditional. The condition - # and the test file can be defined by creating a ConditionalTest instance: - # ConditionalTest(condition: Callable, test_file: str) - setups: list[Test] = [] - tests: list[Test] = [] - cleanups: list[Test] = [] diff --git a/recipes/demo/tests_demo/setup_demo.py b/recipes/demo/tests_demo/setup_demo.py deleted file mode 100644 index d46a9d6..0000000 --- a/recipes/demo/tests_demo/setup_demo.py +++ /dev/null @@ -1,3 +0,0 @@ -# Define functions here that are specifically meant for setup, not for testing. This means -# all actions that simply are required for other tests from 'demo' to run. Runs before all -# tests from 'demo'. diff --git a/recipes/nextcloud/tests_nextcloud/runner_nextcloud.py b/recipes/nextcloud/tests_nextcloud/runner_nextcloud.py index 0cb466c..fe56587 100644 --- a/recipes/nextcloud/tests_nextcloud/runner_nextcloud.py +++ b/recipes/nextcloud/tests_nextcloud/runner_nextcloud.py @@ -6,7 +6,7 @@ class RunnerNextcloud(Runner): dependencies = ["authentik"] setups = [Test(test_file="setup_nextcloud.py", prevent_skip=False)] tests = [ - Test(test_file="tests_nextcloud.py", prevent_skip=True), + Test(test_file="tests_nextcloud.py"), # Test(condition=condition_always_false, test_file="tests_nextcloud_onlyoffice.py"), ] # cleanups = [Test(test_file="cleanup_nextcloud.py")] diff --git a/recipes/nextcloud/tests_nextcloud/setup_nextcloud.py b/recipes/nextcloud/tests_nextcloud/setup_nextcloud.py index 3f18b2c..378c1db 100644 --- a/recipes/nextcloud/tests_nextcloud/setup_nextcloud.py +++ b/recipes/nextcloud/tests_nextcloud/setup_nextcloud.py @@ -10,7 +10,7 @@ from pytest_abra import BaseUrl, DirManager # https://files.test.dev.local-it.cloud/apps/files/ -def setup_nextcloud_admin_session(authentik_admin_page: Page, DIR: DirManager, URL: BaseUrl): +def test_setup_nextcloud_admin_session(authentik_admin_page: Page, DIR: DirManager, URL: BaseUrl): """visit nextcloud from authentik with admin_session to create wordpress_admin_session""" with authentik_admin_page.expect_popup() as event_context: authentik_admin_page.get_by_role("link", name="Nextcloud").click() diff --git a/recipes/wordpress/tests_wordpress/setup_wordpress.py b/recipes/wordpress/tests_wordpress/setup_wordpress.py index a65d8fe..3438db7 100644 --- a/recipes/wordpress/tests_wordpress/setup_wordpress.py +++ b/recipes/wordpress/tests_wordpress/setup_wordpress.py @@ -13,7 +13,7 @@ def test_visit_from_domain(authentik_admin_context: BrowserContext, URL: BaseUrl expect(page.locator("#wpadminbar")).to_be_visible(timeout=3_000) -def setup_wordpress_admin_session(authentik_admin_page: Page, DIR: DirManager): +def test_setup_wordpress_admin_session(authentik_admin_page: Page, DIR: DirManager): """visit wordpress from authentik with admin_session to create wordpress_admin_session""" with authentik_admin_page.expect_popup() as event_context: authentik_admin_page.get_by_role("link", name="Wordpress").click() diff --git a/recipes/wordpress/tests_wordpress/setup_wordpress_trigger_email.py b/recipes/wordpress/tests_wordpress/setup_wordpress_trigger_email.py index 33afb6b..9ae1e28 100644 --- a/recipes/wordpress/tests_wordpress/setup_wordpress_trigger_email.py +++ b/recipes/wordpress/tests_wordpress/setup_wordpress_trigger_email.py @@ -5,7 +5,7 @@ from playwright.sync_api import Page, expect from pytest_abra import BaseUrl -def setup_trigger_email(wordpress_admin_page: Page, URL: BaseUrl): +def test_setup_trigger_email(wordpress_admin_page: Page, URL: BaseUrl): """change profile email to EMAIL to trigger email""" page = wordpress_admin_page page.goto(URL.get("wp-admin/profile.php")) diff --git a/tests/test_cli_full_integration.py b/tests/test_cli_full_integration.py index e2d8520..fcf39b9 100644 --- a/tests/test_cli_full_integration.py +++ b/tests/test_cli_full_integration.py @@ -1,3 +1,4 @@ +import shutil import subprocess from pathlib import Path @@ -8,13 +9,25 @@ from pytest_abra.utils import load_json_to_environ @pytest.fixture(scope="session") -def session_tmp_path_testout(tmp_path_factory: pytest.TempPathFactory) -> Path: - return tmp_path_factory.mktemp("test_out") +def tmp_recipes(tmp_path_factory: pytest.TempPathFactory) -> Path: + tmp_recipes_target = tmp_path_factory.mktemp("recipes") + recipes_dir_source = Path("recipes") + shutil.copytree(recipes_dir_source, tmp_recipes_target, dirs_exist_ok=True) + return tmp_recipes_target + + +@pytest.fixture(scope="session") +def tmp_output(tmp_path_factory: pytest.TempPathFactory) -> Path: + return tmp_path_factory.mktemp("output") @pytest.mark.slow -def test_abratest_cli_full_integration(session_tmp_path_testout: Path): - """run abratest against the dev instance""" +def test_abratest_cli_full_integration(tmp_output: Path, tmp_recipes: Path): + """Full integration test of abratest against the dev instance. Recipes dir not in path + + this test is hard to debug as the output dir is in tmp. If required, try + pytest -s + or find the tmp dir to look into test outputs""" # --------------------- load credentials to env variables -------------------- # @@ -33,9 +46,9 @@ def test_abratest_cli_full_integration(session_tmp_path_testout: Path): # ----------------------------------- dirs ----------------------------------- # - RECIPES_DIR = Path("./recipes").resolve() - # OUTPUT_DIR = Path("./test-output").resolve() - OUTPUT_DIR = session_tmp_path_testout.resolve() + RECIPES_DIR = tmp_recipes.resolve() + # RECIPES_DIR = Path("recipes") + OUTPUT_DIR = tmp_output.resolve() # ------------------------------------ run ----------------------------------- # @@ -57,8 +70,8 @@ def test_abratest_cli_full_integration(session_tmp_path_testout: Path): @pytest.mark.slow -def test_results_abra(session_tmp_path_testout: Path): - OUTPUT_DIR = session_tmp_path_testout.resolve() +def test_full_integration_results(tmp_output: Path): + OUTPUT_DIR = tmp_output.resolve() DIR = DirManager(output_dir=OUTPUT_DIR, session_id="abc") all_files = [f.name for f in DIR.STATUS.rglob("*")] diff --git a/tests/test_coordinator.py b/tests/test_coordinator.py index 3c00ed6..08b7975 100644 --- a/tests/test_coordinator.py +++ b/tests/test_coordinator.py @@ -1,5 +1,6 @@ import os import shutil +import sys from pathlib import Path import pytest @@ -36,7 +37,16 @@ def tmp_recipes(tmp_path_factory: pytest.TempPathFactory) -> Path: return tmp_recipes_target -def test_runner_runner_dict_import(tmp_recipes: Path): +@pytest.fixture +def clear_sys_path(): + """clear sys.path before test, restore after""" + syspath_copy = sys.path.copy() + sys.path.clear() + yield + sys.path.extend(syspath_copy) + + +def test_runner_runner_dict_import(tmp_recipes: Path, clear_sys_path): """import from recipes dict should work, because create_runner_dict has sys.path.append""" RUNNER_DICT = Coordinator.create_runner_dict(tmp_recipes) diff --git a/tests/test_dir_manager.py b/tests/test_dir_manager.py index 8dd78b1..caee6fa 100644 --- a/tests/test_dir_manager.py +++ b/tests/test_dir_manager.py @@ -1,23 +1,26 @@ - import time -import pytest -from pytest_abra.dir_manager import DirManager from pathlib import Path +import pytest + +from pytest_abra.dir_manager import DirManager + + def test_get_latest_session_id_from_non_existing_dir(tmp_path: Path): out = DirManager.get_latest_session_id(tmp_path / "not_exist") assert out is None + def test_get_latest_session_id_from_empty_dir(tmp_path: Path): out = DirManager.get_latest_session_id(tmp_path) assert out is None + def test_get_latest_session_id_single(tmp_path: Path): (tmp_path / "a").mkdir() out = DirManager.get_latest_session_id(tmp_path) assert out == "a" - @pytest.mark.slow def test_get_latest_session_id(tmp_path: Path): @@ -26,5 +29,3 @@ def test_get_latest_session_id(tmp_path: Path): (tmp_path / "b").mkdir() out = DirManager.get_latest_session_id(tmp_path) assert out == "b" - - \ No newline at end of file