From 9da3935940f8cded438f1d71cfb44413b625e611 Mon Sep 17 00:00:00 2001 From: Alex Stephen Date: Fri, 11 Sep 2026 18:22:39 +0000 Subject: [PATCH 1/2] Run all unit tests --- .github/workflows/python-ci.yml | 2 +- Makefile | 2 +- tests/catalog/test_rest.py | 62 ++++++++++++++------------------- tests/conftest.py | 5 ++- 4 files changed, 33 insertions(+), 38 deletions(-) diff --git a/.github/workflows/python-ci.yml b/.github/workflows/python-ci.yml index 4d3cebbbe2..ef0db12406 100644 --- a/.github/workflows/python-ci.yml +++ b/.github/workflows/python-ci.yml @@ -89,7 +89,7 @@ jobs: - name: Install run: uv sync --all-extras --no-extra hive-kerberos - name: Run unit tests - run: uv run python -m pytest tests/ -m "(unmarked or parametrize) and not integration" --ignore=tests/integration -v -x + run: uv run python -m pytest tests/ -m unmarked --ignore=tests/integration -v -x cibw-dev-env-smoke-test: runs-on: ubuntu-latest diff --git a/Makefile b/Makefile index 9a73b240a7..cff86b5840 100644 --- a/Makefile +++ b/Makefile @@ -104,7 +104,7 @@ lint: ## Run code linters via prek (pre-commit hooks) ##@ Testing test: ## Run all unit tests (excluding integration) - $(TEST_RUNNER) pytest tests/ -m "(unmarked or parametrize) and not integration" $(PYTEST_ARGS) + $(TEST_RUNNER) pytest tests/ -m unmarked $(PYTEST_ARGS) test-integration: test-integration-setup test-integration-exec test-integration-cleanup ## Run integration tests diff --git a/tests/catalog/test_rest.py b/tests/catalog/test_rest.py index 2cc01597ee..0953c5f793 100644 --- a/tests/catalog/test_rest.py +++ b/tests/catalog/test_rest.py @@ -206,10 +206,9 @@ def test_token_200(rest_mock: Mocker) -> None: status_code=200, request_headers=OAUTH_TEST_HEADERS, ) - assert ( - RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS)._session.headers["Authorization"] # pylint: disable=W0212 - == f"Bearer {TEST_TOKEN}" - ) + catalog = RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS) + prepared = catalog._session.prepare_request(Request("GET", TEST_URI)) + assert prepared.headers["Authorization"] == f"Bearer {TEST_TOKEN}" @pytest.mark.filterwarnings( @@ -226,10 +225,9 @@ def test_token_200_without_optional_fields(rest_mock: Mocker) -> None: status_code=200, request_headers=OAUTH_TEST_HEADERS, ) - assert ( - RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS)._session.headers["Authorization"] # pylint: disable=W0212 - == f"Bearer {TEST_TOKEN}" - ) + catalog = RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS) + prepared = catalog._session.prepare_request(Request("GET", TEST_URI)) + assert prepared.headers["Authorization"] == f"Bearer {TEST_TOKEN}" @pytest.mark.filterwarnings( @@ -248,12 +246,9 @@ def test_token_with_optional_oauth_params(rest_mock: Mocker) -> None: status_code=200, request_headers=OAUTH_TEST_HEADERS, ) - assert ( - RestCatalog( - "rest", uri=TEST_URI, credential=TEST_CREDENTIALS, audience=TEST_AUDIENCE, resource=TEST_RESOURCE - )._session.headers["Authorization"] - == f"Bearer {TEST_TOKEN}" - ) + catalog = RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS, audience=TEST_AUDIENCE, resource=TEST_RESOURCE) + prepared = catalog._session.prepare_request(Request("GET", TEST_URI)) + assert prepared.headers["Authorization"] == f"Bearer {TEST_TOKEN}" assert TEST_AUDIENCE in mock_request.last_request.text assert TEST_RESOURCE in mock_request.last_request.text @@ -274,10 +269,9 @@ def test_token_with_optional_oauth_params_as_empty(rest_mock: Mocker) -> None: status_code=200, request_headers=OAUTH_TEST_HEADERS, ) - assert ( - RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS, audience="", resource="")._session.headers["Authorization"] - == f"Bearer {TEST_TOKEN}" - ) + catalog = RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS, audience="", resource="") + prepared = catalog._session.prepare_request(Request("GET", TEST_URI)) + assert prepared.headers["Authorization"] == f"Bearer {TEST_TOKEN}" assert TEST_AUDIENCE not in mock_request.last_request.text assert TEST_RESOURCE not in mock_request.last_request.text @@ -298,9 +292,9 @@ def test_token_with_default_scope(rest_mock: Mocker) -> None: status_code=200, request_headers=OAUTH_TEST_HEADERS, ) - assert ( - RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS)._session.headers["Authorization"] == f"Bearer {TEST_TOKEN}" - ) + catalog = RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS) + prepared = catalog._session.prepare_request(Request("GET", TEST_URI)) + assert prepared.headers["Authorization"] == f"Bearer {TEST_TOKEN}" assert "catalog" in mock_request.last_request.text @@ -320,10 +314,9 @@ def test_token_with_custom_scope(rest_mock: Mocker) -> None: status_code=200, request_headers=OAUTH_TEST_HEADERS, ) - assert ( - RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS, scope=TEST_SCOPE)._session.headers["Authorization"] - == f"Bearer {TEST_TOKEN}" - ) + catalog = RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS, scope=TEST_SCOPE) + prepared = catalog._session.prepare_request(Request("GET", TEST_URI)) + assert prepared.headers["Authorization"] == f"Bearer {TEST_TOKEN}" assert TEST_SCOPE in mock_request.last_request.text @@ -343,14 +336,9 @@ def test_token_200_w_oauth2_server_uri(rest_mock: Mocker) -> None: status_code=200, request_headers=OAUTH_TEST_HEADERS, ) - # pylint: disable=W0212 - assert ( - RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS, **{OAUTH2_SERVER_URI: OAUTH2_SERVER_URI})._session.headers[ - "Authorization" - ] - == f"Bearer {TEST_TOKEN}" - ) - # pylint: enable=W0212 + catalog = RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS, **{OAUTH2_SERVER_URI: TEST_OAUTH2_SERVER_URI}) + prepared = catalog._session.prepare_request(Request("GET", TEST_URI)) + assert prepared.headers["Authorization"] == f"Bearer {TEST_TOKEN}" @pytest.mark.filterwarnings( @@ -377,7 +365,8 @@ def test_config_200(requests_mock: Mocker) -> None: RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS, warehouse="s3://some-bucket") assert requests_mock.called - assert requests_mock.call_count == 2 + # The token is fetched for the config request, and again for the catalog session + assert requests_mock.call_count == 3 history = requests_mock.request_history assert history[1].method == "GET" @@ -2904,7 +2893,10 @@ def test_auth_header(rest_mock: Mocker) -> None: ) RestCatalog("rest", uri=TEST_URI, credential=TEST_CREDENTIALS, audience="", resource="", **{"header.Custom": "Value"}) - assert mock_request.last_request.text == "grant_type=client_credentials&client_id=client&client_secret=secret&scope=catalog" + assert ( + mock_request.last_request.text + == "grant_type=client_credentials&client_id=client&client_secret=secret_with%3Acolon&scope=catalog" + ) def test_client_version_header(rest_mock: Mocker) -> None: diff --git a/tests/conftest.py b/tests/conftest.py index cca9146c16..cdbaf8c6ae 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -103,10 +103,13 @@ from pyiceberg.io.pyarrow import PyArrowFileIO +# Markers for suites that run separately from the unit tests +NON_UNIT_TEST_MARKERS = {"integration", "s3", "adls", "gcs", "notebook", "benchmark"} + def pytest_collection_modifyitems(items: list[pytest.Item]) -> None: for item in items: - if not any(item.iter_markers()): + if not any(marker.name in NON_UNIT_TEST_MARKERS for marker in item.iter_markers()): item.add_marker("unmarked") From cb037dcc688b4a7c4e6d0166cebb5791fb86830d Mon Sep 17 00:00:00 2001 From: Kevin Liu Date: Fri, 11 Sep 2026 12:32:13 -0700 Subject: [PATCH 2/2] Use explicit unit test marker Rename the synthesized marker to match its semantics and keep the local unit target from collecting the integration test directory. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- .github/workflows/python-ci.yml | 2 +- Makefile | 2 +- pyproject.toml | 2 +- tests/conftest.py | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/python-ci.yml b/.github/workflows/python-ci.yml index ef0db12406..31f40e83d6 100644 --- a/.github/workflows/python-ci.yml +++ b/.github/workflows/python-ci.yml @@ -89,7 +89,7 @@ jobs: - name: Install run: uv sync --all-extras --no-extra hive-kerberos - name: Run unit tests - run: uv run python -m pytest tests/ -m unmarked --ignore=tests/integration -v -x + run: uv run python -m pytest tests/ -m unit --ignore=tests/integration -v -x cibw-dev-env-smoke-test: runs-on: ubuntu-latest diff --git a/Makefile b/Makefile index cff86b5840..69ad33912f 100644 --- a/Makefile +++ b/Makefile @@ -104,7 +104,7 @@ lint: ## Run code linters via prek (pre-commit hooks) ##@ Testing test: ## Run all unit tests (excluding integration) - $(TEST_RUNNER) pytest tests/ -m unmarked $(PYTEST_ARGS) + $(TEST_RUNNER) pytest tests/ -m unit --ignore=tests/integration $(PYTEST_ARGS) test-integration: test-integration-setup test-integration-exec test-integration-cleanup ## Run integration tests diff --git a/pyproject.toml b/pyproject.toml index 03c1ee42a9..462d3033da 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -229,7 +229,7 @@ quote-style = "double" testpaths = ["tests"] markers = [ - "unmarked: marks a test as a unittest", + "unit: marks a test as a unit test", "s3: marks a test as requiring access to s3 compliant storage (use with --aws-access-key-id, --aws-secret-access-key, and --endpoint args)", "adls: marks a test as requiring access to adls compliant storage (use with --adls.account-name, --adls.account-key, and --adls.endpoint args)", "integration: marks integration tests against Apache Spark", diff --git a/tests/conftest.py b/tests/conftest.py index cdbaf8c6ae..27505a9cb7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -110,7 +110,7 @@ def pytest_collection_modifyitems(items: list[pytest.Item]) -> None: for item in items: if not any(marker.name in NON_UNIT_TEST_MARKERS for marker in item.iter_markers()): - item.add_marker("unmarked") + item.add_marker("unit") @pytest.fixture(autouse=True, scope="session")