From 0798ce8deb354a7927379367d722c3943f1766e4 Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Wed, 14 Oct 2026 14:48:05 +0300 Subject: [PATCH 1/6] feat(standard-tests): remove the ignored scenario fields and reject keys that are not read `ConnectorTestScenario` declared `expect_records` (with `exact_order`), `timeout_seconds` and `file_types`, and nothing read them: a config that set them looked like it configured a check that never ran. Every other undeclared key was dropped silently too, so a misspelled `empty_streams` was ignored. Remove the three fields and their nested models. An entry of a collected section now fails collection for any key the standard tests do not read, with one line per key naming the key, the section and the config; the removed keys (plus `ignored_fields` and `fail_on_extra_columns`, which only configured CAT's record comparison) say they are no longer read. CAT keys that connectors still set and that have no counterpart yet are accepted only in the section CAT read them from. Validation errors no longer print the input. A guard test fails when a field of the model has no reader in `airbyte_cdk.test`. `bypass_reason` is accepted for the zero-scenario rule (airbyte-internal-issues#17131) and is exempt until that rule reads it. Co-Authored-By: Claude Opus 5.5 --- airbyte_cdk/test/models/scenario.py | 124 +++++- .../test/standard_tests/docker_base.py | 64 ++- cdk-migrations.md | 33 ++ unit_tests/test/test_scenario_keys.py | 410 ++++++++++++++++++ 4 files changed, 608 insertions(+), 23 deletions(-) create mode 100644 unit_tests/test/test_scenario_keys.py diff --git a/airbyte_cdk/test/models/scenario.py b/airbyte_cdk/test/models/scenario.py index 217c28549..9ff6dcfcb 100644 --- a/airbyte_cdk/test/models/scenario.py +++ b/airbyte_cdk/test/models/scenario.py @@ -17,12 +17,12 @@ from typing import TYPE_CHECKING, Any, Literal, cast import yaml -from pydantic import BaseModel, ConfigDict, field_validator +from pydantic import BaseModel, ConfigDict, field_validator, model_validator from airbyte_cdk.test.models.outcome import ExpectedOutcome if TYPE_CHECKING: - from collections.abc import Generator, Mapping + from collections.abc import Generator, Iterable, Mapping ScenarioCommand = Literal["spec", "check", "discover", "read"] @@ -63,6 +63,88 @@ ) """Every command; what a scenario built by hand (no `sections`) runs.""" +_NO_RECORD_COMPARISON = ( + "it configured the Connector Acceptance Tests' comparison of the records read against " + "recorded ones, which the standard tests do not run" +) + +REMOVED_ENTRY_KEYS: Mapping[str, str] = MappingProxyType( + { + "expect_records": _NO_RECORD_COMPARISON, + "exact_order": _NO_RECORD_COMPARISON, + "ignored_fields": _NO_RECORD_COMPARISON, + "fail_on_extra_columns": _NO_RECORD_COMPARISON, + "timeout_seconds": "the standard tests have no per-config timeout", + "file_types": "the standard tests have no check that every supported file type is read", + } +) +"""Keys an `acceptance-test-config.yml` entry can no longer set, with why they are not read. + +They configured checks of the retired Connector Acceptance Tests (CAT) that the standard tests do +not run. `ConnectorTestScenario` declared some of them and ignored them; an entry that still sets +one is now rejected, so a config cannot look like it asks for a check that never runs. +""" + +LEGACY_ENTRY_KEYS: Mapping[str, frozenset[str]] = MappingProxyType( + { + "spec": frozenset({"spec_path", "backward_compatibility_tests_config", "deployment_mode"}), + "connection": frozenset({"deployment_mode"}), + "discovery": frozenset({"backward_compatibility_tests_config"}), + "basic_read": frozenset( + { + "client_container_config", + "expect_trace_message_on_failure", + "validate_state_messages", + } + ), + "full_refresh": frozenset({"client_container_config"}), + } +) +"""CAT keys that an entry of each section may still set, although the scenario does not read them. + +`backward_compatibility_tests_config` of a `spec` entry is read from the file itself by the spec +backward-compatibility test (`disable_for_version`). The others configure CAT checks that have no +standard-test counterpart yet, and connectors still set them. They are accepted, and dropped, only +in the section CAT read them from. Every other key that is not a field of `ConnectorTestScenario` +is rejected (see `entry_key_problems`). +""" + + +def entry_key_problems(entry: Mapping[str, Any], sections: Iterable[str]) -> list[str]: + """Return one message per key of an `acceptance-test-config.yml` entry that is not accepted. + + An entry may set the fields of `ConnectorTestScenario`, and the `LEGACY_ENTRY_KEYS` of the + section that lists it. A key of `REMOVED_ENTRY_KEYS` is reported as no longer read, with the + reason; any other key as unknown. Each message names the key, the section and the entry's + `config_path`, never a value. + """ + sections = tuple(sections) + accepted = set(ConnectorTestScenario.model_fields) + for section in sections: + accepted |= LEGACY_ENTRY_KEYS.get(section, frozenset()) + where = (f"the `{'`/`'.join(sections)}` entry" if sections else "the scenario") + ( + f" for config `{entry['config_path']}`" + if entry.get("config_path") + else " without a `config_path`" + ) + # `sections` and `test_strictness_level` are filled in by `get_scenarios()`, not by entries. + listed = sorted(accepted - {"sections", "test_strictness_level"}) + + problems: list[str] = [] + for key in entry: + if key in accepted: + continue + if key in REMOVED_ENTRY_KEYS: + problems.append( + f"`{key}` in {where} is no longer read: {REMOVED_ENTRY_KEYS[key]}. Remove the key." + ) + else: + problems.append( + f"`{key}` in {where} is not a key the standard tests read. Remove it, or fix " + f"its spelling (accepted: {', '.join(f'`{k}`' for k in listed)})." + ) + return problems + class ConnectorTestScenario(BaseModel): """Acceptance test scenario, as a Pydantic model. @@ -70,21 +152,20 @@ class ConnectorTestScenario(BaseModel): This class represents an acceptance test scenario, which is a single test case that can be run against a connector. It is used to deserialize and validate the acceptance test configuration file. + + Every field is read by the standard tests (a unit test enforces it). An entry that sets a key + that is not a field is rejected, except for the `LEGACY_ENTRY_KEYS` of its section; a key the + standard tests used to accept and ignore is reported as no longer read (`REMOVED_ENTRY_KEYS`). """ # Allows the class to be hashable, which PyTest will require # when we use to parameterize tests. - model_config = ConfigDict(frozen=True) - - class AcceptanceTestExpectRecords(BaseModel): - path: Path - exact_order: bool = False - - class AcceptanceTestFileTypes(BaseModel): - skip_test: bool - bypass_reason: str + # `hide_input_in_errors`: a validation error must not print config values. + model_config = ConfigDict(frozen=True, extra="forbid", hide_input_in_errors=True) class AcceptanceTestEmptyStream(BaseModel): + model_config = ConfigDict(extra="forbid") + name: str bypass_reason: str | None = None @@ -99,10 +180,10 @@ def __hash__(self) -> int: configured_catalog_path: Path | None = None empty_streams: list[AcceptanceTestEmptyStream] | None = None - timeout_seconds: int | None = None - expect_records: AcceptanceTestExpectRecords | None = None - file_types: AcceptanceTestFileTypes | None = None status: Literal["succeed", "failed", "exception"] | None = None + # Why the entry tests nothing. Read by the zero-scenario rule (a connector without test + # scenarios must declare why, airbyte-internal-issues#17131), which is not implemented yet. + bypass_reason: str | None = None # CAT's `basic_read` opt-out from validating records against the stream schemas. validate_schema: bool = True # The connector's top-level `test_strictness_level` in `acceptance-test-config.yml`. Populated @@ -112,6 +193,21 @@ def __hash__(self) -> int: # `get_scenarios()`; empty for scenarios built by hand, which run every command. sections: tuple[str, ...] = () + @model_validator(mode="before") + @classmethod + def _reject_keys_not_read(cls, data: Any) -> Any: + """Reject keys that are not read, and drop the `LEGACY_ENTRY_KEYS` of the entry's section.""" + if not isinstance(data, dict): + return data + sections = data.get("sections") or () + if isinstance(sections, str): + sections = (sections,) + problems = entry_key_problems(data, sections) + if problems: + raise ValueError("\n".join(problems)) + legacy = {key for section in sections for key in LEGACY_ENTRY_KEYS.get(section, ())} + return {key: value for key, value in data.items() if key not in legacy} + @field_validator("sections") @classmethod def _sections_must_be_known(cls, sections: tuple[str, ...]) -> tuple[str, ...]: diff --git a/airbyte_cdk/test/standard_tests/docker_base.py b/airbyte_cdk/test/standard_tests/docker_base.py index dd8b365db..25289dabc 100644 --- a/airbyte_cdk/test/standard_tests/docker_base.py +++ b/airbyte_cdk/test/standard_tests/docker_base.py @@ -17,6 +17,7 @@ import requests import yaml from boltons.typeutils import classproperty +from pydantic import ValidationError from airbyte_cdk.models import ( AirbyteStream, @@ -29,7 +30,11 @@ from airbyte_cdk.models.connector_metadata import MetadataFile from airbyte_cdk.test.entrypoint_wrapper import EntrypointOutput from airbyte_cdk.test.models import ConnectorTestScenario -from airbyte_cdk.test.models.scenario import SCENARIO_SECTIONS, ScenarioCommand +from airbyte_cdk.test.models.scenario import ( + SCENARIO_SECTIONS, + ScenarioCommand, + entry_key_problems, +) from airbyte_cdk.test.standard_tests._assertions import ( assert_check_outcome, assert_config_matches_spec, @@ -391,6 +396,16 @@ def _dedup_scenarios(scenarios: list[ConnectorTestScenario]) -> list[ConnectorTe ) return deduped_scenarios + @classmethod + def _raise_if_invalid(cls, problems: list[str]) -> None: + """Fail collection with every problem found in `acceptance-test-config.yml`, if any.""" + if problems: + config_file = cls.get_connector_root_dir().absolute() / ACCEPTANCE_TEST_CONFIG + raise ValueError( + f"`{config_file}` is not valid for the standard tests:\n" + + "\n".join(f"- {problem}" for problem in problems) + ) + @classmethod def get_scenarios( cls, @@ -404,6 +419,11 @@ def get_scenarios( streams of the config's `configured_catalog_path`, or of every discovered stream when it declares none (`SECTION_COMMANDS`). The `incremental` section is not collected. + An entry of a collected section that sets a key the standard tests do not read fails + collection, with one line per key naming the key, the section and the config, including + entries that are skipped (no `config_path`, or an `iam_role` config). See + `entry_key_problems`. + This has to be a separate function because pytest does not allow parametrization of fixtures with arguments from the test class itself. """ @@ -419,7 +439,7 @@ def get_scenarios( return [] test_strictness_level = all_tests_config.get("test_strictness_level") - test_scenarios: list[ConnectorTestScenario] = [] + entries: list[tuple[str, dict[str, Any]]] = [] for category in SCENARIO_SECTIONS: if ( category not in all_tests_config["acceptance_tests"] @@ -428,14 +448,31 @@ def get_scenarios( continue for test in all_tests_config["acceptance_tests"][category]["tests"]: - if "config_path" not in test: - # Skip tests without a config_path - continue + if isinstance(test, dict): + entries.append((category, test)) + + # Every entry is checked, including those skipped below, so that a key that is not read + # fails collection wherever it is set. + cls._raise_if_invalid( + [ + problem + for category, test in entries + for problem in entry_key_problems(test, (category,)) + ] + ) + + problems: list[str] = [] + test_scenarios: list[ConnectorTestScenario] = [] + for category, test in entries: + if "config_path" not in test: + # Skip tests without a config_path + continue - if "iam_role" in test["config_path"]: - # We skip iam_role tests for now, as they are not supported in the test suite. - continue + if "iam_role" in test["config_path"]: + # We skip iam_role tests for now, as they are not supported in the test suite. + continue + try: scenario = ConnectorTestScenario.model_validate( { **test, @@ -443,8 +480,17 @@ def get_scenarios( "test_strictness_level": test_strictness_level, } ) + except ValidationError as error: + # Reported without the input, which can hold config values. + problems.extend( + f"`{category}` entry for config `{test['config_path']}`: " + f"`{'.'.join(str(part) for part in detail['loc'])}`: {detail['msg']}" + for detail in error.errors() + ) + continue - test_scenarios.append(scenario) + test_scenarios.append(scenario) + cls._raise_if_invalid(problems) deduped_test_scenarios = cls._dedup_scenarios(test_scenarios) diff --git a/cdk-migrations.md b/cdk-migrations.md index ae3781c9b..422b18761 100644 --- a/cdk-migrations.md +++ b/cdk-migrations.md @@ -1,5 +1,38 @@ # CDK Migration Guide +## Upgrading to the next minor release (unreleased) + +### `acceptance-test-config.yml` keys the Standard Tests do not read fail collection + +`ConnectorTestScenario` declared `expect_records` (with `exact_order`), `timeout_seconds` and `file_types`, and no test read them: they parsed, and the check they configured in the retired Connector Acceptance Tests (CAT) never ran. They are removed. Any other key that the model did not declare was dropped without a word, so a misspelled key (`empty_stream` for `empty_streams`) was ignored as well. + +An entry of a collected section (`spec`, `connection`, `discovery`, `basic_read`, `full_refresh`) that sets a key the Standard Tests do not read now fails test collection, with one line per key naming the key, the section and the config: + +``` +`.../source-example/acceptance-test-config.yml` is not valid for the standard tests: +- `expect_records` in the `basic_read` entry for config `secrets/config.json` is no longer read: it configured the Connector Acceptance Tests' comparison of the records read against recorded ones, which the standard tests do not run. Remove the key. +- `timeout_seconds` in the `full_refresh` entry for config `secrets/config.json` is no longer read: the standard tests have no per-config timeout. Remove the key. +``` + +| Key | Status | +|---|---| +| `expect_records`, `exact_order`, `ignored_fields`, `fail_on_extra_columns` | Removed: no record comparison. `ignored_fields` and `fail_on_extra_columns` were never declared on the model; they only configured CAT's comparison. | +| `timeout_seconds` | Removed: no per-config timeout. | +| `file_types` | Removed: no file-type coverage check. | +| `config_path`, `config_dict`, `configured_catalog_path`, `empty_streams` (`name`, `bypass_reason`), `status`, `validate_schema`, `bypass_reason` | Read. `bypass_reason` is reserved for the upcoming rule that a connector without test configs must say why. | +| `spec_path`, `deployment_mode` and `backward_compatibility_tests_config` in `spec`; `deployment_mode` in `connection`; `backward_compatibility_tests_config` in `discovery`; `client_container_config`, `expect_trace_message_on_failure` and `validate_state_messages` in `basic_read`; `client_container_config` in `full_refresh` | Accepted in that section only. `backward_compatibility_tests_config.disable_for_version` of a `spec` entry is read by the spec backward-compatibility test; the others are CAT keys without a Standard Test counterpart yet. | +| Anything else | Rejected as unknown. | + +Entries are checked even when they are not run (no `config_path`, or an `iam_role` config). The `incremental` section is not collected and is not checked. A config value that fails validation (an unknown `status`, for example) is reported the same way, without printing the value. + +Migration steps: + +- Remove the keys the error names from `acceptance-test-config.yml`, and delete the recorded-records files that `expect_records.path` pointed at if nothing else uses them. These are test-only changes and need no version bump. +- Fix the spelling of a misspelled key; the error lists the keys the section accepts. +- Python code that builds `ConnectorTestScenario` with `expect_records`, `timeout_seconds` or `file_types`, or uses `ConnectorTestScenario.AcceptanceTestExpectRecords` or `AcceptanceTestFileTypes`, must drop them. + +Rationale: a key that parses and is ignored reads like a check that runs. A connector with `expect_records` looks like its records are compared against a recorded file, and they are not. Removal is loud rather than silent, and a hard error rather than a deprecation window: the monorepo cleanup that removes these keys from every connector merges before this release, so no connector in the monorepo fails on it. + ## Upgrading to 7.36.0 This release of the CDK adds checks to the Standard Tests (`airbyte-cdk connector test` and `airbyte-cdk image test`) and changes which commands they run for the configs listed in `acceptance-test-config.yml`. There is no change to how connectors run. The release is a minor one, since only the test harness changes, but the connector test suites of connectors that do not satisfy the new expectations start failing. The connector monorepo installs the `airbyte-cdk` CLI unpinned (`uv tool install --upgrade 'airbyte-cdk[dev]'`), so connector CI picks the new behaviour up with this release, without a per-connector CDK bump. diff --git a/unit_tests/test/test_scenario_keys.py b/unit_tests/test/test_scenario_keys.py new file mode 100644 index 000000000..43a90a4cb --- /dev/null +++ b/unit_tests/test/test_scenario_keys.py @@ -0,0 +1,410 @@ +# Copyright (c) 2026 Airbyte, Inc., all rights reserved. +"""`ConnectorTestScenario` declares only keys the standard tests read, and rejects the rest. + +An `acceptance-test-config.yml` entry that sets a key the standard tests do not read fails +collection with a message naming the key, the section and the config, instead of being ignored. +""" + +from __future__ import annotations + +import ast +import shutil +from pathlib import Path +from typing import Any + +import pytest +import yaml +from pydantic import ValidationError + +import airbyte_cdk.test +from airbyte_cdk.test.models import scenario as scenario_module +from airbyte_cdk.test.models.scenario import ( + LEGACY_ENTRY_KEYS, + REMOVED_ENTRY_KEYS, + SCENARIO_SECTIONS, + ConnectorTestScenario, + entry_key_problems, +) +from airbyte_cdk.test.standard_tests.docker_base import DockerConnectorTestSuite +from airbyte_cdk.utils.connector_paths import ACCEPTANCE_TEST_CONFIG + +POKEAPI_RESOURCE_DIR = Path(__file__).parent.parent / "resources" / "source_pokeapi_w_components_py" +SECRET = "hunter2-do-not-print" + +# Fields with no reader yet, each with the change that adds the reader. The guard below fails once +# such a field is read, so the entry is removed by the change that starts reading it. +AWAITING_READER: dict[str, str] = { + "bypass_reason": "the zero-scenario rule, airbyte-internal-issues#17131", +} + + +# --- Guard: every field is read outside the model -------------------------------------------- + + +def _names_used_by_the_harness() -> set[str]: + """Attribute names, keyword names and string constants used in `airbyte_cdk.test`, minus the model. + + Only the test harness consumes scenarios, so the rest of the CDK is not scanned: a parameter + elsewhere that happens to share a field's name must not count as a reader. + """ + harness_root = Path(airbyte_cdk.test.__file__).parent + model_module = Path(scenario_module.__file__) + names: set[str] = set() + for path in harness_root.rglob("*.py"): + if path == model_module: + continue + for node in ast.walk(ast.parse(path.read_text(encoding="utf-8"))): + if isinstance(node, ast.Attribute): + names.add(node.attr) + elif isinstance(node, ast.keyword) and node.arg: + names.add(node.arg) + elif isinstance(node, ast.Constant) and isinstance(node.value, str): + names.add(node.value) + return names + + +def _fields_read_by_each_model_member() -> dict[str, set[str]]: + """For each method and property of `ConnectorTestScenario`, the `self.` it reads. + + A field read only through a member (`config_dict` through `get_config_dict()`) is read + whenever that member is used outside the model. + """ + tree = ast.parse(Path(scenario_module.__file__).read_text(encoding="utf-8")) + model = next( + node + for node in tree.body + if isinstance(node, ast.ClassDef) and node.name == ConnectorTestScenario.__name__ + ) + return { + member.name: { + node.attr + for node in ast.walk(member) + if isinstance(node, ast.Attribute) + and isinstance(node.value, ast.Name) + and node.value.id == "self" + } + for member in model.body + if isinstance(member, ast.FunctionDef) + } + + +def _fields_without_reader() -> set[str]: + used = _names_used_by_the_harness() + read_through_members = { + field + for member, fields in _fields_read_by_each_model_member().items() + if member in used + for field in fields + } + return { + field + for field in ConnectorTestScenario.model_fields + if field not in used and field not in read_through_members + } + + +def test_every_scenario_field_is_read_outside_the_model() -> None: + unread = _fields_without_reader() - set(AWAITING_READER) + + assert not unread, ( + f"`ConnectorTestScenario` declares fields that nothing in `airbyte_cdk.test` reads: " + f"{sorted(unread)}. A config that sets such a field looks like it configures a check " + "that never runs. Read the field in a test, or remove it (and add it to " + "`REMOVED_ENTRY_KEYS` so configs that still set it fail loudly)." + ) + + +@pytest.mark.parametrize("field", sorted(AWAITING_READER)) +def test_fields_awaiting_a_reader_are_still_unread(field: str) -> None: + assert field in ConnectorTestScenario.model_fields + assert field in _fields_without_reader(), ( + f"`{field}` is read now ({AWAITING_READER[field]}): remove it from `AWAITING_READER`." + ) + + +def test_guard_detects_an_unread_field() -> None: + """The guard is not vacuous: the fields this change removed would have failed it.""" + used = _names_used_by_the_harness() + for removed in ("expect_records", "timeout_seconds", "file_types"): + assert removed not in used + + +def test_removed_keys_are_not_fields_nor_legacy_keys() -> None: + assert set(REMOVED_ENTRY_KEYS) == { + "expect_records", + "exact_order", + "ignored_fields", + "fail_on_extra_columns", + "timeout_seconds", + "file_types", + } + assert not set(REMOVED_ENTRY_KEYS) & set(ConnectorTestScenario.model_fields) + for section, keys in LEGACY_ENTRY_KEYS.items(): + assert section in SCENARIO_SECTIONS + assert not keys & set(REMOVED_ENTRY_KEYS), section + assert not keys & set(ConnectorTestScenario.model_fields), section + + +# --- The model rejects keys it does not read ------------------------------------------------- + + +@pytest.mark.parametrize("key", sorted(REMOVED_ENTRY_KEYS)) +@pytest.mark.parametrize("section", ["basic_read", "full_refresh"]) +def test_removed_key_is_rejected_as_no_longer_read(key: str, section: str) -> None: + with pytest.raises(ValidationError) as error: + ConnectorTestScenario.model_validate( + {"config_path": "secrets/config.json", key: {"path": SECRET}, "sections": (section,)} + ) + + message = str(error.value) + assert ( + f"`{key}` in the `{section}` entry for config `secrets/config.json` is no longer read: " + f"{REMOVED_ENTRY_KEYS[key]}. Remove the key." in message + ) + assert SECRET not in message + + +def test_unknown_key_is_rejected() -> None: + with pytest.raises(ValidationError) as error: + ConnectorTestScenario.model_validate( + { + "config_path": "secrets/config.json", + "config_dict": {"api_key": SECRET}, + "empty_stream": [{"name": "users"}], + "sections": ("basic_read",), + } + ) + + message = str(error.value) + assert ( + "`empty_stream` in the `basic_read` entry for config `secrets/config.json` is not a key " + "the standard tests read. Remove it, or fix its spelling (accepted: " in message + ) + assert "`empty_streams`" in message + assert "`sections`" not in message, "filled in by get_scenarios(), not a key to suggest" + assert SECRET not in message + + +def test_unknown_key_of_a_scenario_built_by_hand_is_rejected() -> None: + with pytest.raises(ValidationError, match="`expect_records` in the scenario for config"): + ConnectorTestScenario( # type: ignore[call-arg] + config_path=Path("secrets/config.json"), expect_records={"path": "records.jsonl"} + ) + + +@pytest.mark.parametrize( + "section, key", + [(section, key) for section, keys in LEGACY_ENTRY_KEYS.items() for key in sorted(keys)], +) +def test_legacy_key_is_accepted_and_dropped_in_its_section(section: str, key: str) -> None: + scenario = ConnectorTestScenario.model_validate( + {"config_path": "secrets/config.json", key: {"anything": 1}, "sections": (section,)} + ) + + assert key not in scenario.model_dump() + assert scenario.sections == (section,) + + +def test_legacy_key_is_rejected_outside_its_section() -> None: + with pytest.raises( + ValidationError, + match="`backward_compatibility_tests_config` in the `basic_read` entry .* is not a key", + ): + ConnectorTestScenario.model_validate( + { + "config_path": "secrets/config.json", + "backward_compatibility_tests_config": {"disable_for_version": "1.0.0"}, + "sections": ("basic_read",), + } + ) + + +def test_unknown_key_of_an_empty_stream_is_rejected() -> None: + with pytest.raises(ValidationError, match="empty_streams.0.reason"): + ConnectorTestScenario.model_validate( + { + "config_path": "secrets/config.json", + "empty_streams": [{"name": "users", "reason": "no data"}], + "sections": ("basic_read",), + } + ) + + +def test_bypass_reason_is_accepted() -> None: + scenario = ConnectorTestScenario.model_validate( + {"config_path": "secrets/config.json", "bypass_reason": "no sandbox", "sections": ("spec",)} + ) + assert scenario.bypass_reason == "no sandbox" + + +def test_entry_key_problems_without_config_path() -> None: + assert entry_key_problems({"spec_path": "manifest.yaml", "timeout_seconds": 60}, ["spec"]) == [ + "`timeout_seconds` in the `spec` entry without a `config_path` is no longer read: " + "the standard tests have no per-config timeout. Remove the key." + ] + + +# --- get_scenarios() fails collection with every problem of the file -------------------------- + + +def _suite(tmp_path: Path, acceptance_tests: dict[str, Any]) -> type[DockerConnectorTestSuite]: + connector_root = tmp_path / "source-test" + connector_root.mkdir(exist_ok=True) + (connector_root / ACCEPTANCE_TEST_CONFIG).write_text( + yaml.safe_dump({"acceptance_tests": acceptance_tests}) + ) + shutil.copy(POKEAPI_RESOURCE_DIR / "metadata.yaml", connector_root / "metadata.yaml") + + return type( + "_Suite", + (DockerConnectorTestSuite,), + {"get_connector_root_dir": classmethod(lambda cls: connector_root)}, + ) + + +def test_get_scenarios_reports_every_key_not_read(tmp_path: Path) -> None: + suite = _suite( + tmp_path, + { + "spec": {"tests": [{"spec_path": "manifest.yaml", "timeout_seconds": 60}]}, + "connection": { + "tests": [ + {"config_path": "secrets/config.json", "status": "succeed"}, + { + "config_path": "secrets/config_iam_role.json", + "status": "succeed", + "timeout_seconds": 60, + }, + ] + }, + "basic_read": { + "tests": [ + { + "config_path": "secrets/config.json", + "config_dict": {"api_key": SECRET}, + "expect_records": {"path": "integration_tests/expected_records.jsonl"}, + "fail_on_extra_columns": False, + } + ] + }, + "full_refresh": { + "tests": [ + { + "config_path": "secrets/config.json", + "ignored_fields": {"users": [{"name": "updated_at"}]}, + "unknown_key": SECRET, + } + ] + }, + # Not collected: its keys are not checked. + "incremental": { + "tests": [{"config_path": "secrets/config.json", "timeout_seconds": 3600}] + }, + }, + ) + + with pytest.raises(ValueError) as error: + suite.get_scenarios() + + message = str(error.value) + lines = message.splitlines() + assert lines[0] == ( + f"`{tmp_path / 'source-test' / ACCEPTANCE_TEST_CONFIG}` is not valid for the standard tests:" + ) + assert [line.split(" is ")[0] for line in lines[1:]] == [ + "- `timeout_seconds` in the `spec` entry without a `config_path`", + "- `timeout_seconds` in the `connection` entry for config `secrets/config_iam_role.json`", + "- `expect_records` in the `basic_read` entry for config `secrets/config.json`", + "- `fail_on_extra_columns` in the `basic_read` entry for config `secrets/config.json`", + "- `ignored_fields` in the `full_refresh` entry for config `secrets/config.json`", + "- `unknown_key` in the `full_refresh` entry for config `secrets/config.json`", + ] + assert "no longer read" in lines[1] + assert "is not a key the standard tests read" in lines[-1] + assert SECRET not in message + + +def test_get_scenarios_reports_invalid_values_without_printing_them(tmp_path: Path) -> None: + suite = _suite( + tmp_path, + { + "connection": { + "tests": [ + {"config_path": "secrets/config.json", "status": SECRET}, + {"config_path": "secrets/other.json", "config_dict": SECRET}, + ] + } + }, + ) + + with pytest.raises(ValueError) as error: + suite.get_scenarios() + + message = str(error.value) + assert "- `connection` entry for config `secrets/config.json`: `status`: Input should be" in ( + message + ) + assert "- `connection` entry for config `secrets/other.json`: `config_dict`:" in message + assert SECRET not in message + + +def test_get_scenarios_accepts_the_keys_still_in_use(tmp_path: Path) -> None: + suite = _suite( + tmp_path, + { + "spec": { + "tests": [ + { + "spec_path": "manifest.yaml", + "deployment_mode": "cloud", + "backward_compatibility_tests_config": {"disable_for_version": "1.0.0"}, + }, + {"config_path": "secrets/config.json", "spec_path": "manifest.yaml"}, + ] + }, + "connection": { + "tests": [ + { + "config_path": "secrets/config.json", + "status": "succeed", + "deployment_mode": "oss", + } + ] + }, + "discovery": { + "tests": [ + { + "config_path": "secrets/config.json", + "backward_compatibility_tests_config": {"disable_for_version": "1.0.0"}, + } + ] + }, + "basic_read": { + "tests": [ + { + "config_path": "secrets/config.json", + "configured_catalog_path": "integration_tests/configured_catalog.json", + "empty_streams": [{"name": "users", "bypass_reason": "no data"}], + "validate_schema": False, + "expect_trace_message_on_failure": False, + "validate_state_messages": False, + "client_container_config": {"client_container_dockerfile_path": "x"}, + } + ] + }, + "full_refresh": { + "tests": [ + { + "config_path": "secrets/config.json", + "configured_catalog_path": "integration_tests/configured_catalog.json", + "client_container_config": {"client_container_dockerfile_path": "x"}, + } + ] + }, + }, + ) + + [scenario] = suite.get_scenarios() + + assert scenario.sections == ("spec", "connection", "discovery", "basic_read", "full_refresh") + assert scenario.validate_schema is False From f60f190800af19c685db7b6c3207eda18f1c725b Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Wed, 14 Oct 2026 15:56:29 +0300 Subject: [PATCH 2/6] fix(standard-tests): reject `sections` and `test_strictness_level` on an acceptance-test-config entry `get_scenarios()` fills both fields in itself and overwrote whatever an entry set, so an entry that set them was accepted and silently ignored, which is what this branch removes everywhere else. Report them as not entry keys, with the reason, through `entry_key_problems(from_file=True)`; the model validator, which sees the data `get_scenarios()` built, passes `from_file=False`. No connector sets them today. Also note in the migration guide that the enterprise connectors are cleaned up before the release, alongside the monorepo. Co-Authored-By: Claude Opus 5.5 --- airbyte_cdk/test/models/scenario.py | 47 +++++++++++++--- .../test/standard_tests/docker_base.py | 8 +-- cdk-migrations.md | 3 +- unit_tests/test/test_scenario_keys.py | 54 +++++++++++++++++++ 4 files changed, 99 insertions(+), 13 deletions(-) diff --git a/airbyte_cdk/test/models/scenario.py b/airbyte_cdk/test/models/scenario.py index 9ff6dcfcb..8bc053a86 100644 --- a/airbyte_cdk/test/models/scenario.py +++ b/airbyte_cdk/test/models/scenario.py @@ -110,16 +110,41 @@ """ -def entry_key_problems(entry: Mapping[str, Any], sections: Iterable[str]) -> list[str]: +POPULATED_FIELDS: Mapping[str, str] = MappingProxyType( + { + "sections": "the standard tests set it from the section that lists the entry", + "test_strictness_level": ( + "it is read only at the top level of `acceptance-test-config.yml`" + ), + } +) +"""Fields of `ConnectorTestScenario` that `get_scenarios()` fills in, so an entry cannot set them. + +An entry that set one would be overwritten without a word, so it is rejected, with the reason. +""" + + +def entry_key_problems( + entry: Mapping[str, Any], + sections: Iterable[str], + *, + from_file: bool = True, +) -> list[str]: """Return one message per key of an `acceptance-test-config.yml` entry that is not accepted. - An entry may set the fields of `ConnectorTestScenario`, and the `LEGACY_ENTRY_KEYS` of the - section that lists it. A key of `REMOVED_ENTRY_KEYS` is reported as no longer read, with the - reason; any other key as unknown. Each message names the key, the section and the entry's - `config_path`, never a value. + An entry may set the fields of `ConnectorTestScenario`, except the `POPULATED_FIELDS`, and the + `LEGACY_ENTRY_KEYS` of the section that lists it. A key of `REMOVED_ENTRY_KEYS` is reported as + no longer read, and a key of `POPULATED_FIELDS` as not an entry key, with the reason; any + other key as unknown. Each message names the key, the section and the entry's `config_path`, + never a value. + + `from_file=False` checks the data a scenario is built from (by `get_scenarios()` or by hand), + where the `POPULATED_FIELDS` are accepted. """ sections = tuple(sections) accepted = set(ConnectorTestScenario.model_fields) + if from_file: + accepted -= set(POPULATED_FIELDS) for section in sections: accepted |= LEGACY_ENTRY_KEYS.get(section, frozenset()) where = (f"the `{'`/`'.join(sections)}` entry" if sections else "the scenario") + ( @@ -127,8 +152,7 @@ def entry_key_problems(entry: Mapping[str, Any], sections: Iterable[str]) -> lis if entry.get("config_path") else " without a `config_path`" ) - # `sections` and `test_strictness_level` are filled in by `get_scenarios()`, not by entries. - listed = sorted(accepted - {"sections", "test_strictness_level"}) + listed = sorted(accepted - set(POPULATED_FIELDS)) problems: list[str] = [] for key in entry: @@ -138,6 +162,11 @@ def entry_key_problems(entry: Mapping[str, Any], sections: Iterable[str]) -> lis problems.append( f"`{key}` in {where} is no longer read: {REMOVED_ENTRY_KEYS[key]}. Remove the key." ) + elif key in POPULATED_FIELDS: + problems.append( + f"`{key}` in {where} is not an entry key: {POPULATED_FIELDS[key]}. " + "Remove it from the entry." + ) else: problems.append( f"`{key}` in {where} is not a key the standard tests read. Remove it, or fix " @@ -156,6 +185,8 @@ class ConnectorTestScenario(BaseModel): Every field is read by the standard tests (a unit test enforces it). An entry that sets a key that is not a field is rejected, except for the `LEGACY_ENTRY_KEYS` of its section; a key the standard tests used to accept and ignore is reported as no longer read (`REMOVED_ENTRY_KEYS`). + `get_scenarios()` also rejects an entry that sets one of the `POPULATED_FIELDS`, which it fills + in itself. """ # Allows the class to be hashable, which PyTest will require @@ -202,7 +233,7 @@ def _reject_keys_not_read(cls, data: Any) -> Any: sections = data.get("sections") or () if isinstance(sections, str): sections = (sections,) - problems = entry_key_problems(data, sections) + problems = entry_key_problems(data, sections, from_file=False) if problems: raise ValueError("\n".join(problems)) legacy = {key for section in sections for key in LEGACY_ENTRY_KEYS.get(section, ())} diff --git a/airbyte_cdk/test/standard_tests/docker_base.py b/airbyte_cdk/test/standard_tests/docker_base.py index 25289dabc..b66b11f27 100644 --- a/airbyte_cdk/test/standard_tests/docker_base.py +++ b/airbyte_cdk/test/standard_tests/docker_base.py @@ -419,10 +419,10 @@ def get_scenarios( streams of the config's `configured_catalog_path`, or of every discovered stream when it declares none (`SECTION_COMMANDS`). The `incremental` section is not collected. - An entry of a collected section that sets a key the standard tests do not read fails - collection, with one line per key naming the key, the section and the config, including - entries that are skipped (no `config_path`, or an `iam_role` config). See - `entry_key_problems`. + An entry of a collected section that sets a key the standard tests do not read, or one + they fill in themselves (`sections`, `test_strictness_level`), fails collection, with one + line per key naming the key, the section and the config, including entries that are + skipped (no `config_path`, or an `iam_role` config). See `entry_key_problems`. This has to be a separate function because pytest does not allow parametrization of fixtures with arguments from the test class itself. diff --git a/cdk-migrations.md b/cdk-migrations.md index 422b18761..1e69cec18 100644 --- a/cdk-migrations.md +++ b/cdk-migrations.md @@ -21,6 +21,7 @@ An entry of a collected section (`spec`, `connection`, `discovery`, `basic_read` | `file_types` | Removed: no file-type coverage check. | | `config_path`, `config_dict`, `configured_catalog_path`, `empty_streams` (`name`, `bypass_reason`), `status`, `validate_schema`, `bypass_reason` | Read. `bypass_reason` is reserved for the upcoming rule that a connector without test configs must say why. | | `spec_path`, `deployment_mode` and `backward_compatibility_tests_config` in `spec`; `deployment_mode` in `connection`; `backward_compatibility_tests_config` in `discovery`; `client_container_config`, `expect_trace_message_on_failure` and `validate_state_messages` in `basic_read`; `client_container_config` in `full_refresh` | Accepted in that section only. `backward_compatibility_tests_config.disable_for_version` of a `spec` entry is read by the spec backward-compatibility test; the others are CAT keys without a Standard Test counterpart yet. | +| `sections`, `test_strictness_level` | Rejected on an entry: the Standard Tests fill them in. `sections` comes from the section that lists the entry; `test_strictness_level` is read only at the top level of the file. | | Anything else | Rejected as unknown. | Entries are checked even when they are not run (no `config_path`, or an `iam_role` config). The `incremental` section is not collected and is not checked. A config value that fails validation (an unknown `status`, for example) is reported the same way, without printing the value. @@ -31,7 +32,7 @@ Migration steps: - Fix the spelling of a misspelled key; the error lists the keys the section accepts. - Python code that builds `ConnectorTestScenario` with `expect_records`, `timeout_seconds` or `file_types`, or uses `ConnectorTestScenario.AcceptanceTestExpectRecords` or `AcceptanceTestFileTypes`, must drop them. -Rationale: a key that parses and is ignored reads like a check that runs. A connector with `expect_records` looks like its records are compared against a recorded file, and they are not. Removal is loud rather than silent, and a hard error rather than a deprecation window: the monorepo cleanup that removes these keys from every connector merges before this release, so no connector in the monorepo fails on it. +Rationale: a key that parses and is ignored reads like a check that runs. A connector with `expect_records` looks like its records are compared against a recorded file, and they are not. Removal is loud rather than silent, and a hard error rather than a deprecation window: the cleanup that removes these keys from every Airbyte-maintained connector (the monorepo and the enterprise connectors) merges before this release, so none of them fails on it. ## Upgrading to 7.36.0 diff --git a/unit_tests/test/test_scenario_keys.py b/unit_tests/test/test_scenario_keys.py index 43a90a4cb..1d58f9999 100644 --- a/unit_tests/test/test_scenario_keys.py +++ b/unit_tests/test/test_scenario_keys.py @@ -20,6 +20,7 @@ from airbyte_cdk.test.models import scenario as scenario_module from airbyte_cdk.test.models.scenario import ( LEGACY_ENTRY_KEYS, + POPULATED_FIELDS, REMOVED_ENTRY_KEYS, SCENARIO_SECTIONS, ConnectorTestScenario, @@ -244,6 +245,30 @@ def test_entry_key_problems_without_config_path() -> None: ] +@pytest.mark.parametrize("key", sorted(POPULATED_FIELDS)) +@pytest.mark.parametrize("section", ["spec", "basic_read"]) +def test_populated_field_is_not_an_entry_key(key: str, section: str) -> None: + value = ["full_refresh"] if key == "sections" else "high" + + assert entry_key_problems({"config_path": "secrets/config.json", key: value}, [section]) == [ + f"`{key}` in the `{section}` entry for config `secrets/config.json` is not an entry key: " + f"{POPULATED_FIELDS[key]}. Remove it from the entry." + ] + + +def test_populated_fields_are_accepted_when_building_a_scenario() -> None: + data = { + "config_path": "secrets/config.json", + "sections": ("basic_read",), + "test_strictness_level": "high", + } + + assert entry_key_problems(data, ["basic_read"], from_file=False) == [] + scenario = ConnectorTestScenario.model_validate(data) + assert scenario.sections == ("basic_read",) + assert scenario.test_strictness_level == "high" + + # --- get_scenarios() fails collection with every problem of the file -------------------------- @@ -324,6 +349,35 @@ def test_get_scenarios_reports_every_key_not_read(tmp_path: Path) -> None: assert SECRET not in message +def test_get_scenarios_rejects_entries_setting_populated_fields(tmp_path: Path) -> None: + suite = _suite( + tmp_path, + { + "connection": { + "tests": [ + { + "config_path": "secrets/config.json", + "status": "succeed", + "sections": ["basic_read"], + } + ] + }, + "basic_read": { + "tests": [{"config_path": "secrets/config.json", "test_strictness_level": "low"}] + }, + }, + ) + + with pytest.raises(ValueError) as error: + suite.get_scenarios() + + assert [line.split(" is ")[0] for line in str(error.value).splitlines()[1:]] == [ + "- `sections` in the `connection` entry for config `secrets/config.json`", + "- `test_strictness_level` in the `basic_read` entry for config `secrets/config.json`", + ] + assert "is not an entry key: it is read only at the top level" in str(error.value) + + def test_get_scenarios_reports_invalid_values_without_printing_them(tmp_path: Path) -> None: suite = _suite( tmp_path, From 28ad2e04d4bcd80b11dfc40050869a80d16e35dd Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Wed, 14 Oct 2026 17:06:05 +0300 Subject: [PATCH 3/6] fix(standard-tests): give `ignored_fields` and `fail_on_extra_columns` their real removal reasons `fail_on_extra_columns` never compared anything: CAT accepted it and never enforced it. `ignored_fields` in `full_refresh` fed CAT's sequential-reads test, which compared two consecutive reads, not a recorded file. Co-Authored-By: Claude Opus 5.5 --- airbyte_cdk/test/models/scenario.py | 11 +++++++++-- cdk-migrations.md | 4 +++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/airbyte_cdk/test/models/scenario.py b/airbyte_cdk/test/models/scenario.py index 8bc053a86..a12accd9f 100644 --- a/airbyte_cdk/test/models/scenario.py +++ b/airbyte_cdk/test/models/scenario.py @@ -72,8 +72,15 @@ { "expect_records": _NO_RECORD_COMPARISON, "exact_order": _NO_RECORD_COMPARISON, - "ignored_fields": _NO_RECORD_COMPARISON, - "fail_on_extra_columns": _NO_RECORD_COMPARISON, + "ignored_fields": ( + "it listed the fields the Connector Acceptance Tests skipped when they compared " + "records, against `expect_records` or between two consecutive full-refresh reads; " + "the standard tests run neither comparison" + ), + "fail_on_extra_columns": ( + "the Connector Acceptance Tests accepted it but never enforced it, and the standard " + "tests have no such check" + ), "timeout_seconds": "the standard tests have no per-config timeout", "file_types": "the standard tests have no check that every supported file type is read", } diff --git a/cdk-migrations.md b/cdk-migrations.md index 1e69cec18..4f8947a3f 100644 --- a/cdk-migrations.md +++ b/cdk-migrations.md @@ -16,7 +16,9 @@ An entry of a collected section (`spec`, `connection`, `discovery`, `basic_read` | Key | Status | |---|---| -| `expect_records`, `exact_order`, `ignored_fields`, `fail_on_extra_columns` | Removed: no record comparison. `ignored_fields` and `fail_on_extra_columns` were never declared on the model; they only configured CAT's comparison. | +| `expect_records`, `exact_order` | Removed: no comparison against recorded records. | +| `ignored_fields` | Removed: it listed fields CAT skipped when comparing records, against `expect_records` or between two consecutive full-refresh reads, and the Standard Tests run neither comparison. It was never declared on the model. | +| `fail_on_extra_columns` | Removed: CAT accepted it but never enforced it, and the Standard Tests have no such check. It was never declared on the model. | | `timeout_seconds` | Removed: no per-config timeout. | | `file_types` | Removed: no file-type coverage check. | | `config_path`, `config_dict`, `configured_catalog_path`, `empty_streams` (`name`, `bypass_reason`), `status`, `validate_schema`, `bypass_reason` | Read. `bypass_reason` is reserved for the upcoming rule that a connector without test configs must say why. | From cb766e537258172e3ca7eec010f4ac4f0e46e24b Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Wed, 14 Oct 2026 17:18:06 +0300 Subject: [PATCH 4/6] feat(standard-tests): accept the CAT keys connectors still set instead of rejecting them Rejecting expect_records, ignored_fields, fail_on_extra_columns, timeout_seconds and file_types forced a cleanup of 123 connectors, each needing a version bump, before this release. They are now accepted, and dropped, in the section CAT read them from, like the other legacy keys. Only unknown keys, legacy keys in another section and the fields get_scenarios() fills in are rejected; no connector on airbyte or enterprise master sets one. The recorded-records files stay. Co-Authored-By: Claude Opus 5.5 --- airbyte_cdk/test/models/scenario.py | 83 +++++++++++---------------- cdk-migrations.md | 29 +++++----- unit_tests/test/test_scenario_keys.py | 78 +++++++++++++++---------- 3 files changed, 96 insertions(+), 94 deletions(-) diff --git a/airbyte_cdk/test/models/scenario.py b/airbyte_cdk/test/models/scenario.py index a12accd9f..dd8906238 100644 --- a/airbyte_cdk/test/models/scenario.py +++ b/airbyte_cdk/test/models/scenario.py @@ -63,57 +63,50 @@ ) """Every command; what a scenario built by hand (no `sections`) runs.""" -_NO_RECORD_COMPARISON = ( - "it configured the Connector Acceptance Tests' comparison of the records read against " - "recorded ones, which the standard tests do not run" -) - -REMOVED_ENTRY_KEYS: Mapping[str, str] = MappingProxyType( - { - "expect_records": _NO_RECORD_COMPARISON, - "exact_order": _NO_RECORD_COMPARISON, - "ignored_fields": ( - "it listed the fields the Connector Acceptance Tests skipped when they compared " - "records, against `expect_records` or between two consecutive full-refresh reads; " - "the standard tests run neither comparison" - ), - "fail_on_extra_columns": ( - "the Connector Acceptance Tests accepted it but never enforced it, and the standard " - "tests have no such check" - ), - "timeout_seconds": "the standard tests have no per-config timeout", - "file_types": "the standard tests have no check that every supported file type is read", - } -) -"""Keys an `acceptance-test-config.yml` entry can no longer set, with why they are not read. - -They configured checks of the retired Connector Acceptance Tests (CAT) that the standard tests do -not run. `ConnectorTestScenario` declared some of them and ignored them; an entry that still sets -one is now rejected, so a config cannot look like it asks for a check that never runs. -""" - LEGACY_ENTRY_KEYS: Mapping[str, frozenset[str]] = MappingProxyType( { - "spec": frozenset({"spec_path", "backward_compatibility_tests_config", "deployment_mode"}), - "connection": frozenset({"deployment_mode"}), - "discovery": frozenset({"backward_compatibility_tests_config"}), + "spec": frozenset( + { + "spec_path", + "backward_compatibility_tests_config", + "deployment_mode", + "timeout_seconds", + } + ), + "connection": frozenset({"deployment_mode", "timeout_seconds"}), + "discovery": frozenset({"backward_compatibility_tests_config", "timeout_seconds"}), "basic_read": frozenset( { "client_container_config", + "expect_records", "expect_trace_message_on_failure", + "fail_on_extra_columns", + "file_types", + "timeout_seconds", "validate_state_messages", } ), - "full_refresh": frozenset({"client_container_config"}), + "full_refresh": frozenset({"client_container_config", "ignored_fields", "timeout_seconds"}), } ) """CAT keys that an entry of each section may still set, although the scenario does not read them. -`backward_compatibility_tests_config` of a `spec` entry is read from the file itself by the spec -backward-compatibility test (`disable_for_version`). The others configure CAT checks that have no -standard-test counterpart yet, and connectors still set them. They are accepted, and dropped, only -in the section CAT read them from. Every other key that is not a field of `ConnectorTestScenario` -is rejected (see `entry_key_problems`). +They configured checks of the retired Connector Acceptance Tests (CAT). Connectors still set them, +so they are accepted, and dropped, only in the section CAT read them from: + +- `backward_compatibility_tests_config` of a `spec` entry is read from the file itself by the spec + backward-compatibility test (`disable_for_version`). +- `expect_records` (with `exact_order` nested) compared the records read against a recorded file, + and `ignored_fields` listed the fields to skip when CAT compared records, against + `expect_records` or between two consecutive full-refresh reads. The standard tests run neither + comparison yet; the recorded files are kept for when they do. +- `fail_on_extra_columns` was accepted by CAT and never enforced. +- `timeout_seconds` raised CAT's per-test timeout; the standard tests have no per-test timeout. +- `file_types` configured CAT's check that every supported file type is read. +- The others configure CAT checks that have no standard-test counterpart yet. + +Every other key that is not a field of `ConnectorTestScenario` is rejected (see +`entry_key_problems`). """ @@ -140,9 +133,8 @@ def entry_key_problems( """Return one message per key of an `acceptance-test-config.yml` entry that is not accepted. An entry may set the fields of `ConnectorTestScenario`, except the `POPULATED_FIELDS`, and the - `LEGACY_ENTRY_KEYS` of the section that lists it. A key of `REMOVED_ENTRY_KEYS` is reported as - no longer read, and a key of `POPULATED_FIELDS` as not an entry key, with the reason; any - other key as unknown. Each message names the key, the section and the entry's `config_path`, + `LEGACY_ENTRY_KEYS` of the section that lists it. A key of `POPULATED_FIELDS` is reported as + not an entry key, with the reason; any other key as unknown. Each message names the key, the section and the entry's `config_path`, never a value. `from_file=False` checks the data a scenario is built from (by `get_scenarios()` or by hand), @@ -165,11 +157,7 @@ def entry_key_problems( for key in entry: if key in accepted: continue - if key in REMOVED_ENTRY_KEYS: - problems.append( - f"`{key}` in {where} is no longer read: {REMOVED_ENTRY_KEYS[key]}. Remove the key." - ) - elif key in POPULATED_FIELDS: + if key in POPULATED_FIELDS: problems.append( f"`{key}` in {where} is not an entry key: {POPULATED_FIELDS[key]}. " "Remove it from the entry." @@ -190,8 +178,7 @@ class ConnectorTestScenario(BaseModel): acceptance test configuration file. Every field is read by the standard tests (a unit test enforces it). An entry that sets a key - that is not a field is rejected, except for the `LEGACY_ENTRY_KEYS` of its section; a key the - standard tests used to accept and ignore is reported as no longer read (`REMOVED_ENTRY_KEYS`). + that is not a field is rejected, except for the `LEGACY_ENTRY_KEYS` of its section. `get_scenarios()` also rejects an entry that sets one of the `POPULATED_FIELDS`, which it fills in itself. """ diff --git a/cdk-migrations.md b/cdk-migrations.md index 4f8947a3f..fae8babbe 100644 --- a/cdk-migrations.md +++ b/cdk-migrations.md @@ -2,40 +2,37 @@ ## Upgrading to the next minor release (unreleased) -### `acceptance-test-config.yml` keys the Standard Tests do not read fail collection +### Unknown `acceptance-test-config.yml` keys fail collection -`ConnectorTestScenario` declared `expect_records` (with `exact_order`), `timeout_seconds` and `file_types`, and no test read them: they parsed, and the check they configured in the retired Connector Acceptance Tests (CAT) never ran. They are removed. Any other key that the model did not declare was dropped without a word, so a misspelled key (`empty_stream` for `empty_streams`) was ignored as well. +`ConnectorTestScenario` declared `expect_records` (with `exact_order`), `timeout_seconds` and `file_types`, and no test read them. These fields are removed from the model. Any key that the model did not declare was dropped without a word, so a misspelled key (`empty_stream` for `empty_streams`) was ignored too. -An entry of a collected section (`spec`, `connection`, `discovery`, `basic_read`, `full_refresh`) that sets a key the Standard Tests do not read now fails test collection, with one line per key naming the key, the section and the config: +An entry of a collected section (`spec`, `connection`, `discovery`, `basic_read`, `full_refresh`) that sets an unknown key now fails test collection, with one line per key naming the key, the section and the config, and listing the keys the section accepts: ``` `.../source-example/acceptance-test-config.yml` is not valid for the standard tests: -- `expect_records` in the `basic_read` entry for config `secrets/config.json` is no longer read: it configured the Connector Acceptance Tests' comparison of the records read against recorded ones, which the standard tests do not run. Remove the key. -- `timeout_seconds` in the `full_refresh` entry for config `secrets/config.json` is no longer read: the standard tests have no per-config timeout. Remove the key. +- `empty_stream` in the `basic_read` entry for config `secrets/config.json` is not a key the standard tests read. Remove it, or fix its spelling (accepted: ...). ``` +Keys of the retired Connector Acceptance Tests (CAT) that connectors still set keep working in the section CAT read them from. The Standard Tests accept them and do not read them, except for `backward_compatibility_tests_config` in `spec`. + | Key | Status | |---|---| -| `expect_records`, `exact_order` | Removed: no comparison against recorded records. | -| `ignored_fields` | Removed: it listed fields CAT skipped when comparing records, against `expect_records` or between two consecutive full-refresh reads, and the Standard Tests run neither comparison. It was never declared on the model. | -| `fail_on_extra_columns` | Removed: CAT accepted it but never enforced it, and the Standard Tests have no such check. It was never declared on the model. | -| `timeout_seconds` | Removed: no per-config timeout. | -| `file_types` | Removed: no file-type coverage check. | | `config_path`, `config_dict`, `configured_catalog_path`, `empty_streams` (`name`, `bypass_reason`), `status`, `validate_schema`, `bypass_reason` | Read. `bypass_reason` is reserved for the upcoming rule that a connector without test configs must say why. | -| `spec_path`, `deployment_mode` and `backward_compatibility_tests_config` in `spec`; `deployment_mode` in `connection`; `backward_compatibility_tests_config` in `discovery`; `client_container_config`, `expect_trace_message_on_failure` and `validate_state_messages` in `basic_read`; `client_container_config` in `full_refresh` | Accepted in that section only. `backward_compatibility_tests_config.disable_for_version` of a `spec` entry is read by the spec backward-compatibility test; the others are CAT keys without a Standard Test counterpart yet. | +| `backward_compatibility_tests_config` in `spec` | Accepted. Its `disable_for_version` is read by the spec backward-compatibility test. | +| `expect_records` (with `exact_order` nested) in `basic_read`; `ignored_fields` in `full_refresh` | Accepted, not read. They configured CAT's record comparisons (against the recorded file, and between two consecutive full-refresh reads), which the Standard Tests do not run yet. Keep the recorded-records files. | +| `timeout_seconds` in every collected section | Accepted, not read. The Standard Tests have no per-test timeout. | +| `fail_on_extra_columns`, `file_types` in `basic_read` | Accepted, not read. CAT never enforced `fail_on_extra_columns`; the Standard Tests have no file-type coverage check. | +| `spec_path`, `deployment_mode` in `spec`; `deployment_mode` in `connection`; `backward_compatibility_tests_config` in `discovery`; `client_container_config`, `expect_trace_message_on_failure`, `validate_state_messages` in `basic_read`; `client_container_config` in `full_refresh` | Accepted, not read: CAT keys without a Standard Test counterpart yet. | | `sections`, `test_strictness_level` | Rejected on an entry: the Standard Tests fill them in. `sections` comes from the section that lists the entry; `test_strictness_level` is read only at the top level of the file. | -| Anything else | Rejected as unknown. | +| Any of the CAT keys above in another section, or anything else | Rejected as unknown. | Entries are checked even when they are not run (no `config_path`, or an `iam_role` config). The `incremental` section is not collected and is not checked. A config value that fails validation (an unknown `status`, for example) is reported the same way, without printing the value. -Migration steps: +No Airbyte-maintained connector sets a rejected key, so none needs a change. Migration steps: -- Remove the keys the error names from `acceptance-test-config.yml`, and delete the recorded-records files that `expect_records.path` pointed at if nothing else uses them. These are test-only changes and need no version bump. - Fix the spelling of a misspelled key; the error lists the keys the section accepts. - Python code that builds `ConnectorTestScenario` with `expect_records`, `timeout_seconds` or `file_types`, or uses `ConnectorTestScenario.AcceptanceTestExpectRecords` or `AcceptanceTestFileTypes`, must drop them. -Rationale: a key that parses and is ignored reads like a check that runs. A connector with `expect_records` looks like its records are compared against a recorded file, and they are not. Removal is loud rather than silent, and a hard error rather than a deprecation window: the cleanup that removes these keys from every Airbyte-maintained connector (the monorepo and the enterprise connectors) merges before this release, so none of them fails on it. - ## Upgrading to 7.36.0 This release of the CDK adds checks to the Standard Tests (`airbyte-cdk connector test` and `airbyte-cdk image test`) and changes which commands they run for the configs listed in `acceptance-test-config.yml`. There is no change to how connectors run. The release is a minor one, since only the test harness changes, but the connector test suites of connectors that do not satisfy the new expectations start failing. The connector monorepo installs the `airbyte-cdk` CLI unpinned (`uv tool install --upgrade 'airbyte-cdk[dev]'`), so connector CI picks the new behaviour up with this release, without a per-connector CDK bump. diff --git a/unit_tests/test/test_scenario_keys.py b/unit_tests/test/test_scenario_keys.py index 1d58f9999..f2c1b7224 100644 --- a/unit_tests/test/test_scenario_keys.py +++ b/unit_tests/test/test_scenario_keys.py @@ -21,7 +21,6 @@ from airbyte_cdk.test.models.scenario import ( LEGACY_ENTRY_KEYS, POPULATED_FIELDS, - REMOVED_ENTRY_KEYS, SCENARIO_SECTIONS, ConnectorTestScenario, entry_key_problems, @@ -110,8 +109,8 @@ def test_every_scenario_field_is_read_outside_the_model() -> None: assert not unread, ( f"`ConnectorTestScenario` declares fields that nothing in `airbyte_cdk.test` reads: " f"{sorted(unread)}. A config that sets such a field looks like it configures a check " - "that never runs. Read the field in a test, or remove it (and add it to " - "`REMOVED_ENTRY_KEYS` so configs that still set it fail loudly)." + "that never runs. Read the field in a test, or remove it (and add it to the " + "`LEGACY_ENTRY_KEYS` of the sections whose configs still set it)." ) @@ -130,28 +129,30 @@ def test_guard_detects_an_unread_field() -> None: assert removed not in used -def test_removed_keys_are_not_fields_nor_legacy_keys() -> None: - assert set(REMOVED_ENTRY_KEYS) == { - "expect_records", - "exact_order", - "ignored_fields", - "fail_on_extra_columns", - "timeout_seconds", - "file_types", - } - assert not set(REMOVED_ENTRY_KEYS) & set(ConnectorTestScenario.model_fields) +def test_legacy_keys_are_not_fields() -> None: for section, keys in LEGACY_ENTRY_KEYS.items(): assert section in SCENARIO_SECTIONS - assert not keys & set(REMOVED_ENTRY_KEYS), section assert not keys & set(ConnectorTestScenario.model_fields), section +def test_removed_fields_are_legacy_keys_of_their_cat_sections() -> None: + """Configs that set the removed fields keep collecting; nothing reads them.""" + for key in ("expect_records", "fail_on_extra_columns", "file_types", "timeout_seconds"): + assert key in LEGACY_ENTRY_KEYS["basic_read"] + assert "ignored_fields" in LEGACY_ENTRY_KEYS["full_refresh"] + for section in SCENARIO_SECTIONS: + if section in LEGACY_ENTRY_KEYS: + assert "timeout_seconds" in LEGACY_ENTRY_KEYS[section], section + + # --- The model rejects keys it does not read ------------------------------------------------- -@pytest.mark.parametrize("key", sorted(REMOVED_ENTRY_KEYS)) -@pytest.mark.parametrize("section", ["basic_read", "full_refresh"]) -def test_removed_key_is_rejected_as_no_longer_read(key: str, section: str) -> None: +@pytest.mark.parametrize( + "section, key", + [("full_refresh", "expect_records"), ("basic_read", "ignored_fields"), ("spec", "file_types")], +) +def test_legacy_key_of_another_section_is_rejected(section: str, key: str) -> None: with pytest.raises(ValidationError) as error: ConnectorTestScenario.model_validate( {"config_path": "secrets/config.json", key: {"path": SECRET}, "sections": (section,)} @@ -159,8 +160,8 @@ def test_removed_key_is_rejected_as_no_longer_read(key: str, section: str) -> No message = str(error.value) assert ( - f"`{key}` in the `{section}` entry for config `secrets/config.json` is no longer read: " - f"{REMOVED_ENTRY_KEYS[key]}. Remove the key." in message + f"`{key}` in the `{section}` entry for config `secrets/config.json` is not a key the " + "standard tests read." in message ) assert SECRET not in message @@ -239,10 +240,14 @@ def test_bypass_reason_is_accepted() -> None: def test_entry_key_problems_without_config_path() -> None: - assert entry_key_problems({"spec_path": "manifest.yaml", "timeout_seconds": 60}, ["spec"]) == [ - "`timeout_seconds` in the `spec` entry without a `config_path` is no longer read: " - "the standard tests have no per-config timeout. Remove the key." - ] + [problem] = entry_key_problems( + {"spec_path": "manifest.yaml", "timeout_seconds": 60, "spec_pth": "manifest.yaml"}, + ["spec"], + ) + assert problem.startswith( + "`spec_pth` in the `spec` entry without a `config_path` is not a key the standard tests " + "read. Remove it, or fix its spelling (accepted: " + ) @pytest.mark.parametrize("key", sorted(POPULATED_FIELDS)) @@ -291,7 +296,9 @@ def test_get_scenarios_reports_every_key_not_read(tmp_path: Path) -> None: suite = _suite( tmp_path, { - "spec": {"tests": [{"spec_path": "manifest.yaml", "timeout_seconds": 60}]}, + "spec": { + "tests": [{"spec_path": "manifest.yaml", "timeout_seconds": 60, "spec_pth": "x"}] + }, "connection": { "tests": [ {"config_path": "secrets/config.json", "status": "succeed"}, @@ -299,6 +306,7 @@ def test_get_scenarios_reports_every_key_not_read(tmp_path: Path) -> None: "config_path": "secrets/config_iam_role.json", "status": "succeed", "timeout_seconds": 60, + "deployment_mod": "cloud", }, ] }, @@ -309,6 +317,7 @@ def test_get_scenarios_reports_every_key_not_read(tmp_path: Path) -> None: "config_dict": {"api_key": SECRET}, "expect_records": {"path": "integration_tests/expected_records.jsonl"}, "fail_on_extra_columns": False, + "ignored_fields": {"users": [{"name": "updated_at"}]}, } ] }, @@ -337,14 +346,11 @@ def test_get_scenarios_reports_every_key_not_read(tmp_path: Path) -> None: f"`{tmp_path / 'source-test' / ACCEPTANCE_TEST_CONFIG}` is not valid for the standard tests:" ) assert [line.split(" is ")[0] for line in lines[1:]] == [ - "- `timeout_seconds` in the `spec` entry without a `config_path`", - "- `timeout_seconds` in the `connection` entry for config `secrets/config_iam_role.json`", - "- `expect_records` in the `basic_read` entry for config `secrets/config.json`", - "- `fail_on_extra_columns` in the `basic_read` entry for config `secrets/config.json`", - "- `ignored_fields` in the `full_refresh` entry for config `secrets/config.json`", + "- `spec_pth` in the `spec` entry without a `config_path`", + "- `deployment_mod` in the `connection` entry for config `secrets/config_iam_role.json`", + "- `ignored_fields` in the `basic_read` entry for config `secrets/config.json`", "- `unknown_key` in the `full_refresh` entry for config `secrets/config.json`", ] - assert "no longer read" in lines[1] assert "is not a key the standard tests read" in lines[-1] assert SECRET not in message @@ -412,6 +418,7 @@ def test_get_scenarios_accepts_the_keys_still_in_use(tmp_path: Path) -> None: "spec_path": "manifest.yaml", "deployment_mode": "cloud", "backward_compatibility_tests_config": {"disable_for_version": "1.0.0"}, + "timeout_seconds": 60, }, {"config_path": "secrets/config.json", "spec_path": "manifest.yaml"}, ] @@ -422,6 +429,7 @@ def test_get_scenarios_accepts_the_keys_still_in_use(tmp_path: Path) -> None: "config_path": "secrets/config.json", "status": "succeed", "deployment_mode": "oss", + "timeout_seconds": 60, } ] }, @@ -430,6 +438,7 @@ def test_get_scenarios_accepts_the_keys_still_in_use(tmp_path: Path) -> None: { "config_path": "secrets/config.json", "backward_compatibility_tests_config": {"disable_for_version": "1.0.0"}, + "timeout_seconds": 60, } ] }, @@ -443,6 +452,13 @@ def test_get_scenarios_accepts_the_keys_still_in_use(tmp_path: Path) -> None: "expect_trace_message_on_failure": False, "validate_state_messages": False, "client_container_config": {"client_container_dockerfile_path": "x"}, + "expect_records": { + "path": "integration_tests/expected_records.jsonl", + "exact_order": True, + }, + "fail_on_extra_columns": False, + "file_types": {"skip_test": True, "bypass_reason": "no files"}, + "timeout_seconds": 3600, } ] }, @@ -452,6 +468,8 @@ def test_get_scenarios_accepts_the_keys_still_in_use(tmp_path: Path) -> None: "config_path": "secrets/config.json", "configured_catalog_path": "integration_tests/configured_catalog.json", "client_container_config": {"client_container_dockerfile_path": "x"}, + "ignored_fields": {"users": [{"name": "updated_at"}]}, + "timeout_seconds": 3600, } ] }, From c610a13e13a40ea723dec41efc5c09666834af98 Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Wed, 14 Oct 2026 18:26:01 +0300 Subject: [PATCH 5/6] Revert "feat(standard-tests): accept the CAT keys connectors still set instead of rejecting them" This reverts commit cb766e537258172e3ca7eec010f4ac4f0e46e24b. The old keys are cleaned up from the connectors instead; the recorded-records files are kept. Co-Authored-By: Claude Opus 5.5 --- airbyte_cdk/test/models/scenario.py | 83 ++++++++++++++++----------- cdk-migrations.md | 29 +++++----- unit_tests/test/test_scenario_keys.py | 78 ++++++++++--------------- 3 files changed, 94 insertions(+), 96 deletions(-) diff --git a/airbyte_cdk/test/models/scenario.py b/airbyte_cdk/test/models/scenario.py index dd8906238..a12accd9f 100644 --- a/airbyte_cdk/test/models/scenario.py +++ b/airbyte_cdk/test/models/scenario.py @@ -63,50 +63,57 @@ ) """Every command; what a scenario built by hand (no `sections`) runs.""" -LEGACY_ENTRY_KEYS: Mapping[str, frozenset[str]] = MappingProxyType( +_NO_RECORD_COMPARISON = ( + "it configured the Connector Acceptance Tests' comparison of the records read against " + "recorded ones, which the standard tests do not run" +) + +REMOVED_ENTRY_KEYS: Mapping[str, str] = MappingProxyType( { - "spec": frozenset( - { - "spec_path", - "backward_compatibility_tests_config", - "deployment_mode", - "timeout_seconds", - } + "expect_records": _NO_RECORD_COMPARISON, + "exact_order": _NO_RECORD_COMPARISON, + "ignored_fields": ( + "it listed the fields the Connector Acceptance Tests skipped when they compared " + "records, against `expect_records` or between two consecutive full-refresh reads; " + "the standard tests run neither comparison" + ), + "fail_on_extra_columns": ( + "the Connector Acceptance Tests accepted it but never enforced it, and the standard " + "tests have no such check" ), - "connection": frozenset({"deployment_mode", "timeout_seconds"}), - "discovery": frozenset({"backward_compatibility_tests_config", "timeout_seconds"}), + "timeout_seconds": "the standard tests have no per-config timeout", + "file_types": "the standard tests have no check that every supported file type is read", + } +) +"""Keys an `acceptance-test-config.yml` entry can no longer set, with why they are not read. + +They configured checks of the retired Connector Acceptance Tests (CAT) that the standard tests do +not run. `ConnectorTestScenario` declared some of them and ignored them; an entry that still sets +one is now rejected, so a config cannot look like it asks for a check that never runs. +""" + +LEGACY_ENTRY_KEYS: Mapping[str, frozenset[str]] = MappingProxyType( + { + "spec": frozenset({"spec_path", "backward_compatibility_tests_config", "deployment_mode"}), + "connection": frozenset({"deployment_mode"}), + "discovery": frozenset({"backward_compatibility_tests_config"}), "basic_read": frozenset( { "client_container_config", - "expect_records", "expect_trace_message_on_failure", - "fail_on_extra_columns", - "file_types", - "timeout_seconds", "validate_state_messages", } ), - "full_refresh": frozenset({"client_container_config", "ignored_fields", "timeout_seconds"}), + "full_refresh": frozenset({"client_container_config"}), } ) """CAT keys that an entry of each section may still set, although the scenario does not read them. -They configured checks of the retired Connector Acceptance Tests (CAT). Connectors still set them, -so they are accepted, and dropped, only in the section CAT read them from: - -- `backward_compatibility_tests_config` of a `spec` entry is read from the file itself by the spec - backward-compatibility test (`disable_for_version`). -- `expect_records` (with `exact_order` nested) compared the records read against a recorded file, - and `ignored_fields` listed the fields to skip when CAT compared records, against - `expect_records` or between two consecutive full-refresh reads. The standard tests run neither - comparison yet; the recorded files are kept for when they do. -- `fail_on_extra_columns` was accepted by CAT and never enforced. -- `timeout_seconds` raised CAT's per-test timeout; the standard tests have no per-test timeout. -- `file_types` configured CAT's check that every supported file type is read. -- The others configure CAT checks that have no standard-test counterpart yet. - -Every other key that is not a field of `ConnectorTestScenario` is rejected (see -`entry_key_problems`). +`backward_compatibility_tests_config` of a `spec` entry is read from the file itself by the spec +backward-compatibility test (`disable_for_version`). The others configure CAT checks that have no +standard-test counterpart yet, and connectors still set them. They are accepted, and dropped, only +in the section CAT read them from. Every other key that is not a field of `ConnectorTestScenario` +is rejected (see `entry_key_problems`). """ @@ -133,8 +140,9 @@ def entry_key_problems( """Return one message per key of an `acceptance-test-config.yml` entry that is not accepted. An entry may set the fields of `ConnectorTestScenario`, except the `POPULATED_FIELDS`, and the - `LEGACY_ENTRY_KEYS` of the section that lists it. A key of `POPULATED_FIELDS` is reported as - not an entry key, with the reason; any other key as unknown. Each message names the key, the section and the entry's `config_path`, + `LEGACY_ENTRY_KEYS` of the section that lists it. A key of `REMOVED_ENTRY_KEYS` is reported as + no longer read, and a key of `POPULATED_FIELDS` as not an entry key, with the reason; any + other key as unknown. Each message names the key, the section and the entry's `config_path`, never a value. `from_file=False` checks the data a scenario is built from (by `get_scenarios()` or by hand), @@ -157,7 +165,11 @@ def entry_key_problems( for key in entry: if key in accepted: continue - if key in POPULATED_FIELDS: + if key in REMOVED_ENTRY_KEYS: + problems.append( + f"`{key}` in {where} is no longer read: {REMOVED_ENTRY_KEYS[key]}. Remove the key." + ) + elif key in POPULATED_FIELDS: problems.append( f"`{key}` in {where} is not an entry key: {POPULATED_FIELDS[key]}. " "Remove it from the entry." @@ -178,7 +190,8 @@ class ConnectorTestScenario(BaseModel): acceptance test configuration file. Every field is read by the standard tests (a unit test enforces it). An entry that sets a key - that is not a field is rejected, except for the `LEGACY_ENTRY_KEYS` of its section. + that is not a field is rejected, except for the `LEGACY_ENTRY_KEYS` of its section; a key the + standard tests used to accept and ignore is reported as no longer read (`REMOVED_ENTRY_KEYS`). `get_scenarios()` also rejects an entry that sets one of the `POPULATED_FIELDS`, which it fills in itself. """ diff --git a/cdk-migrations.md b/cdk-migrations.md index fae8babbe..4f8947a3f 100644 --- a/cdk-migrations.md +++ b/cdk-migrations.md @@ -2,37 +2,40 @@ ## Upgrading to the next minor release (unreleased) -### Unknown `acceptance-test-config.yml` keys fail collection +### `acceptance-test-config.yml` keys the Standard Tests do not read fail collection -`ConnectorTestScenario` declared `expect_records` (with `exact_order`), `timeout_seconds` and `file_types`, and no test read them. These fields are removed from the model. Any key that the model did not declare was dropped without a word, so a misspelled key (`empty_stream` for `empty_streams`) was ignored too. +`ConnectorTestScenario` declared `expect_records` (with `exact_order`), `timeout_seconds` and `file_types`, and no test read them: they parsed, and the check they configured in the retired Connector Acceptance Tests (CAT) never ran. They are removed. Any other key that the model did not declare was dropped without a word, so a misspelled key (`empty_stream` for `empty_streams`) was ignored as well. -An entry of a collected section (`spec`, `connection`, `discovery`, `basic_read`, `full_refresh`) that sets an unknown key now fails test collection, with one line per key naming the key, the section and the config, and listing the keys the section accepts: +An entry of a collected section (`spec`, `connection`, `discovery`, `basic_read`, `full_refresh`) that sets a key the Standard Tests do not read now fails test collection, with one line per key naming the key, the section and the config: ``` `.../source-example/acceptance-test-config.yml` is not valid for the standard tests: -- `empty_stream` in the `basic_read` entry for config `secrets/config.json` is not a key the standard tests read. Remove it, or fix its spelling (accepted: ...). +- `expect_records` in the `basic_read` entry for config `secrets/config.json` is no longer read: it configured the Connector Acceptance Tests' comparison of the records read against recorded ones, which the standard tests do not run. Remove the key. +- `timeout_seconds` in the `full_refresh` entry for config `secrets/config.json` is no longer read: the standard tests have no per-config timeout. Remove the key. ``` -Keys of the retired Connector Acceptance Tests (CAT) that connectors still set keep working in the section CAT read them from. The Standard Tests accept them and do not read them, except for `backward_compatibility_tests_config` in `spec`. - | Key | Status | |---|---| +| `expect_records`, `exact_order` | Removed: no comparison against recorded records. | +| `ignored_fields` | Removed: it listed fields CAT skipped when comparing records, against `expect_records` or between two consecutive full-refresh reads, and the Standard Tests run neither comparison. It was never declared on the model. | +| `fail_on_extra_columns` | Removed: CAT accepted it but never enforced it, and the Standard Tests have no such check. It was never declared on the model. | +| `timeout_seconds` | Removed: no per-config timeout. | +| `file_types` | Removed: no file-type coverage check. | | `config_path`, `config_dict`, `configured_catalog_path`, `empty_streams` (`name`, `bypass_reason`), `status`, `validate_schema`, `bypass_reason` | Read. `bypass_reason` is reserved for the upcoming rule that a connector without test configs must say why. | -| `backward_compatibility_tests_config` in `spec` | Accepted. Its `disable_for_version` is read by the spec backward-compatibility test. | -| `expect_records` (with `exact_order` nested) in `basic_read`; `ignored_fields` in `full_refresh` | Accepted, not read. They configured CAT's record comparisons (against the recorded file, and between two consecutive full-refresh reads), which the Standard Tests do not run yet. Keep the recorded-records files. | -| `timeout_seconds` in every collected section | Accepted, not read. The Standard Tests have no per-test timeout. | -| `fail_on_extra_columns`, `file_types` in `basic_read` | Accepted, not read. CAT never enforced `fail_on_extra_columns`; the Standard Tests have no file-type coverage check. | -| `spec_path`, `deployment_mode` in `spec`; `deployment_mode` in `connection`; `backward_compatibility_tests_config` in `discovery`; `client_container_config`, `expect_trace_message_on_failure`, `validate_state_messages` in `basic_read`; `client_container_config` in `full_refresh` | Accepted, not read: CAT keys without a Standard Test counterpart yet. | +| `spec_path`, `deployment_mode` and `backward_compatibility_tests_config` in `spec`; `deployment_mode` in `connection`; `backward_compatibility_tests_config` in `discovery`; `client_container_config`, `expect_trace_message_on_failure` and `validate_state_messages` in `basic_read`; `client_container_config` in `full_refresh` | Accepted in that section only. `backward_compatibility_tests_config.disable_for_version` of a `spec` entry is read by the spec backward-compatibility test; the others are CAT keys without a Standard Test counterpart yet. | | `sections`, `test_strictness_level` | Rejected on an entry: the Standard Tests fill them in. `sections` comes from the section that lists the entry; `test_strictness_level` is read only at the top level of the file. | -| Any of the CAT keys above in another section, or anything else | Rejected as unknown. | +| Anything else | Rejected as unknown. | Entries are checked even when they are not run (no `config_path`, or an `iam_role` config). The `incremental` section is not collected and is not checked. A config value that fails validation (an unknown `status`, for example) is reported the same way, without printing the value. -No Airbyte-maintained connector sets a rejected key, so none needs a change. Migration steps: +Migration steps: +- Remove the keys the error names from `acceptance-test-config.yml`, and delete the recorded-records files that `expect_records.path` pointed at if nothing else uses them. These are test-only changes and need no version bump. - Fix the spelling of a misspelled key; the error lists the keys the section accepts. - Python code that builds `ConnectorTestScenario` with `expect_records`, `timeout_seconds` or `file_types`, or uses `ConnectorTestScenario.AcceptanceTestExpectRecords` or `AcceptanceTestFileTypes`, must drop them. +Rationale: a key that parses and is ignored reads like a check that runs. A connector with `expect_records` looks like its records are compared against a recorded file, and they are not. Removal is loud rather than silent, and a hard error rather than a deprecation window: the cleanup that removes these keys from every Airbyte-maintained connector (the monorepo and the enterprise connectors) merges before this release, so none of them fails on it. + ## Upgrading to 7.36.0 This release of the CDK adds checks to the Standard Tests (`airbyte-cdk connector test` and `airbyte-cdk image test`) and changes which commands they run for the configs listed in `acceptance-test-config.yml`. There is no change to how connectors run. The release is a minor one, since only the test harness changes, but the connector test suites of connectors that do not satisfy the new expectations start failing. The connector monorepo installs the `airbyte-cdk` CLI unpinned (`uv tool install --upgrade 'airbyte-cdk[dev]'`), so connector CI picks the new behaviour up with this release, without a per-connector CDK bump. diff --git a/unit_tests/test/test_scenario_keys.py b/unit_tests/test/test_scenario_keys.py index f2c1b7224..1d58f9999 100644 --- a/unit_tests/test/test_scenario_keys.py +++ b/unit_tests/test/test_scenario_keys.py @@ -21,6 +21,7 @@ from airbyte_cdk.test.models.scenario import ( LEGACY_ENTRY_KEYS, POPULATED_FIELDS, + REMOVED_ENTRY_KEYS, SCENARIO_SECTIONS, ConnectorTestScenario, entry_key_problems, @@ -109,8 +110,8 @@ def test_every_scenario_field_is_read_outside_the_model() -> None: assert not unread, ( f"`ConnectorTestScenario` declares fields that nothing in `airbyte_cdk.test` reads: " f"{sorted(unread)}. A config that sets such a field looks like it configures a check " - "that never runs. Read the field in a test, or remove it (and add it to the " - "`LEGACY_ENTRY_KEYS` of the sections whose configs still set it)." + "that never runs. Read the field in a test, or remove it (and add it to " + "`REMOVED_ENTRY_KEYS` so configs that still set it fail loudly)." ) @@ -129,30 +130,28 @@ def test_guard_detects_an_unread_field() -> None: assert removed not in used -def test_legacy_keys_are_not_fields() -> None: +def test_removed_keys_are_not_fields_nor_legacy_keys() -> None: + assert set(REMOVED_ENTRY_KEYS) == { + "expect_records", + "exact_order", + "ignored_fields", + "fail_on_extra_columns", + "timeout_seconds", + "file_types", + } + assert not set(REMOVED_ENTRY_KEYS) & set(ConnectorTestScenario.model_fields) for section, keys in LEGACY_ENTRY_KEYS.items(): assert section in SCENARIO_SECTIONS + assert not keys & set(REMOVED_ENTRY_KEYS), section assert not keys & set(ConnectorTestScenario.model_fields), section -def test_removed_fields_are_legacy_keys_of_their_cat_sections() -> None: - """Configs that set the removed fields keep collecting; nothing reads them.""" - for key in ("expect_records", "fail_on_extra_columns", "file_types", "timeout_seconds"): - assert key in LEGACY_ENTRY_KEYS["basic_read"] - assert "ignored_fields" in LEGACY_ENTRY_KEYS["full_refresh"] - for section in SCENARIO_SECTIONS: - if section in LEGACY_ENTRY_KEYS: - assert "timeout_seconds" in LEGACY_ENTRY_KEYS[section], section - - # --- The model rejects keys it does not read ------------------------------------------------- -@pytest.mark.parametrize( - "section, key", - [("full_refresh", "expect_records"), ("basic_read", "ignored_fields"), ("spec", "file_types")], -) -def test_legacy_key_of_another_section_is_rejected(section: str, key: str) -> None: +@pytest.mark.parametrize("key", sorted(REMOVED_ENTRY_KEYS)) +@pytest.mark.parametrize("section", ["basic_read", "full_refresh"]) +def test_removed_key_is_rejected_as_no_longer_read(key: str, section: str) -> None: with pytest.raises(ValidationError) as error: ConnectorTestScenario.model_validate( {"config_path": "secrets/config.json", key: {"path": SECRET}, "sections": (section,)} @@ -160,8 +159,8 @@ def test_legacy_key_of_another_section_is_rejected(section: str, key: str) -> No message = str(error.value) assert ( - f"`{key}` in the `{section}` entry for config `secrets/config.json` is not a key the " - "standard tests read." in message + f"`{key}` in the `{section}` entry for config `secrets/config.json` is no longer read: " + f"{REMOVED_ENTRY_KEYS[key]}. Remove the key." in message ) assert SECRET not in message @@ -240,14 +239,10 @@ def test_bypass_reason_is_accepted() -> None: def test_entry_key_problems_without_config_path() -> None: - [problem] = entry_key_problems( - {"spec_path": "manifest.yaml", "timeout_seconds": 60, "spec_pth": "manifest.yaml"}, - ["spec"], - ) - assert problem.startswith( - "`spec_pth` in the `spec` entry without a `config_path` is not a key the standard tests " - "read. Remove it, or fix its spelling (accepted: " - ) + assert entry_key_problems({"spec_path": "manifest.yaml", "timeout_seconds": 60}, ["spec"]) == [ + "`timeout_seconds` in the `spec` entry without a `config_path` is no longer read: " + "the standard tests have no per-config timeout. Remove the key." + ] @pytest.mark.parametrize("key", sorted(POPULATED_FIELDS)) @@ -296,9 +291,7 @@ def test_get_scenarios_reports_every_key_not_read(tmp_path: Path) -> None: suite = _suite( tmp_path, { - "spec": { - "tests": [{"spec_path": "manifest.yaml", "timeout_seconds": 60, "spec_pth": "x"}] - }, + "spec": {"tests": [{"spec_path": "manifest.yaml", "timeout_seconds": 60}]}, "connection": { "tests": [ {"config_path": "secrets/config.json", "status": "succeed"}, @@ -306,7 +299,6 @@ def test_get_scenarios_reports_every_key_not_read(tmp_path: Path) -> None: "config_path": "secrets/config_iam_role.json", "status": "succeed", "timeout_seconds": 60, - "deployment_mod": "cloud", }, ] }, @@ -317,7 +309,6 @@ def test_get_scenarios_reports_every_key_not_read(tmp_path: Path) -> None: "config_dict": {"api_key": SECRET}, "expect_records": {"path": "integration_tests/expected_records.jsonl"}, "fail_on_extra_columns": False, - "ignored_fields": {"users": [{"name": "updated_at"}]}, } ] }, @@ -346,11 +337,14 @@ def test_get_scenarios_reports_every_key_not_read(tmp_path: Path) -> None: f"`{tmp_path / 'source-test' / ACCEPTANCE_TEST_CONFIG}` is not valid for the standard tests:" ) assert [line.split(" is ")[0] for line in lines[1:]] == [ - "- `spec_pth` in the `spec` entry without a `config_path`", - "- `deployment_mod` in the `connection` entry for config `secrets/config_iam_role.json`", - "- `ignored_fields` in the `basic_read` entry for config `secrets/config.json`", + "- `timeout_seconds` in the `spec` entry without a `config_path`", + "- `timeout_seconds` in the `connection` entry for config `secrets/config_iam_role.json`", + "- `expect_records` in the `basic_read` entry for config `secrets/config.json`", + "- `fail_on_extra_columns` in the `basic_read` entry for config `secrets/config.json`", + "- `ignored_fields` in the `full_refresh` entry for config `secrets/config.json`", "- `unknown_key` in the `full_refresh` entry for config `secrets/config.json`", ] + assert "no longer read" in lines[1] assert "is not a key the standard tests read" in lines[-1] assert SECRET not in message @@ -418,7 +412,6 @@ def test_get_scenarios_accepts_the_keys_still_in_use(tmp_path: Path) -> None: "spec_path": "manifest.yaml", "deployment_mode": "cloud", "backward_compatibility_tests_config": {"disable_for_version": "1.0.0"}, - "timeout_seconds": 60, }, {"config_path": "secrets/config.json", "spec_path": "manifest.yaml"}, ] @@ -429,7 +422,6 @@ def test_get_scenarios_accepts_the_keys_still_in_use(tmp_path: Path) -> None: "config_path": "secrets/config.json", "status": "succeed", "deployment_mode": "oss", - "timeout_seconds": 60, } ] }, @@ -438,7 +430,6 @@ def test_get_scenarios_accepts_the_keys_still_in_use(tmp_path: Path) -> None: { "config_path": "secrets/config.json", "backward_compatibility_tests_config": {"disable_for_version": "1.0.0"}, - "timeout_seconds": 60, } ] }, @@ -452,13 +443,6 @@ def test_get_scenarios_accepts_the_keys_still_in_use(tmp_path: Path) -> None: "expect_trace_message_on_failure": False, "validate_state_messages": False, "client_container_config": {"client_container_dockerfile_path": "x"}, - "expect_records": { - "path": "integration_tests/expected_records.jsonl", - "exact_order": True, - }, - "fail_on_extra_columns": False, - "file_types": {"skip_test": True, "bypass_reason": "no files"}, - "timeout_seconds": 3600, } ] }, @@ -468,8 +452,6 @@ def test_get_scenarios_accepts_the_keys_still_in_use(tmp_path: Path) -> None: "config_path": "secrets/config.json", "configured_catalog_path": "integration_tests/configured_catalog.json", "client_container_config": {"client_container_dockerfile_path": "x"}, - "ignored_fields": {"users": [{"name": "updated_at"}]}, - "timeout_seconds": 3600, } ] }, From f2af64b1ed2a1e548b8677e1079391c3ca7432d2 Mon Sep 17 00:00:00 2001 From: Anatolii Yatsuk Date: Wed, 14 Oct 2026 18:26:51 +0300 Subject: [PATCH 6/6] docs(standard-tests): keep the recorded-records files when removing `expect_records` Co-Authored-By: Claude Opus 5.5 --- cdk-migrations.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cdk-migrations.md b/cdk-migrations.md index 4f8947a3f..31426600c 100644 --- a/cdk-migrations.md +++ b/cdk-migrations.md @@ -30,7 +30,7 @@ Entries are checked even when they are not run (no `config_path`, or an `iam_rol Migration steps: -- Remove the keys the error names from `acceptance-test-config.yml`, and delete the recorded-records files that `expect_records.path` pointed at if nothing else uses them. These are test-only changes and need no version bump. +- Remove the keys the error names from `acceptance-test-config.yml`. Keep the recorded-records files that `expect_records.path` pointed at: they stay useful as a reference for the records a connector produces. These are test-only changes and need no version bump. - Fix the spelling of a misspelled key; the error lists the keys the section accepts. - Python code that builds `ConnectorTestScenario` with `expect_records`, `timeout_seconds` or `file_types`, or uses `ConnectorTestScenario.AcceptanceTestExpectRecords` or `AcceptanceTestFileTypes`, must drop them.