From 72a50a294cce41c778f5b2ba539f75290d51c4e5 Mon Sep 17 00:00:00 2001 From: Yonatan Goldschmidt Date: Sun, 18 Sep 2022 01:30:23 +0300 Subject: [PATCH 1/2] tests: Break up application_docker_images() More lines-of-code for the same task, but the image generators are much more clear now. This code has become a hassle to work with, and now it's simple again :) --- tests/conftest.py | 122 ++++++++++++------ ...kerfile.libpython => libpython.Dockerfile} | 0 tests/test_java.py | 14 +- tests/{test_libpython.py => test_python.py} | 12 +- tests/type_utils.py | 7 +- 5 files changed, 97 insertions(+), 58 deletions(-) rename tests/containers/python/{Dockerfile.libpython => libpython.Dockerfile} (100%) rename tests/{test_libpython.py => test_python.py} (73%) diff --git a/tests/conftest.py b/tests/conftest.py index 09dc28b28..3134b609f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,14 +2,13 @@ # Copyright (c) Granulate. All rights reserved. # Licensed under the AGPL3 License. See LICENSE.md in the project root for license information. # -import glob import os import stat import subprocess from contextlib import _GeneratorContextManager, contextmanager from functools import partial from pathlib import Path -from typing import Any, Callable, Generator, Iterable, Iterator, List, Mapping, Optional, cast +from typing import Any, Callable, Dict, Generator, Iterable, Iterator, List, Mapping, Optional, cast import docker import pytest @@ -184,47 +183,93 @@ def gprofiler_docker_image(docker_client: DockerClient) -> Iterable[Image]: yield docker_client.images.get("gprofiler") +def _build_image( + docker_client: DockerClient, runtime: str, dockerfile: str = "Dockerfile", **kwargs: Dict[str, Any] +) -> Image: + base_path = CONTAINERS_DIRECTORY / runtime + return docker_client.images.build(path=str(base_path), rm=True, dockerfile=str(base_path / dockerfile), **kwargs)[0] + + +def dotnet_images(docker_client: DockerClient) -> Mapping[str, Image]: + return { + "": _build_image(docker_client, "dotnet"), + } + + +def golang_images(docker_client: DockerClient) -> Mapping[str, Image]: + return { + "": _build_image(docker_client, "golang"), + } + + +def java_images(docker_client: DockerClient) -> Mapping[str, Image]: + return { + "": _build_image(docker_client, "java"), + "j9": _build_image(docker_client, "java", buildargs={"JAVA_BASE_IMAGE": "adoptopenjdk/openjdk8-openj9"}), + "zing": _build_image(docker_client, "java", dockerfile="zing.Dockerfile"), + "musl": _build_image(docker_client, "java", dockerfile="musl.Dockerfile"), + } + + +def native_images(docker_client: DockerClient) -> Mapping[str, Image]: + return { + "fp": _build_image(docker_client, "native", dockerfile="fp.Dockerfile"), + "dwarf": _build_image(docker_client, "native", dockerfile="dwarf.Dockerfile"), + "change_comm": _build_image(docker_client, "native", dockerfile="change_comm.Dockerfile"), + "thread_comm": _build_image(docker_client, "native", dockerfile="thread_comm.Dockerfile"), + } + + +def nodejs_images(docker_client: DockerClient) -> Mapping[str, Image]: + return { + "": _build_image(docker_client, "nodejs"), + } + + +def php_images(docker_client: DockerClient) -> Mapping[str, Image]: + return { + "": _build_image(docker_client, "php"), + } + + +def python_images(docker_client: DockerClient) -> Mapping[str, Image]: + return { + "": _build_image(docker_client, "python"), + "libpython": _build_image(docker_client, "python", dockerfile="libpython.Dockerfile"), + } + + +def ruby_images(docker_client: DockerClient) -> Mapping[str, Image]: + return { + "": _build_image(docker_client, "ruby"), + } + + +def image_name(runtime: str, image_tag: str) -> str: + return runtime + ("_" + image_tag if image_tag else "") + + @fixture(scope="session") def application_docker_images(docker_client: DockerClient) -> Mapping[str, Image]: images = {} - for runtime in os.listdir(str(CONTAINERS_DIRECTORY)): - if runtime == "native": - for dockerfile in glob.glob(str(CONTAINERS_DIRECTORY / runtime / "*.Dockerfile")): - suffix = os.path.splitext(os.path.basename(dockerfile))[0] - images[f"{runtime}_{suffix}"], _ = docker_client.images.build( - path=str(CONTAINERS_DIRECTORY / runtime), dockerfile=str(dockerfile), rm=True - ) - continue - - images[runtime], _ = docker_client.images.build(path=str(CONTAINERS_DIRECTORY / runtime), rm=True) - - # for java - add additional images - if runtime == "java": - images[runtime + "_j9"], _ = docker_client.images.build( - path=str(CONTAINERS_DIRECTORY / runtime), - rm=True, - buildargs={"JAVA_BASE_IMAGE": "adoptopenjdk/openjdk8-openj9"}, - ) - - images[runtime + "_zing"], _ = docker_client.images.build( - path=str(CONTAINERS_DIRECTORY / runtime), - rm=True, - dockerfile=str(CONTAINERS_DIRECTORY / runtime / "zing.Dockerfile"), - ) - - # build musl image if exists - musl_dockerfile = CONTAINERS_DIRECTORY / runtime / "musl.Dockerfile" - if musl_dockerfile.exists(): - images[runtime + "_musl"], _ = docker_client.images.build( - path=str(CONTAINERS_DIRECTORY / runtime), dockerfile=str(musl_dockerfile), rm=True - ) - + image_generators: Dict[str, Callable[[DockerClient], Mapping[str, Image]]] = { + "dotnet": dotnet_images, + "golang": golang_images, + "java": java_images, + "native": native_images, + "nodejs": nodejs_images, + "php": php_images, + "python": python_images, + "ruby": ruby_images, + } + for runtime, func in image_generators.items(): + images.update({image_name(runtime, tag): image for tag, image in func(docker_client).items()}) return images @fixture -def image_suffix() -> str: - # lets tests override this value and use a suffixed image, e.g _musl or _j9. +def application_image_tag() -> str: + # lets tests override this value and use a "tagged" image, e.g "musl" or "j9". return "" @@ -232,10 +277,9 @@ def image_suffix() -> str: def application_docker_image( application_docker_images: Mapping[str, Image], runtime: str, - image_suffix: str, + application_image_tag: str, ) -> Iterable[Image]: - runtime = runtime + image_suffix - yield application_docker_images[runtime] + yield application_docker_images[image_name(runtime, application_image_tag)] @fixture diff --git a/tests/containers/python/Dockerfile.libpython b/tests/containers/python/libpython.Dockerfile similarity index 100% rename from tests/containers/python/Dockerfile.libpython rename to tests/containers/python/libpython.Dockerfile diff --git a/tests/test_java.py b/tests/test_java.py index 1a98fd5c4..d10a4218c 100644 --- a/tests/test_java.py +++ b/tests/test_java.py @@ -48,6 +48,11 @@ ) +@pytest.fixture +def runtime() -> str: + return "java" + + def get_lib_path(application_pid: int, path: str) -> str: libs = set() for m in psutil.Process(application_pid).memory_maps(): @@ -78,11 +83,6 @@ def status_async_profiler(self) -> None: ) -@pytest.fixture -def runtime() -> str: - return "java" - - def test_async_profiler_already_running( application_pid: int, assert_collapsed: AssertInCollapsed, tmp_path: Path, caplog: LogCaptureFixture ) -> None: @@ -146,7 +146,7 @@ def test_java_async_profiler_cpu_mode( @pytest.mark.parametrize("in_container", [True]) -@pytest.mark.parametrize("image_suffix", ["_musl"]) +@pytest.mark.parametrize("application_image_tag", ["musl"]) def test_java_async_profiler_musl_and_cpu( tmp_path: Path, application_pid: int, @@ -339,7 +339,7 @@ def test_async_profiler_stops_after_given_timeout( @pytest.mark.parametrize("in_container", [True]) -@pytest.mark.parametrize("image_suffix,search_for", [("_j9", "OpenJ9"), ("_zing", "Zing")]) +@pytest.mark.parametrize("application_image_tag,search_for", [("j9", "OpenJ9"), ("zing", "Zing")]) def test_sanity_other_jvms( tmp_path: Path, application_pid: int, diff --git a/tests/test_libpython.py b/tests/test_python.py similarity index 73% rename from tests/test_libpython.py rename to tests/test_python.py index 305934c94..88e3d24c3 100644 --- a/tests/test_libpython.py +++ b/tests/test_python.py @@ -6,12 +6,9 @@ from threading import Event import pytest -from docker import DockerClient from docker.models.containers import Container -from docker.models.images import Image from gprofiler.profilers.python import PythonProfiler -from tests import CONTAINERS_DIRECTORY from tests.conftest import AssertInCollapsed from tests.utils import snapshot_pid_collapsed @@ -21,15 +18,8 @@ def runtime() -> str: return "python" -@pytest.fixture(scope="session") -def application_docker_image(docker_client: DockerClient) -> Image: - dockerfile = CONTAINERS_DIRECTORY / "python" / "Dockerfile.libpython" - image: Image = docker_client.images.build(path=str(dockerfile.parent), dockerfile=str(dockerfile), rm=True)[0] - yield image - docker_client.images.remove(image.id, force=True) - - @pytest.mark.parametrize("in_container", [True]) +@pytest.mark.parametrize("application_image_tag", ["libpython"]) def test_python_select_by_libpython( tmp_path: Path, application_docker_container: Container, diff --git a/tests/type_utils.py b/tests/type_utils.py index 9f0a42c6e..369b999f8 100644 --- a/tests/type_utils.py +++ b/tests/type_utils.py @@ -2,7 +2,7 @@ # Copyright (c) Granulate. All rights reserved. # Licensed under the AGPL3 License. See LICENSE.md in the project root for license information. # -from typing import Optional, TypeVar +from typing import Any, Optional, Type, TypeVar T = TypeVar("T") @@ -10,3 +10,8 @@ def cast_away_optional(arg: Optional[T]) -> T: assert arg is not None return arg + + +def assert_cast(typ: Type[T], arg: Any) -> T: + assert isinstance(arg, typ) + return arg From 68a9941f51f7f089e503229498d43f6139f3e92b Mon Sep 17 00:00:00 2001 From: Yonatan Goldschmidt Date: Sun, 18 Sep 2022 01:43:14 +0300 Subject: [PATCH 2/2] simplify --- tests/conftest.py | 106 +++++++++++++++++----------------------------- 1 file changed, 38 insertions(+), 68 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 3134b609f..58195ea4e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -184,86 +184,56 @@ def gprofiler_docker_image(docker_client: DockerClient) -> Iterable[Image]: def _build_image( - docker_client: DockerClient, runtime: str, dockerfile: str = "Dockerfile", **kwargs: Dict[str, Any] + docker_client: DockerClient, runtime: str, dockerfile: str = "Dockerfile", **kwargs: Mapping[str, Any] ) -> Image: base_path = CONTAINERS_DIRECTORY / runtime return docker_client.images.build(path=str(base_path), rm=True, dockerfile=str(base_path / dockerfile), **kwargs)[0] -def dotnet_images(docker_client: DockerClient) -> Mapping[str, Image]: - return { - "": _build_image(docker_client, "dotnet"), - } - - -def golang_images(docker_client: DockerClient) -> Mapping[str, Image]: - return { - "": _build_image(docker_client, "golang"), - } - - -def java_images(docker_client: DockerClient) -> Mapping[str, Image]: - return { - "": _build_image(docker_client, "java"), - "j9": _build_image(docker_client, "java", buildargs={"JAVA_BASE_IMAGE": "adoptopenjdk/openjdk8-openj9"}), - "zing": _build_image(docker_client, "java", dockerfile="zing.Dockerfile"), - "musl": _build_image(docker_client, "java", dockerfile="musl.Dockerfile"), - } - - -def native_images(docker_client: DockerClient) -> Mapping[str, Image]: - return { - "fp": _build_image(docker_client, "native", dockerfile="fp.Dockerfile"), - "dwarf": _build_image(docker_client, "native", dockerfile="dwarf.Dockerfile"), - "change_comm": _build_image(docker_client, "native", dockerfile="change_comm.Dockerfile"), - "thread_comm": _build_image(docker_client, "native", dockerfile="thread_comm.Dockerfile"), - } - - -def nodejs_images(docker_client: DockerClient) -> Mapping[str, Image]: - return { - "": _build_image(docker_client, "nodejs"), - } - - -def php_images(docker_client: DockerClient) -> Mapping[str, Image]: - return { - "": _build_image(docker_client, "php"), - } - - -def python_images(docker_client: DockerClient) -> Mapping[str, Image]: - return { - "": _build_image(docker_client, "python"), - "libpython": _build_image(docker_client, "python", dockerfile="libpython.Dockerfile"), - } - - -def ruby_images(docker_client: DockerClient) -> Mapping[str, Image]: - return { - "": _build_image(docker_client, "ruby"), - } - - def image_name(runtime: str, image_tag: str) -> str: return runtime + ("_" + image_tag if image_tag else "") @fixture(scope="session") def application_docker_images(docker_client: DockerClient) -> Mapping[str, Image]: - images = {} - image_generators: Dict[str, Callable[[DockerClient], Mapping[str, Image]]] = { - "dotnet": dotnet_images, - "golang": golang_images, - "java": java_images, - "native": native_images, - "nodejs": nodejs_images, - "php": php_images, - "python": python_images, - "ruby": ruby_images, + runtime_image_listing: Dict[str, Dict[str, Dict[str, Any]]] = { + "dotnet": { + "": {}, + }, + "golang": { + "": {}, + }, + "java": { + "": {}, + "j9": dict(buildargs={"JAVA_BASE_IMAGE": "adoptopenjdk/openjdk8-openj9"}), + "zing": dict(dockerfile="zing.Dockerfile"), + "musl": dict(dockerfile="musl.Dockerfile"), + }, + "native": { + "fp": dict(dockerfile="fp.Dockerfile"), + "dwarf": dict(dockerfile="dwarf.Dockerfile"), + "change_comm": dict(dockerfile="change_comm.Dockerfile"), + "thread_comm": dict(dockerfile="thread_comm.Dockerfile"), + }, + "nodejs": { + "": {}, + }, + "php": { + "": {}, + }, + "python": { + "": {}, + "libpython": dict(dockerfile="libpython.Dockerfile"), + }, + "ruby": {"": {}}, } - for runtime, func in image_generators.items(): - images.update({image_name(runtime, tag): image for tag, image in func(docker_client).items()}) + + images = {} + for runtime, tags_listing in runtime_image_listing.items(): + for tag, args in tags_listing.items(): + name = image_name(runtime, tag) + assert name not in images + images[name] = _build_image(docker_client, runtime, **args) return images