Skip to content

feat(standard-tests): fail on an empty test-scenario set unless acceptance-test-config.yml declares a bypass_reason - #1228

Draft
Anatolii Yatsuk (tolik0) wants to merge 1 commit into
mainfrom
tolik0/cdk/empty-scenarios-fail
Draft

Anatolii Yatsuk (tolik0) wants to merge 1 commit into
mainfrom
tolik0/cdk/empty-scenarios-fail

Conversation

@tolik0

Copy link
Copy Markdown
Contributor

Stricter standard tests, no runtime change. On the next CDK release, a Python or manifest-only connector whose acceptance-test-config.yml yields no runnable test scenario, or that has no such file, fails collection instead of passing with every scenario test skipped, unless the file declares a bypass_reason. Connector CI installs airbyte-cdk[dev] unpinned, so this reaches every connector at once. On master today that is 34 connectors (9 without a bypass, 25 without the file). Two monorepo PRs fix all of them and must merge first: airbytehq/airbyte#88931 (bypass reasons) and airbytehq/airbyte#88932 (legacy list sections). With both merged, the classifier finds 0 connectors in either group.

What

Implements P0.3 of the CAT → FAST coverage epic (airbytehq/airbyte-internal-issues#17131, under airbytehq/airbyte-internal-issues#17129): an empty test-scenario set must fail, not silently skip.

Until now, pytest_generate_tests parametrized scenario with whatever get_scenarios() returned. An empty list made pytest skip every scenario test as got empty parameter set and the run passed. A connector whose configs were never collected, whose file was missing, or whose layout the reader did not understand looked the same as a tested one.

How

  • Zero scenarios is an error. When no scenario can run and no bypass_reason explains why, collection fails with one message that names the file and says why each section and entry produced nothing (e.g. `acceptance_tests.spec.tests[0]`: no `config_path` and no `bypass_reason` (keys: `spec_path`)), followed by the fix. The error is raised with pytrace=False outside the except block, so it prints once, without a traceback.

  • Explicit, machine-readable bypass. A bypass_reason counts in three places:

    • at the top level of the file: the whole connector. A file holding only this key is the opt-out for a connector without test configs. Combined with runnable configs it is an error.
    • on a section (connection: {bypass_reason: ...}), the layout CAT used and that ~285 connectors have today. It is ignored when the section lists a runnable config, as before.
    • on an entry without config_path. An entry with a config_path always runs.

    Only the collected sections count (spec, connection, discovery, basic_read, full_refresh). An incremental bypass or an IAM-role config does not make a connector "bypassed".

  • Bypasses are visible. Each scenario test gets one extra skipped param per distinct reason ([bypassed-connection-discovery-basic_read-full_refresh], [bypassed-connector]) whose skip message is acceptance-test-config.yml bypasses <scope>: <reason>, and collection prints one line per connector, e.g. source-hibob: acceptance-test-config.yml declares 1 bypass(es), reported as skipped tests: the connector: No test config in GSM for this connector. The reasons land in the JUnit XML.

  • Legacy list-format sections (connection: [...]) are read as the section's tests list, through section_entries(), which the spec backward-compatibility waiver reader from feat(standard-tests): check spec backward compatibility against the published version #1220 now shares.

  • Legacy top-level tests: fails with one message naming the file and the migration (rename to acceptance_tests:, nest entries under tests). Converting those 34 connectors is airbytehq/airbyte-internal-issues#16264.

  • A missing file is an error for Python and manifest-only connectors. Java connectors (language:java in metadata.yaml) are reported as bypassed, since their Gradle suites test them.

  • No values in failure messages. An entry the scenario model rejects (e.g. empty_streams as plain strings) is reported by location, field and rule; pydantic's input values are not printed.

Changes

  • airbyte_cdk/test/standard_tests/_acceptance_test_config.py (new, private): loading, layout errors, scenario and bypass collection, and the empty-set message.
  • docker_base.py: acceptance_test_config and get_scenarios() use it. New get_scenario_bypasses() and describe_empty_scenarios(). get_scenarios() raises MissingAcceptanceTestConfigError (a FileNotFoundError) or AcceptanceTestConfigError (a ValueError) instead of warning, so existing except FileNotFoundError / except ValueError callers keep working.
  • pytest_hooks.py: the failure, the bypass params and the collection line (pytest_report_collectionfinish).
  • scenario.py: one field, ConnectorTestScenario.bypass_reason, holding the reason on the placeholder scenario of a skipped bypass test. Kept minimal on purpose (see Rollout).
  • _spec_compatibility.py: reads the spec section through the shared section_entries().
  • cdk-migrations.md: new ## Upgrading to the next minor release (unreleased) section with a before/now table, the three bypass forms and the migration steps.

Fleet impact

Classifier over the 591 non-archived Python and manifest-only connectors on airbyte master (1e22e3c), and over master plus both fleet PRs:

Group Master count Today After this release, fleet PRs not merged After this release, fleet PRs merged
Runnable scenarios 236 Scenarios run Same scenarios (identical count per connector) Same, 237
Only declared bypasses 285 Every scenario test skipped as an empty parameter set Skipped with the declared reason Same, 319
No runnable config, no bypass 9 Silently skipped Collection fails 0, bypass declared (PR A)
No acceptance-test-config.yml 25 UserWarning, silently skipped Collection fails 0, bypass-only file (PR A)
Legacy list-format sections 3 Sections ignored Configs run: 2 connectors fail (dead or missing GSM configs), 1 config error Converted, runnable configs pass (PR B)
Legacy top-level tests: 34 Collection fails Collection fails, clearer message Unchanged, tracked in airbytehq/airbyte-internal-issues#16264
Invalid entry (expect_records without path) 1 Collection fails Collection fails, no values printed Unchanged; fixed by P1.5 (#1226)

Certified connectors newly declaring a bypass: destination-pinecone (GSM holds a config no entry lists) and source-granola (no GSM config).

The connectors kept in the private enterprise repository, whose CI runs the same standard tests, are not affected: the same classifier finds every Python and manifest-only connector there runnable.

Rollout

Follow-ups

Two minor gaps from review, left for a follow-up so this PR stays as reviewed:

  • A suite that overrides get_scenarios() with in-code scenarios and has no acceptance-test-config.yml now fails collection, because the bypass lookup reads the file. No connector in the monorepo or the CDK overrides get_scenarios(). Fix: treat a missing file as "no bypasses" when get_scenarios() returned scenarios.
  • The YAML parse error includes PyYAML's source snippet of the offending line. Fix: build the message from the problem and the line and column numbers only.

Test plan

  • unit_tests/test/test_empty_scenarios.py (39 tests, pytester runs of the real hooks): every bypass form and its scope; the empty-set message; section bypass ignored next to runnable entries; blank and non-string reasons; top-level bypass with and without runnable entries; legacy list sections; legacy top-level tests:; unusable layouts; missing file for Python, manifest-only and Java; collection failures print once, without a traceback or chained exception; skipped bypass params, ids and grouping; validation errors without values.

  • poetry run pytest unit_tests/test unit_tests/resources: 834 passed, 20 skipped. The skip count depends on the registry: pokeapi's spec backward-compatibility test skips while the registry publishes a newer version than its fixture.

  • poe lint and ruff format --check clean; poe type-check reports no error in the changed files (the 51 existing errors are unchanged against main); Prettier clean on the new migration section.

  • CLI proofs with this branch's airbyte-cdk, on copies of monorepo connectors with GSM secrets fetched by airbyte-cdk secrets fetch:

    Connector Shape Ref Result
    source-pokeapi runnable master connector test exit 0, 9 passed, 3 skipped
    source-100ms section bypasses only master exit 0, 3 passed, 9 skipped with the declared reasons
    source-hibob no runnable config, no bypass master exit 2, "No test scenarios for connector 'source-hibob'"
    source-clazar no file master exit 2, "Acceptance test config file not found"
    source-activecampaign legacy top-level tests: master exit 2, migration message
    source-hibob, source-clazar top-level bypass PR A exit 0, 3 passed, 9 skipped "bypasses the connector: No test config in GSM for this connector"
    destination-pinecone, source-granola top-level bypass PR A collect: bypass line printed
    source-pardot legacy list, converted PR B connector test exit 0 (5 passed, 16 skipped); image test exit 0
    source-google-directory legacy list, converted PR B image test exit 0, 4 passed (discover kept)
    source-netsuite legacy list, converted PR B image test exit 0, 3 passed
  • Classifier: master has 9 + 25 connectors to fix; master plus both fleet PRs has 0 and 0.

🤖 Generated with Claude Code

…tance-test-config.yml declares a bypass_reason

An acceptance-test-config.yml that yielded no runnable config made pytest
skip every scenario test as an empty parameter set, and the run passed. A
connector whose configs were never collected, whose file was missing, or
whose layout the reader did not understand looked the same as a tested one.

Collection now fails when no scenario can run and no bypass_reason says
why. A bypass is accepted at the top level of the file (the whole
connector, and the opt-out for a connector without test configs), on a
section, or on an entry without config_path. Bypasses are reported as
skipped tests carrying the declared reason, grouped by reason, plus one
line after collection.

The reader also:
- parses legacy list-format sections as the section's tests list;
- fails with a migration message on a legacy top-level `tests:` key;
- treats a missing file as an error for Python and manifest-only
  connectors, and as a bypass for Java connectors;
- wraps invalid entries in an error that names the entry and field but
  never prints the offending values.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@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/empty-scenarios-fail#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/empty-scenarios-fail

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.

Comment on lines +123 to +124
"`acceptance_tests.spec.tests[0]`: no `config_path` and no `bypass_reason` "
"(keys: `spec_path`).",
Comment on lines +400 to +401
"*No test scenarios for connector 'source-test': *acceptance-test-config.yml lists no "
"config the standard tests can run, and declares no `bypass_reason`. Found:",
Comment on lines +475 to +476
"SKIPPED [[]1[]] *: acceptance-test-config.yml bypasses the connector: "
"No sandbox account",
Comment on lines +503 to +505
"source-test: acceptance-test-config.yml declares 3 bypass(es), reported as skipped "
"tests: connection.tests[[]1[]], basic_read: The GSM config fails check; "
"full_refresh: Covered by basic_read",
@github-actions

Copy link
Copy Markdown

PyTest Results (Fast)

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

Results for commit 747d79c. ± 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

@github-actions

Copy link
Copy Markdown

PyTest Results (Full)

5 791 tests  +40   5 760 ✅ +39   16m 45s ⏱️ -15s
    1 suites ± 0      31 💤 + 1 
    1 files   ± 0       0 ❌ ± 0 

Results for commit 747d79c. ± 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 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.

1 participant