Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SAT: make future_state configuration mandatory in high test strictness level #19085

Merged

Conversation

alafanechere
Copy link
Contributor

@alafanechere alafanechere commented Nov 7, 2022

What

Closes #18223
In high_test_strictness level we want to have the new following rules in the incremental test:

  1. future_state to be mandatory
  2. The future_state file must declare all existing streams.
  3. Missing streams can declared with a valid bypass_reason

How

  • Change the config structure for incremental tests
  • Change future_state_path and future_state fixture to make tests using it fail if the rules described above are not followed.
  • Unit tests the two edited fixtures

@github-actions
Copy link
Contributor

github-actions bot commented Nov 7, 2022

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.9
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.

@alafanechere alafanechere force-pushed the augustin/sat/more-strictness-in-future-state-path branch from 1303e14 to 8a230c5 Compare November 7, 2022 21:44
@github-actions github-actions bot removed the area/connectors Connector related issues label Nov 7, 2022
@github-actions github-actions bot added the area/connectors Connector related issues label Nov 7, 2022
@alafanechere
Copy link
Contributor Author

alafanechere commented Nov 7, 2022

/test connector=connectors/source-github

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

Name                             Stmts   Miss  Cover
----------------------------------------------------
source_github/utils.py              14      0   100%
source_github/github_schema.py    8807      0   100%
source_github/__init__.py            2      0   100%
source_github/graphql.py           145      1    99%
source_github/streams.py           796     44    94%
source_github/source.py            104     27    74%
----------------------------------------------------
TOTAL                             9868     72    99%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       139      5    96%   87, 93, 235, 239-240
	 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       158     14    91%   52-59, 64-77, 240
	 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                                                 1531    371    76%

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: disabled for demo
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestConnection.test_check: disabled for demo
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestDiscovery.test_discover: disabled for demo
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestBasicRead.test_read: disabled for demo
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestFullRefresh.test_sequential_reads: disabled for demo
=================== 3 passed, 5 skipped in 176.01s (0:02:56) ===================

@alafanechere
Copy link
Contributor Author

alafanechere commented Nov 7, 2022

☝️ The test above should succeed.
Acceptance test config changed for Source GitHub in afb73f7

  • high test strictness level
  • A future_state with future_state_path is declared. All the missing streams are declared under missing_streams.

@alafanechere
Copy link
Contributor Author

/test connector=connectors/source-github

@alafanechere
Copy link
Contributor Author

alafanechere commented Nov 7, 2022

☝️ The test above should fail.
Acceptance test config changed for Source GitHub in ffe33de

  • high test strictness level
  • A future_state with future_state_path is declared but a stream is missing under missing_streams.

@github-actions github-actions bot added the area/documentation Improvements or additions to documentation label Nov 7, 2022
@alafanechere alafanechere changed the title SAT: more-strictness-in-future-state-path SAT: make future_state configuration mandatory in high test strictness level Nov 7, 2022
@alafanechere
Copy link
Contributor Author

alafanechere commented Nov 8, 2022

/test connector=connectors/source-github

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

Name                             Stmts   Miss  Cover
----------------------------------------------------
source_github/utils.py              14      0   100%
source_github/github_schema.py    8807      0   100%
source_github/__init__.py            2      0   100%
source_github/graphql.py           145      1    99%
source_github/streams.py           796     44    94%
source_github/source.py            104     27    74%
----------------------------------------------------
TOTAL                             9868     72    99%
	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       139      5    96%   87, 93, 235, 239-240
	 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       158     14    91%   52-59, 64-77, 240
	 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                                                 1531    371    76%

Build Passed

Test summary info:

All Passed

@alafanechere
Copy link
Contributor Author

☝️ The test above should not fail, I reverted source github's acceptance-test-config.yaml to its original form.

@alafanechere alafanechere marked this pull request as ready for review November 8, 2022 00:10
@alafanechere alafanechere requested a review from a team November 8, 2022 00:10
@evantahler
Copy link
Contributor

... eagerly waits for #19085 (comment) to report an error...

@alafanechere
Copy link
Contributor Author

... eagerly waits for #19085 (comment) to report an error...

Ok let's try again 😄

@alafanechere
Copy link
Contributor Author

alafanechere commented Nov 8, 2022

/test connector=connectors/source-github

🕑 connectors/source-github https://github.com/airbytehq/airbyte/actions/runs/3417545775
❌ connectors/source-github https://github.com/airbytehq/airbyte/actions/runs/3417545775
🐛 https://gradle.com/s/iib3sxmseyzng

Build Failed

Test summary info:

=========================== short test summary info ============================
ERROR test_incremental.py::TestIncremental::test_state_with_abnormally_large_values[inputs0]
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestSpec.test_config_match_spec: disabled for demo
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestConnection.test_check: disabled for demo
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestDiscovery.test_discover: disabled for demo
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestBasicRead.test_read: disabled for demo
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/plugin.py:63: Skipping TestFullRefresh.test_sequential_reads: disabled for demo
============== 2 passed, 5 skipped, 1 error in 183.86s (0:03:03) ===============

@alafanechere
Copy link
Contributor Author

alafanechere commented Nov 8, 2022

☝️ The test above should fail.
Acceptance test config changed for Source GitHub in a5b7ff2

  • high test strictness level
  • A future_state with future_state_path is declared but a stream is missing under missing_streams.

Edit:
✅ It failed with the following error:

E               Failed: High test strictness level error: team_members streams are missing in your future_state file, please declare a state for those streams or fill-in a valid bypass_reason.

@alafanechere
Copy link
Contributor Author

alafanechere commented Nov 8, 2022

/publish connector=bases/source-acceptance-test auto-bump-version=false

🕑 Publishing the following connectors:
bases/source-acceptance-test
https://github.com/airbytehq/airbyte/actions/runs/3423146304


Connector Did it publish? Were definitions generated?
bases/source-acceptance-test

if you have connectors that successfully published but failed definition generation, follow step 4 here ▶️

@alafanechere alafanechere merged commit a196cf7 into master Nov 8, 2022
@alafanechere alafanechere deleted the augustin/sat/more-strictness-in-future-state-path branch November 8, 2022 22:10
akashkulk pushed a commit that referenced this pull request Dec 2, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation connectors/source/github connectors/source/salesforce
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SAT [strict_mode]: future_state for all incremental streams
3 participants