From e9dcfa2eb2950ff43a266238bb94cb2ec70fb8bc Mon Sep 17 00:00:00 2001 From: Alexander Bayandin Date: Mon, 11 Nov 2024 18:07:01 +0000 Subject: [PATCH] test_runner: skip more tests using decorator instead of pytest.skip (#9704) ## Problem Running `pytest.skip(...)` in a test body instead of marking the test with `@pytest.mark.skipif(...)` makes all fixtures to be initialised, which is not necessary if the test is going to be skipped anyway. Also, some tests are unnecessarily skipped (e.g. `test_layer_bloating` on Postgres 17, or `test_idle_reconnections` at all) or run (e.g. `test_parse_project_git_version_output_positive` more than on once configuration) according to comments. ## Summary of changes - Move `skip_on_postgres` / `xfail_on_postgres` / `run_only_on_default_postgres` decorators to `fixture.utils` - Add new `skip_in_debug_build` and `skip_on_ci` decorators - Replace `pytest.skip(...)` calls with decorators where possible --- test_runner/fixtures/pg_version.py | 31 ++------------ test_runner/fixtures/utils.py | 41 ++++++++++++++++++- ...er_max_throughput_getpage_at_latest_lsn.py | 13 +++--- test_runner/regress/test_branch_and_gc.py | 8 ++-- test_runner/regress/test_compaction.py | 5 +-- .../regress/test_download_extensions.py | 8 ++-- .../regress/test_ingestion_layer_size.py | 9 ++-- test_runner/regress/test_layer_bloating.py | 13 ++++-- test_runner/regress/test_layer_eviction.py | 11 ++--- test_runner/regress/test_logging.py | 3 +- test_runner/regress/test_neon_cli.py | 21 ++++------ .../regress/test_pageserver_layer_rolling.py | 12 ++---- .../regress/test_pageserver_restart.py | 10 ++--- .../regress/test_pageserver_secondary.py | 4 +- test_runner/regress/test_pg_regress.py | 4 +- test_runner/regress/test_replica_start.py | 11 +++-- test_runner/regress/test_sharding.py | 11 ++--- .../regress/test_storage_controller.py | 3 +- test_runner/regress/test_tenant_size.py | 5 +-- .../regress/test_timeline_detach_ancestor.py | 18 ++++---- test_runner/regress/test_wal_acceptor.py | 7 ++-- .../regress/test_wal_acceptor_async.py | 5 +-- 22 files changed, 123 insertions(+), 130 deletions(-) diff --git a/test_runner/fixtures/pg_version.py b/test_runner/fixtures/pg_version.py index 01f02456653e..4feab52c43bb 100644 --- a/test_runner/fixtures/pg_version.py +++ b/test_runner/fixtures/pg_version.py @@ -1,10 +1,8 @@ from __future__ import annotations import enum -import os from typing import TYPE_CHECKING -import pytest from typing_extensions import override if TYPE_CHECKING: @@ -18,12 +16,15 @@ # Inherit PgVersion from str rather than int to make it easier to pass as a command-line argument # TODO: use enum.StrEnum for Python >= 3.11 -@enum.unique class PgVersion(str, enum.Enum): V14 = "14" V15 = "15" V16 = "16" V17 = "17" + + # Default Postgres Version for tests that don't really depend on Postgres itself + DEFAULT = V16 + # Instead of making version an optional parameter in methods, we can use this fake entry # to explicitly rely on the default server version (could be different from pg_version fixture value) NOT_SET = "<-POSTRGRES VERSION IS NOT SET->" @@ -59,27 +60,3 @@ def _missing_(cls, value: object) -> Optional[PgVersion]: # Make mypy happy # See https://github.com/python/mypy/issues/3974 return None - - -DEFAULT_VERSION: PgVersion = PgVersion.V16 - - -def skip_on_postgres(version: PgVersion, reason: str): - return pytest.mark.skipif( - PgVersion(os.environ.get("DEFAULT_PG_VERSION", DEFAULT_VERSION)) is version, - reason=reason, - ) - - -def xfail_on_postgres(version: PgVersion, reason: str): - return pytest.mark.xfail( - PgVersion(os.environ.get("DEFAULT_PG_VERSION", DEFAULT_VERSION)) is version, - reason=reason, - ) - - -def run_only_on_default_postgres(reason: str): - return pytest.mark.skipif( - PgVersion(os.environ.get("DEFAULT_PG_VERSION", DEFAULT_VERSION)) is not DEFAULT_VERSION, - reason=reason, - ) diff --git a/test_runner/fixtures/utils.py b/test_runner/fixtures/utils.py index 01b7cf1026b3..96a651f0dba6 100644 --- a/test_runner/fixtures/utils.py +++ b/test_runner/fixtures/utils.py @@ -25,6 +25,7 @@ parse_delta_layer, parse_image_layer, ) +from fixtures.pg_version import PgVersion if TYPE_CHECKING: from collections.abc import Iterable @@ -37,6 +38,7 @@ Fn = TypeVar("Fn", bound=Callable[..., Any]) + COMPONENT_BINARIES = { "storage_controller": ("storage_controller",), "storage_broker": ("storage_broker",), @@ -519,7 +521,7 @@ def assert_pageserver_backups_equal(left: Path, right: Path, skip_files: set[str This is essentially: lines=$(comm -3 \ - <(mkdir left && cd left && tar xf "$left" && find . -type f -print0 | xargs sha256sum | sort -k2) \ + <(mkdir left && cd left && tar xf "$left" && find . -type f -print0 | xargs sha256sum | sort -k2) \ <(mkdir right && cd right && tar xf "$right" && find . -type f -print0 | xargs sha256sum | sort -k2) \ | wc -l) [ "$lines" = "0" ] @@ -643,3 +645,40 @@ def allpairs_versions(): ) ids.append(f"combination_{''.join(cur_id)}") return {"argnames": "combination", "argvalues": tuple(argvalues), "ids": ids} + + +def skip_on_postgres(version: PgVersion, reason: str): + return pytest.mark.skipif( + PgVersion(os.getenv("DEFAULT_PG_VERSION", PgVersion.DEFAULT)) is version, + reason=reason, + ) + + +def xfail_on_postgres(version: PgVersion, reason: str): + return pytest.mark.xfail( + PgVersion(os.getenv("DEFAULT_PG_VERSION", PgVersion.DEFAULT)) is version, + reason=reason, + ) + + +def run_only_on_default_postgres(reason: str): + return pytest.mark.skipif( + PgVersion(os.getenv("DEFAULT_PG_VERSION", PgVersion.DEFAULT)) is not PgVersion.DEFAULT, + reason=reason, + ) + + +def skip_in_debug_build(reason: str): + return pytest.mark.skipif( + os.getenv("BUILD_TYPE", "debug") == "debug", + reason=reason, + ) + + +def skip_on_ci(reason: str): + # `CI` variable is always set to `true` on GitHub + # Ref: https://docs.github.com/en/actions/writing-workflows/choosing-what-your-workflow-does/store-information-in-variables#default-environment-variables + return pytest.mark.skipif( + os.getenv("CI", "false") == "true", + reason=reason, + ) diff --git a/test_runner/performance/pageserver/pagebench/test_pageserver_max_throughput_getpage_at_latest_lsn.py b/test_runner/performance/pageserver/pagebench/test_pageserver_max_throughput_getpage_at_latest_lsn.py index c038fc3fd2d2..3dbbb197f442 100644 --- a/test_runner/performance/pageserver/pagebench/test_pageserver_max_throughput_getpage_at_latest_lsn.py +++ b/test_runner/performance/pageserver/pagebench/test_pageserver_max_throughput_getpage_at_latest_lsn.py @@ -1,7 +1,6 @@ from __future__ import annotations import json -import os from pathlib import Path from typing import TYPE_CHECKING @@ -14,7 +13,7 @@ PgBin, wait_for_last_flush_lsn, ) -from fixtures.utils import get_scale_for_db, humantime_to_ms +from fixtures.utils import get_scale_for_db, humantime_to_ms, skip_on_ci from performance.pageserver.util import ( setup_pageserver_with_tenants, @@ -38,9 +37,8 @@ @pytest.mark.parametrize("pgbench_scale", [get_scale_for_db(200)]) @pytest.mark.parametrize("n_tenants", [500]) @pytest.mark.timeout(10000) -@pytest.mark.skipif( - os.getenv("CI", "false") == "true", - reason="This test needs lot of resources and should run on dedicated HW, not in github action runners as part of CI", +@skip_on_ci( + "This test needs lot of resources and should run on dedicated HW, not in github action runners as part of CI" ) def test_pageserver_characterize_throughput_with_n_tenants( neon_env_builder: NeonEnvBuilder, @@ -66,9 +64,8 @@ def test_pageserver_characterize_throughput_with_n_tenants( @pytest.mark.parametrize("n_clients", [1, 64]) @pytest.mark.parametrize("n_tenants", [1]) @pytest.mark.timeout(2400) -@pytest.mark.skipif( - os.getenv("CI", "false") == "true", - reason="This test needs lot of resources and should run on dedicated HW, not in github action runners as part of CI", +@skip_on_ci( + "This test needs lot of resources and should run on dedicated HW, not in github action runners as part of CI" ) def test_pageserver_characterize_latencies_with_1_client_and_throughput_with_many_clients_one_tenant( neon_env_builder: NeonEnvBuilder, diff --git a/test_runner/regress/test_branch_and_gc.py b/test_runner/regress/test_branch_and_gc.py index 6d1565c5e51a..fccfbc7f0924 100644 --- a/test_runner/regress/test_branch_and_gc.py +++ b/test_runner/regress/test_branch_and_gc.py @@ -8,7 +8,7 @@ from fixtures.log_helper import log from fixtures.neon_fixtures import NeonEnv from fixtures.pageserver.http import TimelineCreate406 -from fixtures.utils import query_scalar +from fixtures.utils import query_scalar, skip_in_debug_build # Test the GC implementation when running with branching. @@ -48,10 +48,8 @@ # Because the delta layer D covering lsn1 is corrupted, creating a branch # starting from lsn1 should return an error as follows: # could not find data for key ... at LSN ..., for request at LSN ... -def test_branch_and_gc(neon_simple_env: NeonEnv, build_type: str): - if build_type == "debug": - pytest.skip("times out in debug builds") - +@skip_in_debug_build("times out in debug builds") +def test_branch_and_gc(neon_simple_env: NeonEnv): env = neon_simple_env pageserver_http_client = env.pageserver.http_client() diff --git a/test_runner/regress/test_compaction.py b/test_runner/regress/test_compaction.py index 420055ac3ad7..370df3c3792e 100644 --- a/test_runner/regress/test_compaction.py +++ b/test_runner/regress/test_compaction.py @@ -2,7 +2,6 @@ import enum import json -import os import time from typing import TYPE_CHECKING @@ -13,7 +12,7 @@ generate_uploads_and_deletions, ) from fixtures.pageserver.http import PageserverApiException -from fixtures.utils import wait_until +from fixtures.utils import skip_in_debug_build, wait_until from fixtures.workload import Workload if TYPE_CHECKING: @@ -32,7 +31,7 @@ } -@pytest.mark.skipif(os.environ.get("BUILD_TYPE") == "debug", reason="only run with release build") +@skip_in_debug_build("only run with release build") def test_pageserver_compaction_smoke(neon_env_builder: NeonEnvBuilder): """ This is a smoke test that compaction kicks in. The workload repeatedly churns diff --git a/test_runner/regress/test_download_extensions.py b/test_runner/regress/test_download_extensions.py index 0134f80769ae..b2e19ad713a1 100644 --- a/test_runner/regress/test_download_extensions.py +++ b/test_runner/regress/test_download_extensions.py @@ -12,6 +12,7 @@ NeonEnvBuilder, ) from fixtures.pg_version import PgVersion +from fixtures.utils import skip_on_postgres from pytest_httpserver import HTTPServer from werkzeug.wrappers.request import Request from werkzeug.wrappers.response import Response @@ -41,17 +42,14 @@ def neon_env_builder_local( return neon_env_builder +@skip_on_postgres(PgVersion.V16, reason="TODO: PG16 extension building") +@skip_on_postgres(PgVersion.V17, reason="TODO: PG17 extension building") def test_remote_extensions( httpserver: HTTPServer, neon_env_builder_local: NeonEnvBuilder, httpserver_listen_address, pg_version, ): - if pg_version == PgVersion.V16: - pytest.skip("TODO: PG16 extension building") - if pg_version == PgVersion.V17: - pytest.skip("TODO: PG17 extension building") - # setup mock http server # that expects request for anon.tar.zst # and returns the requested file diff --git a/test_runner/regress/test_ingestion_layer_size.py b/test_runner/regress/test_ingestion_layer_size.py index 646dac8e6eaa..291674892554 100644 --- a/test_runner/regress/test_ingestion_layer_size.py +++ b/test_runner/regress/test_ingestion_layer_size.py @@ -4,25 +4,22 @@ from dataclasses import dataclass from typing import TYPE_CHECKING -import pytest from fixtures.log_helper import log from fixtures.neon_fixtures import NeonEnvBuilder, wait_for_last_flush_lsn from fixtures.pageserver.http import HistoricLayerInfo, LayerMapInfo -from fixtures.utils import human_bytes +from fixtures.utils import human_bytes, skip_in_debug_build if TYPE_CHECKING: from typing import Union -def test_ingesting_large_batches_of_images(neon_env_builder: NeonEnvBuilder, build_type: str): +@skip_in_debug_build("debug run is unnecessarily slow") +def test_ingesting_large_batches_of_images(neon_env_builder: NeonEnvBuilder): """ Build a non-small GIN index which includes similarly batched up images in WAL stream as does pgvector to show that we no longer create oversized layers. """ - if build_type == "debug": - pytest.skip("debug run is unnecessarily slow") - minimum_initdb_size = 20 * 1024**2 checkpoint_distance = 32 * 1024**2 minimum_good_layer_size = checkpoint_distance * 0.9 diff --git a/test_runner/regress/test_layer_bloating.py b/test_runner/regress/test_layer_bloating.py index a08d522fc2f7..d9043fef7fb3 100644 --- a/test_runner/regress/test_layer_bloating.py +++ b/test_runner/regress/test_layer_bloating.py @@ -2,7 +2,6 @@ import os -import pytest from fixtures.log_helper import log from fixtures.neon_fixtures import ( NeonEnvBuilder, @@ -10,12 +9,18 @@ wait_for_last_flush_lsn, ) from fixtures.pg_version import PgVersion +from fixtures.utils import skip_on_postgres +@skip_on_postgres( + PgVersion.V14, + reason="pg_log_standby_snapshot() function is available since Postgres 16", +) +@skip_on_postgres( + PgVersion.V15, + reason="pg_log_standby_snapshot() function is available since Postgres 16", +) def test_layer_bloating(neon_env_builder: NeonEnvBuilder, vanilla_pg): - if neon_env_builder.pg_version != PgVersion.V16: - pytest.skip("pg_log_standby_snapshot() function is available only in PG16") - env = neon_env_builder.init_start( initial_tenant_conf={ "gc_period": "0s", diff --git a/test_runner/regress/test_layer_eviction.py b/test_runner/regress/test_layer_eviction.py index c49ac6893ed8..2eb38c49b200 100644 --- a/test_runner/regress/test_layer_eviction.py +++ b/test_runner/regress/test_layer_eviction.py @@ -2,7 +2,6 @@ import time -import pytest from fixtures.log_helper import log from fixtures.neon_fixtures import ( NeonEnvBuilder, @@ -12,17 +11,13 @@ from fixtures.pageserver.common_types import parse_layer_file_name from fixtures.pageserver.utils import wait_for_upload from fixtures.remote_storage import RemoteStorageKind +from fixtures.utils import skip_in_debug_build # Crates a few layers, ensures that we can evict them (removing locally but keeping track of them anyway) # and then download them back. -def test_basic_eviction( - neon_env_builder: NeonEnvBuilder, - build_type: str, -): - if build_type == "debug": - pytest.skip("times out in debug builds") - +@skip_in_debug_build("times out in debug builds") +def test_basic_eviction(neon_env_builder: NeonEnvBuilder): neon_env_builder.enable_pageserver_remote_storage(RemoteStorageKind.LOCAL_FS) env = neon_env_builder.init_start( diff --git a/test_runner/regress/test_logging.py b/test_runner/regress/test_logging.py index 9a3fdd835dfc..f6fbdcabfd9d 100644 --- a/test_runner/regress/test_logging.py +++ b/test_runner/regress/test_logging.py @@ -5,8 +5,7 @@ import pytest from fixtures.log_helper import log from fixtures.neon_fixtures import NeonEnvBuilder -from fixtures.pg_version import run_only_on_default_postgres -from fixtures.utils import wait_until +from fixtures.utils import run_only_on_default_postgres, wait_until @pytest.mark.parametrize("level", ["trace", "debug", "info", "warn", "error"]) diff --git a/test_runner/regress/test_neon_cli.py b/test_runner/regress/test_neon_cli.py index 783fb813cf27..72db72f2b9f8 100644 --- a/test_runner/regress/test_neon_cli.py +++ b/test_runner/regress/test_neon_cli.py @@ -1,6 +1,5 @@ from __future__ import annotations -import os import subprocess from pathlib import Path from typing import cast @@ -15,7 +14,7 @@ parse_project_git_version_output, ) from fixtures.pageserver.http import PageserverHttpClient -from fixtures.pg_version import PgVersion, skip_on_postgres +from fixtures.utils import run_only_on_default_postgres, skip_in_debug_build def helper_compare_timeline_list( @@ -195,10 +194,8 @@ def test_cli_start_stop_multi(neon_env_builder: NeonEnvBuilder): res.check_returncode() -@skip_on_postgres(PgVersion.V14, reason="does not use postgres") -@pytest.mark.skipif( - os.environ.get("BUILD_TYPE") == "debug", reason="unit test for test support, either build works" -) +@run_only_on_default_postgres(reason="does not use postgres") +@skip_in_debug_build("unit test for test support, either build works") def test_parse_project_git_version_output_positive(): commit = "b6f77b5816cf1dba12a3bc8747941182ce220846" @@ -217,10 +214,8 @@ def test_parse_project_git_version_output_positive(): assert parse_project_git_version_output(example) == commit -@skip_on_postgres(PgVersion.V14, reason="does not use postgres") -@pytest.mark.skipif( - os.environ.get("BUILD_TYPE") == "debug", reason="unit test for test support, either build works" -) +@run_only_on_default_postgres(reason="does not use postgres") +@skip_in_debug_build("unit test for test support, either build works") def test_parse_project_git_version_output_local_docker(): """ Makes sure the tests don't accept the default version in Dockerfile one gets without providing @@ -234,10 +229,8 @@ def test_parse_project_git_version_output_local_docker(): assert input in str(e) -@skip_on_postgres(PgVersion.V14, reason="does not use postgres") -@pytest.mark.skipif( - os.environ.get("BUILD_TYPE") == "debug", reason="cli api sanity, either build works" -) +@run_only_on_default_postgres(reason="does not use postgres") +@skip_in_debug_build("unit test for test support, either build works") def test_binaries_version_parses(neon_binpath: Path): """ Ensures that we can parse the actual outputs of --version from a set of binaries. diff --git a/test_runner/regress/test_pageserver_layer_rolling.py b/test_runner/regress/test_pageserver_layer_rolling.py index c0eb598891f4..200a323a3adb 100644 --- a/test_runner/regress/test_pageserver_layer_rolling.py +++ b/test_runner/regress/test_pageserver_layer_rolling.py @@ -1,7 +1,6 @@ from __future__ import annotations import asyncio -import os import time from typing import TYPE_CHECKING @@ -16,7 +15,7 @@ ) from fixtures.pageserver.http import PageserverHttpClient from fixtures.pageserver.utils import wait_for_last_record_lsn, wait_for_upload -from fixtures.utils import wait_until +from fixtures.utils import skip_in_debug_build, wait_until if TYPE_CHECKING: from typing import Optional @@ -227,12 +226,9 @@ def test_idle_checkpoints(neon_env_builder: NeonEnvBuilder): assert get_dirty_bytes(env) >= dirty_after_write -@pytest.mark.skipif( - # We have to use at least ~100MB of data to hit the lowest limit we can configure, which is - # prohibitively slow in debug mode - os.getenv("BUILD_TYPE") == "debug", - reason="Avoid running bulkier ingest tests in debug mode", -) +# We have to use at least ~100MB of data to hit the lowest limit we can configure, which is +# prohibitively slow in debug mode +@skip_in_debug_build("Avoid running bulkier ingest tests in debug mode") def test_total_size_limit(neon_env_builder: NeonEnvBuilder): """ Test that checkpoints are done based on total ephemeral layer size, even if no one timeline is diff --git a/test_runner/regress/test_pageserver_restart.py b/test_runner/regress/test_pageserver_restart.py index f7c42fc893f7..fb6050689cf0 100644 --- a/test_runner/regress/test_pageserver_restart.py +++ b/test_runner/regress/test_pageserver_restart.py @@ -8,7 +8,7 @@ from fixtures.log_helper import log from fixtures.neon_fixtures import NeonEnvBuilder from fixtures.remote_storage import s3_storage -from fixtures.utils import wait_until +from fixtures.utils import skip_in_debug_build, wait_until # Test restarting page server, while safekeeper and compute node keep @@ -155,12 +155,8 @@ def assert_complete(): # safekeeper and compute node keep running. @pytest.mark.timeout(540) @pytest.mark.parametrize("shard_count", [None, 4]) -def test_pageserver_chaos( - neon_env_builder: NeonEnvBuilder, build_type: str, shard_count: Optional[int] -): - if build_type == "debug": - pytest.skip("times out in debug builds") - +@skip_in_debug_build("times out in debug builds") +def test_pageserver_chaos(neon_env_builder: NeonEnvBuilder, shard_count: Optional[int]): # same rationale as with the immediate stop; we might leave orphan layers behind. neon_env_builder.disable_scrub_on_exit() neon_env_builder.enable_pageserver_remote_storage(s3_storage()) diff --git a/test_runner/regress/test_pageserver_secondary.py b/test_runner/regress/test_pageserver_secondary.py index 705b4ff05473..d4aef967353e 100644 --- a/test_runner/regress/test_pageserver_secondary.py +++ b/test_runner/regress/test_pageserver_secondary.py @@ -17,7 +17,7 @@ wait_for_upload_queue_empty, ) from fixtures.remote_storage import LocalFsStorage, RemoteStorageKind, S3Storage, s3_storage -from fixtures.utils import wait_until +from fixtures.utils import skip_in_debug_build, wait_until from fixtures.workload import Workload from werkzeug.wrappers.request import Request from werkzeug.wrappers.response import Response @@ -765,7 +765,7 @@ def await_log(pageserver, deadline, expression): assert download_rate < expect_download_rate * 2 -@pytest.mark.skipif(os.environ.get("BUILD_TYPE") == "debug", reason="only run with release build") +@skip_in_debug_build("only run with release build") @pytest.mark.parametrize("via_controller", [True, False]) def test_slow_secondary_downloads(neon_env_builder: NeonEnvBuilder, via_controller: bool): """ diff --git a/test_runner/regress/test_pg_regress.py b/test_runner/regress/test_pg_regress.py index b97fccddf56e..f4698191eb9c 100644 --- a/test_runner/regress/test_pg_regress.py +++ b/test_runner/regress/test_pg_regress.py @@ -3,7 +3,6 @@ # from __future__ import annotations -import os from concurrent.futures import ThreadPoolExecutor from pathlib import Path from typing import TYPE_CHECKING, cast @@ -19,6 +18,7 @@ ) from fixtures.pg_version import PgVersion from fixtures.remote_storage import s3_storage +from fixtures.utils import skip_in_debug_build if TYPE_CHECKING: from typing import Optional @@ -329,7 +329,7 @@ def test_sql_regress( post_checks(env, test_output_dir, DBNAME, endpoint) -@pytest.mark.skipif(os.environ.get("BUILD_TYPE") == "debug", reason="only run with release build") +@skip_in_debug_build("only run with release build") def test_tx_abort_with_many_relations( neon_env_builder: NeonEnvBuilder, ): diff --git a/test_runner/regress/test_replica_start.py b/test_runner/regress/test_replica_start.py index e81e7dad761f..8e7c01f95029 100644 --- a/test_runner/regress/test_replica_start.py +++ b/test_runner/regress/test_replica_start.py @@ -30,7 +30,7 @@ from fixtures.log_helper import log from fixtures.neon_fixtures import NeonEnv, wait_for_last_flush_lsn, wait_replica_caughtup from fixtures.pg_version import PgVersion -from fixtures.utils import query_scalar, wait_until +from fixtures.utils import query_scalar, skip_on_postgres, wait_until CREATE_SUBXACTS_FUNC = """ create or replace function create_subxacts(n integer) returns void as $$ @@ -137,6 +137,12 @@ def test_replica_start_scan_clog_crashed_xids(neon_simple_env: NeonEnv): assert secondary_cur.fetchone() == (1,) +@skip_on_postgres( + PgVersion.V14, reason="pg_log_standby_snapshot() function is available since Postgres 16" +) +@skip_on_postgres( + PgVersion.V15, reason="pg_log_standby_snapshot() function is available since Postgres 16" +) def test_replica_start_at_running_xacts(neon_simple_env: NeonEnv, pg_version): """ Test that starting a replica works right after the primary has @@ -149,9 +155,6 @@ def test_replica_start_at_running_xacts(neon_simple_env: NeonEnv, pg_version): """ env = neon_simple_env - if env.pg_version == PgVersion.V14 or env.pg_version == PgVersion.V15: - pytest.skip("pg_log_standby_snapshot() function is available only in PG16") - primary = env.endpoints.create_start(branch_name="main", endpoint_id="primary") primary_conn = primary.connect() primary_cur = primary_conn.cursor() diff --git a/test_runner/regress/test_sharding.py b/test_runner/regress/test_sharding.py index ec633e352c0e..0a4a53356d94 100644 --- a/test_runner/regress/test_sharding.py +++ b/test_runner/regress/test_sharding.py @@ -20,7 +20,7 @@ ) from fixtures.pageserver.utils import assert_prefix_empty, assert_prefix_not_empty from fixtures.remote_storage import s3_storage -from fixtures.utils import wait_until +from fixtures.utils import skip_in_debug_build, wait_until from fixtures.workload import Workload from pytest_httpserver import HTTPServer from typing_extensions import override @@ -853,12 +853,9 @@ def assert_restart_notification(): wait_until(10, 1, assert_restart_notification) -@pytest.mark.skipif( - # The quantity of data isn't huge, but debug can be _very_ slow, and the things we're - # validating in this test don't benefit much from debug assertions. - os.getenv("BUILD_TYPE") == "debug", - reason="Avoid running bulkier ingest tests in debug mode", -) +# The quantity of data isn't huge, but debug can be _very_ slow, and the things we're +# validating in this test don't benefit much from debug assertions. +@skip_in_debug_build("Avoid running bulkier ingest tests in debug mode") def test_sharding_ingest_layer_sizes( neon_env_builder: NeonEnvBuilder, ): diff --git a/test_runner/regress/test_storage_controller.py b/test_runner/regress/test_storage_controller.py index a069e0d01c79..2c3d79b18a95 100644 --- a/test_runner/regress/test_storage_controller.py +++ b/test_runner/regress/test_storage_controller.py @@ -36,11 +36,12 @@ remote_storage_delete_key, timeline_delete_wait_completed, ) -from fixtures.pg_version import PgVersion, run_only_on_default_postgres +from fixtures.pg_version import PgVersion from fixtures.port_distributor import PortDistributor from fixtures.remote_storage import RemoteStorageKind, s3_storage from fixtures.storage_controller_proxy import StorageControllerProxy from fixtures.utils import ( + run_only_on_default_postgres, run_pg_bench_small, subprocess_capture, wait_until, diff --git a/test_runner/regress/test_tenant_size.py b/test_runner/regress/test_tenant_size.py index 0c431fa453eb..8b733da0c67f 100644 --- a/test_runner/regress/test_tenant_size.py +++ b/test_runner/regress/test_tenant_size.py @@ -1,6 +1,5 @@ from __future__ import annotations -import os from concurrent.futures import ThreadPoolExecutor from pathlib import Path @@ -21,7 +20,7 @@ wait_until_tenant_active, ) from fixtures.pg_version import PgVersion -from fixtures.utils import wait_until +from fixtures.utils import skip_in_debug_build, wait_until def test_empty_tenant_size(neon_env_builder: NeonEnvBuilder): @@ -279,7 +278,7 @@ def test_only_heads_within_horizon(neon_simple_env: NeonEnv, test_output_dir: Pa size_debug_file.write(size_debug) -@pytest.mark.skipif(os.environ.get("BUILD_TYPE") == "debug", reason="only run with release build") +@skip_in_debug_build("only run with release build") def test_single_branch_get_tenant_size_grows( neon_env_builder: NeonEnvBuilder, test_output_dir: Path, pg_version: PgVersion ): diff --git a/test_runner/regress/test_timeline_detach_ancestor.py b/test_runner/regress/test_timeline_detach_ancestor.py index 0e8519e07b53..ef0eb056126c 100644 --- a/test_runner/regress/test_timeline_detach_ancestor.py +++ b/test_runner/regress/test_timeline_detach_ancestor.py @@ -869,8 +869,17 @@ def restart_restarted(): assert count == 10000 -@pytest.mark.parametrize("mode", ["delete_timeline", "delete_tenant"]) -@pytest.mark.parametrize("sharded", [False, True]) +@pytest.mark.parametrize( + "mode, sharded", + [ + ("delete_timeline", False), + ("delete_timeline", True), + ("delete_tenant", False), + # the shared/exclusive lock for tenant is blocking this: + # timeline detach ancestor takes shared, delete tenant takes exclusive + # ("delete_tenant", True) + ], +) def test_timeline_detach_ancestor_interrupted_by_deletion( neon_env_builder: NeonEnvBuilder, mode: str, sharded: bool ): @@ -885,11 +894,6 @@ def test_timeline_detach_ancestor_interrupted_by_deletion( - shutdown winning over complete, see test_timeline_is_deleted_before_timeline_detach_ancestor_completes """ - if sharded and mode == "delete_tenant": - # the shared/exclusive lock for tenant is blocking this: - # timeline detach ancestor takes shared, delete tenant takes exclusive - pytest.skip("tenant deletion while timeline ancestor detach is underway cannot happen") - shard_count = 2 if sharded else 1 neon_env_builder.num_pageservers = shard_count diff --git a/test_runner/regress/test_wal_acceptor.py b/test_runner/regress/test_wal_acceptor.py index e224d5eb0100..0676b3dd9ab4 100644 --- a/test_runner/regress/test_wal_acceptor.py +++ b/test_runner/regress/test_wal_acceptor.py @@ -54,6 +54,8 @@ PropagatingThread, get_dir_size, query_scalar, + run_only_on_default_postgres, + skip_in_debug_build, start_in_background, wait_until, ) @@ -2104,10 +2106,9 @@ def unevicted_on_dest(): # The only way to verify this without manipulating time is to sleep for a while. # In this test we sleep for 60 seconds, so this test takes at least 1 minute to run. # This is longer than most other tests, we run it only for v16 to save CI resources. +@run_only_on_default_postgres("run only on release build to save CI resources") +@skip_in_debug_build("run only on release build to save CI resources") def test_idle_reconnections(neon_env_builder: NeonEnvBuilder): - if os.environ.get("PYTEST_CURRENT_TEST", "").find("[debug-pg16]") == -1: - pytest.skip("run only on debug postgres v16 to save CI resources") - neon_env_builder.num_safekeepers = 3 env = neon_env_builder.init_start() diff --git a/test_runner/regress/test_wal_acceptor_async.py b/test_runner/regress/test_wal_acceptor_async.py index f328974264fa..d3e989afa8c2 100644 --- a/test_runner/regress/test_wal_acceptor_async.py +++ b/test_runner/regress/test_wal_acceptor_async.py @@ -14,6 +14,7 @@ from fixtures.log_helper import getLogger from fixtures.neon_fixtures import Endpoint, NeonEnv, NeonEnvBuilder, Safekeeper from fixtures.remote_storage import RemoteStorageKind +from fixtures.utils import skip_in_debug_build if TYPE_CHECKING: from typing import Optional @@ -760,10 +761,8 @@ def adjust_safekeepers(env: NeonEnv, active_sk: list[bool]): # The test takes more than default 5 minutes on Postgres 16, # see https://github.com/neondatabase/neon/issues/5305 @pytest.mark.timeout(600) +@skip_in_debug_build("times out in debug builds") def test_wal_lagging(neon_env_builder: NeonEnvBuilder, test_output_dir: Path, build_type: str): - if build_type == "debug": - pytest.skip("times out in debug builds") - neon_env_builder.num_safekeepers = 3 env = neon_env_builder.init_start()