SAT: basic read on full catalog when `test_strictness_level == high` #18937

alafanechere · 2022-11-03T19:40:44Z

What

Closes #18306
When test_strictness_level is high we want to discard any custom configured catalog passed and use the discovered catalog instead, from which all the declared empty_streams are removed.

How

Refacto: change test_read signature by splitting inputs's attributes into granular fixtures (on which strictness level is enforced)
Work on the configured_catalog_fixture to not use any custom configured catalog when the strictness is high. We build the configured catalog from the discovered catalog from which we remove the declared empty_streams.
Make the use of custom catalog for testing safer: all stream declared in the configured catalog must exists in the discovered catalog

🚨 User Impact 🚨

This should not impact existing configuration as they are not yet migrated to high test strictness level.

…ictness level checks

github-actions · 2022-11-03T19:41:42Z

Affected Connector Report

NOTE ⚠️ Changes in this PR affect the following connectors. Make sure to do the following as needed:

Run integration tests
Bump connector version
Add changelog
Publish the new version

❌ Sources (63)

Connector	Version	Changelog	Publish
`source-airtable`	`0.1.3`	✅	✅
`source-amazon-ads`	`0.1.24`	✅	✅
`source-amazon-seller-partner`	`0.2.28`	✅	✅
`source-amazon-sqs`	`0.1.0`	✅	✅
`source-amplitude`	`0.1.17`	✅	✅
`source-appsflyer`	`0.1.0`	⚠ (doc not found)	⚠ (not in seed)
`source-asana`	`0.1.4`	✅	✅
`source-azure-table`	`0.1.3`	✅	✅
`source-braintree`	`0.1.3`	✅	✅
`source-cart`	`0.2.0`	❌ (changelog missing)	✅
`source-chargebee`	`0.1.16`	✅	❌ (diff seed version)
`source-commercetools`	`0.1.0`	✅	✅
`source-confluence`	`0.1.1`	✅	✅
`source-datadog`	`0.1.0`	✅	✅
`source-delighted`	`0.1.4`	✅	✅
`source-drift`	`0.2.5`	✅	✅
`source-facebook-marketing`	`0.2.72`	✅	✅
`source-facebook-pages`	`0.1.6`	✅	✅
`source-freshcaller`	`0.1.0`	✅	✅
`source-freshsales`	`0.1.2`	✅	✅
`source-freshservice`	`0.1.1`	✅	✅
`source-github`	`0.3.7`	✅	✅
`source-gitlab`	`0.1.6`	✅	✅
`source-google-ads`	`0.2.3`	✅	✅
`source-google-search-console`	`0.1.18`	✅	✅
`source-greenhouse`	`0.3.0`	✅	✅
`source-harvest`	`0.1.11`	✅	✅
`source-instagram`	`1.0.0`	✅	✅
`source-iterable`	`0.1.21`	✅	✅
`source-klaviyo`	`0.1.10`	✅	✅
`source-lemlist`	`0.1.1`	❌ (changelog missing)	❌ (diff seed version)
`source-lever-hiring`	`0.1.3`	✅	✅
`source-linnworks`	`0.1.5`	❌ (changelog missing)	✅
`source-mailchimp`	`0.2.15`	✅	✅
`source-mailgun`	`0.1.0`	⚠ (doc not found)	✅
`source-monday`	`0.1.4`	✅	✅
`source-notion`	`0.1.10`	✅	✅
`source-okta`	`0.1.13`	✅	✅
`source-onesignal`	`0.1.2`	✅	✅
`source-openweather`	`0.1.6`	✅	✅
`source-outreach`	`0.1.2`	✅	✅
`source-pardot`	`0.1.0`	⚠ (doc not found)	⚠ (not in seed)
`source-paystack`	`0.1.1`	✅	✅
`source-pinterest`	`0.1.8`	✅	✅
`source-pipedrive`	`0.1.13`	✅	✅
`source-plaid`	`0.3.2`	❌ (changelog missing)	✅
`source-posthog`	`0.1.7`	✅	✅
`source-prestashop`	`0.2.0`	⚠ (doc not found)	✅
`source-quickbooks-singer`	`0.1.5`	⚠ (doc not found)	✅
`source-recharge`	`0.2.4`	❌ (changelog missing)	✅
`source-retently`	`0.1.2`	✅	✅
`source-salesforce`	`1.0.24`	✅	✅
`source-salesloft`	`0.1.3`	✅	✅
`source-sendgrid`	`0.2.16`	✅	✅
`source-sentry`	`0.1.7`	✅	✅
`source-strava`	`0.1.2`	✅	✅
`source-surveymonkey`	`0.1.11`	✅	✅
`source-tplcentral`	`0.1.0`	✅	⚠ (not in seed)
`source-twilio`	`0.1.13`	✅	✅
`source-youtube-analytics`	`0.1.3`	✅	✅
`source-zendesk-sunshine`	`0.1.1`	✅	✅
`source-zendesk-talk`	`0.1.5`	✅	✅
`source-zenloop`	`0.1.3`	✅	✅

See "Actionable Items" below for how to resolve warnings and errors.

✅ Destinations (0)

Connector	Version	Changelog	Publish

See "Actionable Items" below for how to resolve warnings and errors.

Actionable Items

(click to expand)

Category	Status	Actionable Item
Version	❌ mismatch	The version of the connector is different from its normal variant. Please bump the version of the connector.
	⚠ doc not found	The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like `source-jdbc` is not published or documented). Please double-check to make sure that it is not a bug.
Changelog	⚠ doc not found	The connector does not seem to have a documentation file. This can be normal (e.g. basic connector like `source-jdbc` is not published or documented). Please double-check to make sure that it is not a bug.
	❌ changelog missing	There is no chnagelog for the current version of the connector. If you are the author of the current version, please add a changelog.
Publish	⚠ not in seed	The connector is not in the seed file (e.g. `source_definitions.yaml`), so its publication status cannot be checked. This can be normal (e.g. some connectors are cloud-specific, and only listed in the cloud seed file). Please double-check to make sure that it is not a bug.
	❌ diff seed version	The connector exists in the seed file, but the latest version is not listed there. This usually means that the latest version is not published. Please use the `/publish` command to publish the latest version.

sherifnada

Could we be transparent to the developer about this? i.e: they shouldn't be able to pass in a configured_catalog if strictness is high. Otherwise it will lead to confusing test results. Otherwise the rest of the PR looks good!

…airbytehq/airbyte into augustin/sat/basic-read-on-full-catalog

alafanechere · 2022-11-04T10:42:46Z

I realized the changes I was about to make on the configured_catalog_fixture would also affect other tests using it (full_refresh, incremental). I don't want these tests to be impacted by the test strictness level yet. I decided to implement move my changes to a new configured_catalog_fixture declared in the scope of TestBasicRead to prevent this.
This involved moving other helper functions out from conftest to utils.commons.

alafanechere · 2022-11-04T10:44:20Z

@sherifnada I changed to fail the test if a custom configured catalog is passed in the basic_read config while we are on a high test strictness level.

alafanechere · 2022-11-04T10:46:24Z

/test connector=connectors/source-pokeapi

🕑 connectors/source-pokeapi https://github.com/airbytehq/airbyte/actions/runs/3393158810
❌ connectors/source-pokeapi https://github.com/airbytehq/airbyte/actions/runs/3393158810
🐛 https://gradle.com/s/ykdqngiuz6ws2

Build Failed

Test summary info:

=========================== short test summary info ============================
ERROR test_core.py::TestBasicRead::test_read[inputs0] - Failed: High strictne...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestSpec.test_config_match_spec: The spec is currently invalid: it has additionalProperties set to false
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: This connector does not support incremental syncs.
==================== 12 passed, 2 skipped, 1 error in 9.24s ====================

alafanechere · 2022-11-04T10:52:49Z

☝️ the test above should fail because a configured_catalog_path is defined in the basic_read configuration.
Edit: ✅ -> E Failed: High strictness level error: you can't set a custom configured catalog on the basic read test.

alafanechere · 2022-11-04T11:06:48Z

/test connector=connectors/source-pokeapi

🕑 connectors/source-pokeapi https://github.com/airbytehq/airbyte/actions/runs/3393291171
✅ connectors/source-pokeapi https://github.com/airbytehq/airbyte/actions/runs/3393291171
Python tests coverage:

	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       133      3    98%   87, 93, 230
	 source_acceptance_test/conftest.py                     196     92    53%   35, 41-43, 48, 54, 60, 66, 72-74, 93, 98-100, 106-108, 114-115, 120-121, 126, 132, 141-150, 156-161, 176, 200, 231, 237, 243-248, 256-261, 269-282, 287-293, 300-311, 318-334
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              345    110    68%   53, 64-72, 77-84, 88-89, 93-94, 178, 216-233, 242-250, 254-259, 265, 298-303, 341-348, 391-393, 396, 461-469, 481-484, 489, 543-544, 550, 553, 589-599, 612-637
	 source_acceptance_test/tests/test_incremental.py       145     20    86%   21-23, 29-31, 36-43, 48-61, 224
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/config_migration.py        23     23     0%   5-37
	 source_acceptance_test/utils/connector_runner.py       112     50    55%   23-26, 32, 36, 39-68, 71-73, 76-78, 81-83, 86-88, 91-93, 96-114, 148-150
	 source_acceptance_test/utils/json_schema_helper.py     105     13    88%   30-31, 38, 41, 65-68, 96, 120, 190-192
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1512    375    75%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestSpec.test_config_match_spec: The spec is currently invalid: it has additionalProperties set to false
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: This connector does not support incremental syncs.
======================== 13 passed, 2 skipped in 9.31s =========================

alafanechere · 2022-11-04T11:07:26Z

☝️ I removed configured_catalog_path for the basic_read configuration.
The test should pass

airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py

…tance_test/tests/test_core.py Co-authored-by: Sherif A. Nada <snadalive@gmail.com>

alafanechere · 2022-11-04T15:02:31Z

/test connector=connectors/source-google-analytics-v4

🕑 connectors/source-google-analytics-v4 https://github.com/airbytehq/airbyte/actions/runs/3394847702
✅ connectors/source-google-analytics-v4 https://github.com/airbytehq/airbyte/actions/runs/3394847702
Python tests coverage:

Name                                                     Stmts   Miss  Cover
----------------------------------------------------------------------------
source_google_analytics_v4/custom_reports_validator.py      41      0   100%
source_google_analytics_v4/__init__.py                       2      0   100%
source_google_analytics_v4/source.py                       297     20    93%
----------------------------------------------------------------------------
TOTAL                                                      340     20    94%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       133      3    98%   87, 93, 230
	 source_acceptance_test/conftest.py                     196     92    53%   35, 41-43, 48, 54, 60, 66, 72-74, 93, 98-100, 106-108, 114-115, 120-121, 126, 132, 141-150, 156-161, 176, 200, 231, 237, 243-248, 256-261, 269-282, 287-293, 300-311, 318-334
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              345    110    68%   53, 64-72, 77-84, 88-89, 93-94, 178, 216-233, 242-250, 254-259, 265, 298-303, 341-348, 391-393, 396, 461-469, 481-484, 489, 545-546, 552, 555, 591-601, 614-639
	 source_acceptance_test/tests/test_incremental.py       145     20    86%   21-23, 29-31, 36-43, 48-61, 224
	 source_acceptance_test/utils/asserts.py                 37      2    95%   57-58
	 source_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 source_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 source_acceptance_test/utils/config_migration.py        23     23     0%   5-37
	 source_acceptance_test/utils/connector_runner.py       112     50    55%   23-26, 32, 36, 39-68, 71-73, 76-78, 81-83, 86-88, 91-93, 96-114, 148-150
	 source_acceptance_test/utils/json_schema_helper.py     105     13    88%   30-31, 38, 41, 65-68, 96, 120, 190-192
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1512    375    75%

Build Passed

Test summary info:

All Passed

alafanechere · 2022-11-04T15:02:57Z

☝️ Testing on a unmigrated acceptance-test-config.yml for sanity

…airbytehq/airbyte into augustin/sat/basic-read-on-full-catalog

* master: (69 commits) 🪟 🐛 Fix wrong geography dropdown type #19021 SAT: basic read on full catalog when `test_strictness_level == high` (#18937) Unhide DynamoDB destination (#18994) Fixed tests for destination connectors (#19007) 🐛 Source Facebook Marketing: handle FacebookBadObjectError (#18971) Edit multi-cloud docs (#18972) 🪟 🎉 Load credits consumption separate (#18986) Bmoric/extract source api (#18944) Migrating InvalidCursorException -> ConfigErrorException (#18995) 🪟 🎨 Fix banner link color (#18978) Handling configuration exceptions in IntegrationRunner (#18989) Add new workspace api endpoint (#18983) Add normalization to destination definition and actor definition table (#18300) Fix oauth controller (#18981) Fix migration dev center schema dump by run db-specific initialization script (#18984) fix master build failure (#18982) cleanup: delete debezium 1-4-2 module (#18733) Remove unused job persistence methods. (#18952) Hash filenames of extracted CSS (#18976) Fix typo in source code comment DataDaog ==> Datadog (#18911) ...

* master: (73 commits) 🪟 🐛 Fix wrong geography dropdown type #19021 SAT: basic read on full catalog when `test_strictness_level == high` (#18937) Unhide DynamoDB destination (#18994) Fixed tests for destination connectors (#19007) 🐛 Source Facebook Marketing: handle FacebookBadObjectError (#18971) Edit multi-cloud docs (#18972) 🪟 🎉 Load credits consumption separate (#18986) Bmoric/extract source api (#18944) Migrating InvalidCursorException -> ConfigErrorException (#18995) 🪟 🎨 Fix banner link color (#18978) Handling configuration exceptions in IntegrationRunner (#18989) Add new workspace api endpoint (#18983) Add normalization to destination definition and actor definition table (#18300) Fix oauth controller (#18981) Fix migration dev center schema dump by run db-specific initialization script (#18984) fix master build failure (#18982) cleanup: delete debezium 1-4-2 module (#18733) Remove unused job persistence methods. (#18952) Hash filenames of extracted CSS (#18976) Fix typo in source code comment DataDaog ==> Datadog (#18911) ...

alafanechere added 5 commits November 2, 2022 10:52

Repace inputs fixture in test_read by more granular one with test str…

5195985

…ictness level checks

cleaner configured catalog fixture

2df295e

add tests

9da3dd0

add unit tests

31db654

add unit tests

5acd80d

alafanechere requested review from sherifnada and a team November 3, 2022 19:49

Merge branch 'master' into augustin/sat/basic-read-on-full-catalog

fe35df3

sherifnada suggested changes Nov 4, 2022

View reviewed changes

alafanechere added 2 commits November 4, 2022 11:37

reorganize

572f7f2

Merge branch 'augustin/sat/basic-read-on-full-catalog' of github.com:…

f3b0aa7

…airbytehq/airbyte into augustin/sat/basic-read-on-full-catalog

demo with poke api

486fa9d

github-actions bot added the area/connectors Connector related issues label Nov 4, 2022

octavia-squidington-iv added the connectors/source/pokeapi label Nov 4, 2022

alafanechere requested review from a team and removed request for a team November 4, 2022 10:55

demo with poke api

bd89923

sherifnada approved these changes Nov 4, 2022

View reviewed changes

airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py Outdated Show resolved Hide resolved

Update airbyte-integrations/bases/source-acceptance-test/source_accep…

a645ef9

…tance_test/tests/test_core.py Co-authored-by: Sherif A. Nada <snadalive@gmail.com>

update docs

d24c830

github-actions bot added the area/documentation Improvements or additions to documentation label Nov 4, 2022

alafanechere and others added 5 commits November 4, 2022 19:02

Merge branch 'master' into augustin/sat/basic-read-on-full-catalog

9776133

bump version

6aa4404

Merge branch 'master' into augustin/sat/basic-read-on-full-catalog

c66ccf4

format

4af5396

Merge branch 'augustin/sat/basic-read-on-full-catalog' of github.com:…

2f21040

…airbytehq/airbyte into augustin/sat/basic-read-on-full-catalog

alafanechere merged commit 7d73b08 into master Nov 7, 2022

alafanechere deleted the augustin/sat/basic-read-on-full-catalog branch November 7, 2022 07:09

github-actions bot mentioned this pull request Nov 15, 2022

Bump helm chart version reference to 0.40.43 #19453

Closed

octavia-squidington-iii mentioned this pull request Nov 18, 2022

Bump Airbyte version from 0.40.18 to 0.40.19 #19579

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SAT: basic read on full catalog when `test_strictness_level == high` #18937

SAT: basic read on full catalog when `test_strictness_level == high` #18937

alafanechere commented Nov 3, 2022 •

edited

Loading

github-actions bot commented Nov 3, 2022 •

edited

Loading

❌ Sources (63)

✅ Destinations (0)

Actionable Items

sherifnada left a comment

alafanechere commented Nov 4, 2022 •

edited

Loading

alafanechere commented Nov 4, 2022

alafanechere commented Nov 4, 2022 •

edited by github-actions bot

Loading

alafanechere commented Nov 4, 2022 •

edited

Loading

alafanechere commented Nov 4, 2022 •

edited by github-actions bot

Loading

alafanechere commented Nov 4, 2022

alafanechere commented Nov 4, 2022 •

edited by github-actions bot

Loading

alafanechere commented Nov 4, 2022

SAT: basic read on full catalog when test_strictness_level == high #18937

SAT: basic read on full catalog when test_strictness_level == high #18937

Conversation

alafanechere commented Nov 3, 2022 • edited Loading

What

How

Recommended reading order

🚨 User Impact 🚨

github-actions bot commented Nov 3, 2022 • edited Loading

Affected Connector Report

❌ Sources (63)

✅ Destinations (0)

Actionable Items

sherifnada left a comment

Choose a reason for hiding this comment

alafanechere commented Nov 4, 2022 • edited Loading

alafanechere commented Nov 4, 2022

alafanechere commented Nov 4, 2022 • edited by github-actions bot Loading

Build Failed

alafanechere commented Nov 4, 2022 • edited Loading

alafanechere commented Nov 4, 2022 • edited by github-actions bot Loading

Build Passed

alafanechere commented Nov 4, 2022

alafanechere commented Nov 4, 2022 • edited by github-actions bot Loading

Build Passed

alafanechere commented Nov 4, 2022

SAT: basic read on full catalog when `test_strictness_level == high` #18937

SAT: basic read on full catalog when `test_strictness_level == high` #18937

alafanechere commented Nov 3, 2022 •

edited

Loading

github-actions bot commented Nov 3, 2022 •

edited

Loading

alafanechere commented Nov 4, 2022 •

edited

Loading

alafanechere commented Nov 4, 2022 •

edited by github-actions bot

Loading

alafanechere commented Nov 4, 2022 •

edited

Loading

alafanechere commented Nov 4, 2022 •

edited by github-actions bot

Loading

alafanechere commented Nov 4, 2022 •

edited by github-actions bot

Loading