Repository navigation
feat(standard-tests): assert per-stream records and record schemas in the basic read test - #1218
Conversation
…n the basic read test `SourceTestSuiteBase.test_basic_read` only failed when the whole read returned no records, so one stream with records passed the test for every stream, and no record was ever checked against its schema. Port the two CAT basic read checks that ran by default: - every stream of a `basic_read` config returns at least one record, except the ones declared in `empty_streams` (which are still not read); - every record matches its stream's JSON schema, with CAT's validator (Draft 7, strict integers, lenient `date-time`, extra columns allowed) and its structure check, unless the config sets `validate_schema: false`. Failure messages name the stream, the number of failing records, and the path of each offending value inside the record. The assertions live in `_read_assertions.py` and take the read output and catalog, so the Docker read test can reuse them once it runs. Part of airbytehq/airbyte-internal-issues#17135. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
👋 Greetings, Airbyte Team Member!Here are some helpful tips and reminders for your convenience. 💡 Show Tips and TricksTesting This CDK VersionYou 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/basic-read-assertions#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/basic-read-assertionsPR Slash CommandsAirbyte Maintainers can execute the following slash commands on your PR:
|
… of printing the value A type error's value can be a whole nested object, which made the failure line long and copied record values into public CI logs. Report "expected type 'number', got string" instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A config listed only under `connection` (for example when `basic_read` is bypassed) was schema validated, but had no `basic_read` entry to put the `validate_schema: false` opt-out in, and CAT never ran a basic read with it. Both record checks now apply only to `basic_read` configs, as in CAT; the others keep the "some records" floor. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`read` now runs only for `basic_read` and `full_refresh` configs, and a config without a `status` expects success. The record checks keep applying to `basic_read` configs only; a config read because of `full_refresh` keeps the "some records" floor, which now runs before the record checks so a read without any records keeps its message. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ages Only top-level `type` errors were rewritten. Every other jsonschema message embeds the failing value: an `anyOf`/`oneOf` error printed the whole nested object, and `not`, `enum`, `const`, `pattern` and `format` printed the value too. These messages end up in public CI logs. Each schema error is now described by its validator keyword, the schema's own scalar constraint (a format name, a pattern, a length) and the JSON type of the value, next to the record number, the record path and the schema rule. For `anyOf`/`oneOf` the closest option's failure is named the same way, preferring options whose type the value has. A schema the validator cannot apply (an unknown `type`, an unresolvable `$ref`, an invalid `pattern`) used to abort the whole check with the raw jsonschema exception, whose text prints the instance. It now fails that stream with the schema problem and stops validating its records, while the other streams are still reported. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…_level: high CAT's basic read test failed a config that set `validate_schema: false` when the connector declared `test_strictness_level: high`, so certified connectors could not drop schema validation by config. The standard tests honoured the opt-out at every strictness level. `get_scenarios()` now carries the connector's top-level `test_strictness_level` into each scenario, and `test_basic_read` fails a `basic_read` config that combines it with `validate_schema: false` before reading, with a message that points at fixing the schema or lowering the strictness level. A config read only because it is listed under `full_refresh` is unaffected, as its records are not validated anyway. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… reads `test_basic_read` built the full list of record messages to check that the read returned any, then re-read and re-parsed the message file for the record assertions. `assert_read_records` now owns the "Expected records but got none." floor, so a `basic_read` config's records are parsed in one pass; a config read only for `full_refresh` stops at the first record. The early return for a read expected to fail (`status: failed` or `exception`) had no test. The basic read matrix now runs a source whose read raises under both statuses, a `failed` config whose read succeeds, and a `succeed` config whose read raises. The `validate_schema` dedup is tested with two `basic_read` entries for the same config, in both orders. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… read assertions The basic read standard test is gaining CAT's record checks (airbytehq/airbyte-python-cdk#1218): every stream in a `basic_read` config must return records, and every record must match its stream's schema. A credentialed dry run of that branch turns these 20 connectors red. - Declare the streams that returned no records under `empty_streams`, each with a `bypass_reason`. - Set `validate_schema: false`, with a comment naming the mismatching streams, for the 9 connectors whose records do not match their schema and that declare no `test_strictness_level`. - source-merge: fix the `job-interview-stages` entry to the real stream name, `job_interview_stages`. Only `acceptance-test-config.yml` changes, so no connector is released. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings in main and the retitled migration section of the base PR. Both sections now sit under one "next minor release (unreleased)" heading, since the change only touches the test harness and ships as a minor. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…sertion' into tolik0/cdk/basic-read-assertions
…sertion' into tolik0/cdk/basic-read-assertions
… read assertions GET_FLAT_FILE_ALL_ORDERS_DATA_BY_LAST_UPDATE_GENERAL returned no records in the test account on 2026-10-09. Declare it under empty_streams so the restored basic read record check in airbytehq/airbyte-python-cdk#1218 does not fail this connector. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Daryna Ishchenko (darynaishchenko)
left a comment
There was a problem hiding this comment.
Approving the change itself. The assertions are a single lazy pass over the records, every branch of test_basic_read and _dedup_scenarios is pinned by a test (all 13 wiring mutants caught, 30/32 overall), and date-time verdicts match CAT's on the shapes I tried. This is BREAKING under A.7 only (a new mandatory fleet-wide test gate). That is called out in the body and in cdk-migrations.md, and feat: (minor) matches #1217.
One thing to settle before merge: the release order, inline. CI is green apart from Check: destination-motherduck, which is red on every recent PR (expired token).
| if scenario.is_basic_read_config: | ||
| # Also asserts that the read returned records, in the same pass over them. | ||
| assert_read_records( | ||
| records=result.records_iterator, | ||
| configured_catalog=configured_catalog, | ||
| require_records_per_stream=True, | ||
| validate_schema=scenario.validate_schema, | ||
| ) |
There was a problem hiding this comment.
🟠 medium: nothing enforces the rollout order, and 22 connectors depend on five open monorepo PRs.
Connector CI installs the CDK CLI unpinned (connector-ci-checks.yml:592, which the migration entry also says). So this check applies to every connector PR from the day of the release. Companion PR state today: #88405 (shopify) and #88159 (customer-io) are merged. Still open: airbytehq/airbyte#88406 (18 connectors), #88477 (amazon-seller-partner), #88407 (freshdesk), #88408 (sentry), #88409 (jira). freshdesk, jira and sentry are at test_strictness_level: high, so validate_schema: false isn't an escape for them (_read_assertions.py:350–358). Only their schema fixes clear the check.
Suggest adding a merge-order checklist to the PR body (#1142, then the five PRs above, then this PR, then release) and holding merge until it's done. Re-run the manifest-only sweep on current master just before release, since test-account data drifts. Optionally list the 16 rotating-token connectors that weren't measured, so their owners expect a possible red run.
There was a problem hiding this comment.
Added a merge-order checklist to the PR body: the connector PRs (airbytehq/airbyte#88406, airbytehq/airbyte#88477, airbytehq/airbyte#88407, airbytehq/airbyte#88408, airbytehq/airbyte#88409 for this PR; airbytehq/airbyte#88412, airbytehq/airbyte#88480 for #1142) land first, then #1142 and this PR, then a fresh manifest-only sweep on current master, then the release. Merging into main doesn't publish, so the release is the gate.
…sertion' into tolik0/cdk/basic-read-assertions # Conflicts: # airbyte_cdk/test/standard_tests/docker_base.py # airbyte_cdk/test/standard_tests/source_base.py
…talog With #1142's `configured_catalog_path` support, the per-stream record check covers the streams of the config's declared catalog, and records are validated against the schemas that `discover` reports. Pin both, and say so in the migration guide. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
f0fad4e
into
devin/1788275534-shared-check-assertion
Part of airbytehq/airbyte-internal-issues#17135 (P1.2 "Restore read assertions", epic #17129).
What
SourceTestSuiteBase.test_basic_read, the in-processreadtest thatairbyte-cdk connector testruns, only failed when the whole read returned no records. One stream with records passed the test for every stream, and no record was ever checked against its schema. For configs listed underbasic_readand expected to succeed (status: succeed, or nostatus, which #1142 makes meansucceed), this PR restores the two record checks of CAT'sTestBasicRead.test_read:empty_streamsstays the list of exceptions, with the same meaning as in CAT: those streams are not read.date-timecheck, and no failure on extra columns. Like CAT, a record that shares no field with a schema that declarespropertiesalso fails.validate_schema: falsein thebasic_readentry turns the check off for that config. As in CAT, it is rejected for a connector attest_strictness_level: high: the test fails before reading, and points at fixing the schema or lowering the strictness level.Both checks run in one pass over the records, which also enforces the existing "Expected records but got none" floor, and all failures are reported in a single
AssertionError. The message names each stream and each distinct schema error once (up to 5 per stream), with the record number, the path inside the record, the failing schema keyword and the schema rule:Record values are never printed, for any kind of schema error, so record contents (often whole nested objects) stay out of public CI logs. The message names the value's JSON type and the schema's own scalar constraint (a format name, a pattern, a length), and for an
anyOf/oneOfthe closest option's failure, described the same way:A stream schema the validator cannot apply (an unknown
type, an unresolvable$ref, an invalidpattern) fails that stream with the schema problem, instead of aborting the check with a rawjsonschemaexception (whose text prints the record).Landing order for #17135
The issue covers more than one PR. This PR is the first:
AirbyteTraceMessage.Which scenarios get which check
With #1142,
test_basic_readruns only for configs listed underbasic_readorfull_refresh.basic_read(or built by hand, sosectionsis empty),status: succeedor nonefull_refreshbut notbasic_readstatus: failed/exceptionA config read only because of
full_refreshkeeps the old floor: CAT's full refresh test did not run these checks, and the config has nobasic_readentry to declareempty_streamsorvalidate_schema: falsein.At
test_strictness_level: high, abasic_readconfig withvalidate_schema: falsefails outright (CAT'stest_core.pydid the same).get_scenarios()now carries the top-leveltest_strictness_levelinto each scenario._dedup_scenariosmerges entries for the sameconfig_path. Avalidate_schema: falsefrom any entry is kept.Deviations from CAT
boolis anintsubclass, so CAT's strict-integer checker acceptedtruefor anintegerfield. JSON does not, and neither do destinations.date-timevalues pass the format check. As injsonschemaitself, a format only constrains strings; a wrong type is thetypekeyword's job. CAT's checker also skipped them.pendulumis replaced byab_datetime_try_parse. The shape regex is CAT's.$refs are resolved when collecting a schema's field paths, with a cycle guard, so recursive schemas do not loop.Not in this PR
readtest (test_docker_image_build_and_read) is not changed. It does not run by default, and P1.4 (#17137) owns it.assert_read_recordstakes the record messages and the configured catalog, not a scenario, so P1.4 can call it as is.Impact
The connector monorepo installs the
airbyte-cdkCLI unpinned, so connector CI picks this up with the release, without a per-connector bump. Python connectors' in-process tests are deselected until P0.7, so the change reaches manifest-only connectors first.The fleet count below covers manifest-only connectors only. Python connectors were not measured. They are deselected in monorepo CI until P0.7, but the CDK's own
Test Connectorsmatrix runs their in-process suites today:basic_readin-processsource-shopifysource-hardcoded-recordssource-google-drivefile_basedchanges, so it was skipped here and will first run on the next file-based CDK PRdestination-motherducksource-intercom,source-pokeapiBefore P0.7 turns Python in-process tests back on, re-run this measurement on every Python connector with a credentialed
basic_readconfig. They will go red the same way, and shopify shows one extra failure class that the Docker-based CAT never saw: class-level mutable state shared between configs read in the same process (seecountriesbelow).Measured on the manifest-only fleet
I ran
test_basic_readfrom this branch (on #1142 atd64a79c7) locally against airbytehq/airbytemaster. The population was every manifest-only connector whoseacceptance-test-config.ymlhas a credentialed config that reaches the read test: 188 connectors. 16 were skipped, because their test secret holds a single-use rotating refresh token, and a local run would invalidate the copy CI uses. Each scenario was read once, and the old verdict and the new checks came from the same read. 11 connectors whose manifests need CDK features newer than #1142's base were re-run on a local merge of this branch with currentmain. The later #1142 commit (e105226c) does not change the set below: the record checks only apply tobasic_readconfigs, and the onlybasic_readconfig without astatus(nowsucceed) returned records from every stream and matched its schemas.So 23 of the 88 connectors whose read passes today (26%) go red. These are 24 scenarios: 16 with streams that return no records and 12 with schema errors; some have both.
I checked each failure by hand, and none is a false positive of the validator:
configured_catalog.json, so CAT would have caught them too. The test accounts' data has drifted since CAT was removed: for example GitHub classic projects and workflow runs, PagerDuty incidents, and LinkedIn member analytics.number/integerfields, nulls in non-nullable fields,datevalues underformat: date-time,''underformat: email, and objects where the schema saysstring. The only failure caused by a deviation from CAT is bool-as-integer insource-sentry'sproject_detail, andsource-sentryalso fails the per-stream check.Connectors that go red (stream names only)
source-aircallconfigcalls,numberssource-amazon-seller-partnerconfigGET_FLAT_FILE_ALL_ORDERS_DATA_BY_LAST_UPDATE_GENERALsource-ashbyconfigapplications,job_postingssource-clickup-apiconfiglist_comments,list_custom_fields,space_tags,team,team_custom_fields,team_goals,time_tracking,time_tracking_tagssource-customer-ioconfigcampaigns_actionssource-fleetioconfigsubmitted_inspection_formssource-freshdeskconfigticket_activities; schema:sla_policiessource-githubconfigproject_cards,project_columns,projects,workflow_jobs,workflow_runssource-jiraservice_account_configfilter_sharing,filters,issue_comments,issue_properties,issue_remote_links,issue_worklogs,labels,project_categories,project_components,project_versionssource-jiraconfigissuessource-klaviyoconfigsegmentssource-linkedin-adsconfig_oauthad_member_company_analytics,ad_member_company_size_analytics,ad_member_country_analytics,ad_member_industry_analytics,ad_member_job_function_analytics,ad_member_job_title_analytics,ad_member_region_analytics,ad_member_seniority_analyticssource-mergeconfigjob_interview_stages,userssource-pagerdutyconfigincident_logs,incidentssource-paypal-transactionconfiglist_paymentssource-pendoconfigReport,Visitor Metadatasource-pexels-apiconfigvideos_searchsource-railzconfigbank_accounts,bank_transfers,commerce_disputes,commerce_orders,commerce_products,commerce_transactionssource-recurlyconfigplans; schema:accounts,subscriptionssource-sentryconfigreleases; schema:project_detail,all_projects_detailsource-snapchat-marketingconfigsegmentssource-tmdbconfigmovies_latest,movies_reviewssource-uptickconfigpromptanswergroups; schema:clients,branches,products,contractors,clientcontacts,propertycontactssource-woocommerceconfigproduct_variations,shipping_zone_methodsMerge order
Connector CI installs the CDK CLI unpinned, so these checks apply to every connector PR from the day the CDK is released. Merging into
maindoes not publish anything; the release is the gate. All connector PRs below are green on the current released CDK, so they can land first, in any order:sla_policiesids as integers airbyte#88407 (freshdesk), fix(source-sentry): type thesentry:replay_rage_click_issuesproject option as boolean airbyte#88408 (sentry), fix(source-jira): type rendered comment bodies inissuesas strings airbyte#88409 (jira), fix(source-shopify): keep the countries dedup list per stream instance airbyte#88405 (shopify), fix(source-customer-io)!: 🚨🚨 declare integer from_id, reply_to_id and sent_at and typed array items 🚨🚨 airbyte#88159 (customer-io).## Upgrading to 7.36.0, which also covers feat(standard-tests): assert discovered catalog properties in test_discover #1217, feat(standard-tests): lintairbyte_secretmarking and secret leaks #1219, feat(standard-tests): check spec backward compatibility against the published version #1220 and feat(standard-tests): run each acceptance-test-config.yml config with its section's command, and assert the check status and basic_read records #1142, so all five must ship in that one release.Rollout
The 23 connectors go red on their next PR after the release. Proposal: before the release, land one monorepo PR that declares the empty streams in
empty_streamsand setsvalidate_schema: falsefor these configs, each with abypass_reasonlinking a follow-up issue, so the release turns nothing red, and then fix the connectors one by one. The other option is to release as is and fix each connector when it is next touched.The opt-out PR also needs:
source-shopify(certified,test_strictness_level: high; CI job 113491220276).deleted_productsreturns no records for bothbasic_readconfigs, andcountriesreturns none forsecrets/config.json.deleted_products, both configs: declare it empty (or seed a product deletion in the test shop). It is in neitherexpected_records*.jsonl; the stream readsaction:destroyproduct events.countries,secrets/config.json: a connector bug, not an empty stream. Both expected-records files contain the samecountriesrecords, so the shop has data.Countriesdeduplicates through a class attribute,_synced_countries_ids = [](source_shopify/streams/streams.py:773), which every instance shares. The in-process suite readsconfig_transactions_with_user_idfirst, which fills the list, so the second config's read skips every country. Production syncs run one config per process and are not affected; CAT ran each config in its own container. The fix is a one-line connector change (initialise the list per instance) with a patch bump. Until it lands, a stopgapempty_streamsentry with a reason that names the bug keeps CI green.No
validate_schema: falsefor high-strictness connectors. Three of the twelve connectors with schema failures declaretest_strictness_level: high:source-freshdesk(sla_policies),source-jira(issues) andsource-sentry(project_detail,all_projects_detail). The opt-out PR cannot setvalidate_schema: falsefor them (the test now fails on it), so their schemas must be fixed before the release, or the release moves after those fixes. The other nine (ashby, customer-io, fleetio, pendo, pexels-api, recurly, tmdb, uptick, woocommerce) declare no strictness level, which CAT treated aslow, so the opt-out stays available to them. woocommerce is certified but declares no strictness level.source-azure-tablealready setsvalidate_schema: Falseon master (no strictness level), so it keeps working.cdk-migrations.mdhas a "### Every stream returns records that match its schema (basic_read)" subsection with the migration steps: add data or declareempty_streamswith abypass_reason; fix the schema or setvalidate_schema: false(not available attest_strictness_level: high). It sits under the "Upgrading to the next minor release (unreleased)" heading shared with #1142, #1217, #1219 and #1220; rename the heading to the released version when the release is cut. This ships as a minor release: only the test harness changes.Tests
unit_tests/test/test_read_assertions.py(77 tests) covers:date-timeshapes; nested and non-identifier record paths; the structure check; the per-stream error cap; dedup per schema rule; recursive$ref;anyOf/oneOf,not,allOf,enum,const,pattern,format,uniqueItems,required, each with a sentinel value that must not appear;type, unresolvable local and remote$ref, invalidpattern;SourceTestSuiteBase.test_basic_readfor every combination of outcome and sections, includingstatus: failedandexceptionreads that raise, afailedread that succeeds, andtest_strictness_level: highandlowwith and withoutvalidate_schema: false;validate_schema(with aconnectionentry, and twobasic_readentries in both orders) and the parsing oftest_strictness_level.unit_tests/testplusunit_tests/resources(including the pokeapi Docker image tests): 380 passed, 12 skipped. This includes feat(standard-tests): run each acceptance-test-config.yml config with its section's command, and assert the check status and basic_read records #1142'stest_scenario_sections.py.ruff check,ruff format --checkandmypy airbyte_cdk/testare clean.🤖 Generated with Claude Code