Skip to content

feat(standard-tests): remove the ignored scenario fields and reject acceptance-test-config keys that are not read - #1226

Draft
Anatolii Yatsuk (tolik0) wants to merge 6 commits into
mainfrom
tolik0/cdk/remove-dead-scenario-fields
Draft

Anatolii Yatsuk (tolik0) wants to merge 6 commits into
mainfrom
tolik0/cdk/remove-dead-scenario-fields

Conversation

@tolik0

@tolik0 Anatolii Yatsuk (tolik0) commented Oct 14, 2026 •

Copy link
Copy Markdown
Contributor

On airbyte master today, 113 connectors (47 certified; 81 manifest-only, 31 Python, and the Java source-mysql, which does not run the standard tests) set at least one acceptance-test-config.yml key that the standard tests parse and ignore. With this release their test collection fails with a message naming each key. Eight cleanup PRs (airbytehq/airbyte#88948, #88949, #88950, #88951, #88952, #88954, #88955 and #88953) remove every such key from the monorepo and keep the recorded-records files; a separate cleanup PR does the same for 4 enterprise connectors kept in a private repository whose CI runs the same standard tests. All of them must merge before this CDK release.

What

ConnectorTestScenario declared expect_records (with its AcceptanceTestExpectRecords model and exact_order), timeout_seconds and file_types (with AcceptanceTestFileTypes). Nothing read them. A connector with expect_records looked like its records were compared against a recorded file, and they were not. Any other undeclared key was also dropped without a word (pydantic's default extra="ignore"), so a misspelled empty_stream silently did nothing.

This PR:

  • removes the three fields and their nested models;
  • rejects every entry key the standard tests do not read, at test collection, with one line per key naming the key, the section and the config;
  • reports the removed keys, plus ignored_fields (fields CAT skipped when comparing records, against expect_records or between two full-refresh reads) and fail_on_extra_columns (accepted by CAT but never enforced), as "no longer read", with the reason;
  • adds a guard test that fails when a field of ConnectorTestScenario has no reader in airbyte_cdk.test.

configured_catalog_path stays: #1142 honours it.

Resolves https://github.com/airbytehq/airbyte-internal-issues/issues/17138 (part of epic airbytehq/airbyte-internal-issues#17129).

How

  • airbyte_cdk/test/models/scenario.py
    • REMOVED_ENTRY_KEYS: the six removed keys, each with why it is not read.
    • LEGACY_ENTRY_KEYS: CAT keys that connectors still set and that the scenario does not read, accepted only in the section CAT read them from. backward_compatibility_tests_config in spec is read from the file by the spec backward-compatibility test (feat(standard-tests): check spec backward compatibility against the published version #1220); the rest have no standard-test counterpart yet and are left to the fleet cleanup (P3.4).
    • entry_key_problems(entry, sections): one message per key that is neither a field nor a legacy key of the section. It never prints a value.
    • POPULATED_FIELDS: sections and test_strictness_level are fields that get_scenarios() fills in (test_strictness_level from the top level of the file). An entry that sets one is rejected as "not an entry key" instead of being overwritten without a word.
    • The model is extra="forbid" and hide_input_in_errors=True. A mode="before" validator raises the entry_key_problems messages and drops the legacy keys. AcceptanceTestEmptyStream is extra="forbid" too.
    • bypass_reason is accepted on an entry. It is read by the zero-scenario rule of P0.3 (airbytehq/airbyte-internal-issues#17131), developed in parallel.
  • airbyte_cdk/test/standard_tests/docker_base.py: get_scenarios() checks every entry of every collected section first, including the entries it skips (no config_path, or an iam_role config). It raises one ValueError listing every problem in the file, prefixed with the file path. A pydantic error on a value (an unknown status, for example) is reported the same way, without the input.
  • Guard test (unit_tests/test/test_scenario_keys.py): scans airbyte_cdk/test (minus the model module) for attribute names, keyword names and string constants. A field counts as read if its name appears there, or if a method or property of the model that reads it (config_dict through get_config_dict()) does. bypass_reason is exempt until P0.3 reads it, and a second test fails once it is read, so the exemption is removed with P0.3.

Example, source-stripe on master:

ValueError: `.../source-stripe/acceptance-test-config.yml` is not valid for the standard tests:
- `fail_on_extra_columns` in the `basic_read` entry for config `secrets/config.json` is no longer read: the Connector Acceptance Tests accepted it but never enforced it, and the standard tests have no such check. Remove the key.
- `timeout_seconds` in the `basic_read` entry for config `secrets/config.json` is no longer read: the standard tests have no per-config timeout. Remove the key.
- `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.
- `ignored_fields` in the `full_refresh` entry for config `secrets/config.json` is no longer read: 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. Remove the key.

Which keys are accepted

From an inventory of every acceptance-test-config.yml on airbyte master (1e22e3c689c, 592 files, 553 with acceptance_tests):

Key Section Entries on master Here
config_path, status, configured_catalog_path, empty_streams, validate_schema collected sections (read) accepted, read
config_dict any 0 (Python callers) accepted, read
bypass_reason any 0 on entries (295 files set it at section level) accepted, read by P0.3
spec_path / backward_compatibility_tests_config / deployment_mode spec 552 / 42 / 2 accepted
deployment_mode connection 1 accepted
backward_compatibility_tests_config discovery 81 accepted
client_container_config, expect_trace_message_on_failure, validate_state_messages basic_read 1 / 5 / 5 accepted
client_container_config full_refresh 1 accepted
expect_records (exact_order nested) basic_read 133 rejected, no longer read
fail_on_extra_columns basic_read 34 rejected, no longer read
ignored_fields full_refresh 26 rejected, no longer read
timeout_seconds every section 140 in collected sections rejected, no longer read
file_types basic_read 23 rejected, no longer read
sections, test_strictness_level any 0 rejected, not entry keys (filled in by get_scenarios())
anything else 0 rejected, unknown

The incremental section is not collected (#1142) and is not checked. Files with the legacy top-level tests: key (39) already fail collection (airbytehq/airbyte-internal-issues#16264); they are untouched here.

Fleet impact

Connectors rejected at collection Certified
airbyte master (1e22e3c689c) 113 47
airbyte with all 8 cleanup PRs applied 0 0
enterprise connectors (private repository), master 4 n/a
enterprise connectors with their cleanup PR 0 n/a

The 4 enterprise connectors live in a private repository whose connector CI calls airbyte's connector-ci-checks.yml@master, which installs the CDK CLI unpinned; they are covered by a separate cleanup PR in that repository.

Measured two ways with this branch: a static run of the check over every file at both refs (static_sweep.py), and airbyte-cdk connector test --collect-only through the CLI over every Python and manifest-only connector of the cleaned branch (584 Python and manifest-only connectors: 549 collect, 35 fail collection for the pre-existing legacy top-level tests: layout (airbytehq/airbyte-internal-issues#16264), 0 are rejected by the key check; 8 Java connectors skipped; 3 runs killed by SIGTERM mid-sweep were re-run individually and collect). The split into 8 PRs applies the same edits; with all 8 merged, the static check rejects 0 of 592 files.

Rollout

  • Hard error, no deprecation window. The 8 monorepo cleanup PRs remove every rejected key and merge first, together with the separate cleanup PR for the 4 enterprise connectors in the private repository; connector CI installs the CDK CLI unpinned, so this release reaches every connector at once (the enterprise repository reuses the same CI workflow).
  • Right before the release, re-run the static check on the latest airbyte master and the enterprise master: a connector PR merged in between can re-add a key.
  • This PR's Test Connectors job is red until the monorepo cleanup merges. It checks out airbyte master and runs airbyte-cdk connector test; 4 of its matrix connectors (source-hardcoded-records, source-shopify, source-google-drive, source-intercom) still set rejected keys there. Test Connectors fails collection for these 4 until their batches merge (test(connectors): remove acceptance-test-config keys the standard tests never read (6/8: community Python) airbyte#88954, #88950, #88953 and #88948); re-run it after.
  • Minor release: only the test harness changes. Migration note in cdk-migrations.md under "Upgrading to the next minor release (unreleased)".
  • Conflicts expected with the P0.3 branch (airbytehq/airbyte-internal-issues#17131), which also changes get_scenarios() and scenario.py: rebase after whichever merges first. When P0.3 reads bypass_reason, remove it from AWAITING_READER in test_scenario_keys.py (a test fails until you do).

Test plan

  • poetry run pytest unit_tests/test/ -p no:cacheprovider: 824 passed, 9 skipped
  • New unit_tests/test/test_scenario_keys.py (40 tests): each removed key in basic_read and full_refresh, an unknown key, a legacy key inside and outside its section, sections / test_strictness_level set on an entry, an unknown empty_streams key, entries without config_path and iam_role entries, a value error without its value, and the guard test.
  • ruff format, ruff check, mypy on the touched modules (mypy airbyte_cdk reports the same 51 pre-existing errors as main).
  • airbyte-cdk connector test --collect-only from this branch: source-linkedin-ads, source-clickup-api, source-coingecko-coins and source-stripe fail collection on master with the messages above, and collect (39/21/21/21 tests) on the cleaned branch.
  • Fleet collection sweep, see Fleet impact.
  • The same CLI collection over the 5 Python and manifest-only enterprise connectors: 4 rejected on their master, all 5 collect with their cleanup branch.
  • Test Connectors fails collection for source-hardcoded-records, source-shopify, source-google-drive and source-intercom until their batches merge; re-run it after.

🤖 Generated with Claude Code

…eys 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 <noreply@anthropic.com>
… 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 <noreply@anthropic.com>
@tolik0
Anatolii Yatsuk (tolik0) requested a review from a team as a code owner October 14, 2026 13:12
@github-actions

Copy link
Copy Markdown

👋 Greetings, Airbyte Team Member!

Here are some helpful tips and reminders for your convenience.

💡 Show Tips and Tricks

Testing This CDK Version

You can test this version of the CDK using the following:

# Run the CLI from this branch:
uvx 'git+https://github.com/airbytehq/airbyte-python-cdk.git@tolik0/cdk/remove-dead-scenario-fields#egg=airbyte-python-cdk[dev]' --help

# Update a connector to use the CDK from this branch ref:
cd airbyte-integrations/connectors/source-example
poe use-cdk-branch tolik0/cdk/remove-dead-scenario-fields

PR Slash Commands

Airbyte Maintainers can execute the following slash commands on your PR:

  • /autofix - Fixes most formatting and linting issues
  • /poetry-lock - Updates poetry.lock file
  • /test - Runs connector tests with the updated CDK
  • /prerelease - Triggers a prerelease publish with default arguments
  • /poe build - Regenerate git-committed build artifacts, such as the pydantic models which are generated from the manifest JSON schema in YAML.
  • /poe <command> - Runs any poe command in the CDK environment
📚 Show Repo Guidance

Helpful Resources

📝 Edit this welcome message.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

PyTest Results (Fast)

5 788 tests  +40   5 757 ✅ +39   10m 47s ⏱️ - 1m 23s
    1 suites ± 0      31 💤 + 1 
    1 files   ± 0       0 ❌ ± 0 

Results for commit f2af64b. ± Comparison against base commit 2082c2d.

This pull request skips 1 test.
unit_tests.resources.source_pokeapi_w_components_py.integration_tests.test_airbyte_standards.TestSuiteSourcePokeAPI ‑ test_docker_image_spec_backward_compatibility

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Oct 14, 2026 •

Copy link
Copy Markdown

PyTest Results (Full)

5 791 tests  +40   5 760 ✅ +39   17m 7s ⏱️ +7s
    1 suites ± 0      31 💤 + 1 
    1 files   ± 0       0 ❌ ± 0 

Results for commit f2af64b. ± Comparison against base commit 2082c2d.

This pull request skips 1 test.
unit_tests.resources.source_pokeapi_w_components_py.integration_tests.test_airbyte_standards.TestSuiteSourcePokeAPI ‑ test_docker_image_spec_backward_compatibility

♻️ This comment has been updated with latest results.

…` 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 <noreply@anthropic.com>
@tolik0
Anatolii Yatsuk (tolik0) marked this pull request as draft October 14, 2026 14:12
…d 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 <noreply@anthropic.com>
@tolik0 Anatolii Yatsuk (tolik0) changed the title feat(standard-tests): remove the ignored scenario fields and reject acceptance-test-config keys that are not read feat(standard-tests): remove the dead scenario fields and reject unknown acceptance-test-config keys Oct 14, 2026
…t instead of rejecting them"

This reverts commit cb766e5. The old keys are
cleaned up from the connectors instead; the recorded-records files are kept.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…expect_records`

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants