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

Add emitted_at SAT test (22240) #22291

Merged
merged 8 commits into from
Feb 9, 2023
Merged

Conversation

bnchrch
Copy link
Contributor

@bnchrch bnchrch commented Feb 2, 2023

What

We've recently encountered bugs with normalization when normalizing deeply nested fields that don't have cursors or primary keys for incremental syncs. Because we can't tell if a new nested object is new or not, we end up creating ever-more rows.

This adds a test for that case

Closes #22240

How

Add a new SAT test or extend an existing one so that:

  1. a READ is preformed using working CONFIG, CATALOG, and STATE such that at least 1 record is produced
  2. store the max value for emitted at
  3. sleep for 1 second
  4. run the READ again, with the same CONFIG, CATALOG, and STATE.
  5. assert that the same records are produced but with different emitted_ats, and the second emitted_ats are always greater than the previous run's max(emitted_at)

@bnchrch bnchrch requested a review from a team February 2, 2023 02:39
@bnchrch bnchrch self-assigned this Feb 2, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Feb 2, 2023

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 or module version
  • Add changelog
  • Publish the new version

❌ Sources (63)

Connector Version Changelog Publish
source-airtable 2.0.3
source-amazon-ads 1.0.0
source-amazon-seller-partner 0.2.31
source-amazon-sqs 0.1.0
source-amplitude 0.1.21
source-appsflyer 0.1.0
source-asana 0.1.5
source-azure-table 0.1.3
source-braintree 0.1.3
source-cart 0.2.0
source-chargebee 0.1.16
source-commercetools 0.1.0
source-confluence 0.1.1
source-datadog 0.1.0
source-delighted 0.2.0
source-drift 0.2.5
source-facebook-marketing 0.2.84
source-freshcaller 0.1.0
source-freshsales 0.1.2
source-freshservice 0.1.1
source-github 0.4.1
source-gitlab 1.0.2
source-google-ads 0.2.9
source-google-search-console 0.1.20
source-greenhouse 0.3.0
source-harvest 0.1.15
source-instagram 1.0.1
source-iterable 0.1.23
source-klaviyo 0.1.12
source-lemlist 0.1.1
source-lever-hiring 0.1.3
source-linnworks 0.1.5
source-mailchimp 0.3.4
source-mailgun 0.1.0
source-monday 0.2.2
source-notion 1.0.1
source-okta 0.1.14
source-onesignal 0.1.2
source-openweather 0.1.6
source-outreach 0.1.2
source-pardot 0.1.1
source-paystack 0.1.1
source-pinterest 0.2.2
source-pipedrive 0.1.13
source-plaid 0.3.2
source-posthog 0.1.8
source-prestashop 0.3.0
source-quickbooks-singer 0.1.5
source-recharge 0.2.6
(diff seed version)
source-retently 0.1.3
source-salesforce 2.0.0
source-salesloft 0.1.3
source-sendgrid 0.3.1
source-sentry 0.1.11
source-strava 0.1.2
source-surveymonkey 0.1.14
source-tplcentral 0.1.1
source-twilio 0.1.15
source-weatherstack 0.1.0
source-youtube-analytics 0.1.3
source-zendesk-sunshine 0.1.1
source-zendesk-talk 0.1.6
source-zenloop 0.1.4
  • 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.

✅ Other Modules (0)

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.

@evantahler evantahler requested a review from grishick February 2, 2023 16:10
@evantahler
Copy link
Contributor

evantahler commented Feb 2, 2023

/test connector=connectors/source-hubspot

🕑 connectors/source-hubspot https://github.com/airbytehq/airbyte/actions/runs/4076392708
❌ connectors/source-hubspot https://github.com/airbytehq/airbyte/actions/runs/4076392708
🐛 https://gradle.com/s/vyugzoxnbak76

Build Failed

Test summary info:

	 =========================== short test summary info ============================
	 ERROR integration_tests/test_associations.py::test_incremental_read_fetches_associations
	 �[31m======================== �[33m53 warnings�[0m, �[31m�[1m1 error�[0m�[31m in 0.96s�[0m�[31m =========================�[0m

@evantahler
Copy link
Contributor

evantahler commented Feb 2, 2023

/test connector=connectors/source-faker

🕑 connectors/source-faker https://github.com/airbytehq/airbyte/actions/runs/4076393457
❌ connectors/source-faker https://github.com/airbytehq/airbyte/actions/runs/4076393457
🐛 https://gradle.com/s/cayitnzulkx4m

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestBasicRead::test_read[inputs0] - AssertionError: Stre...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:509: The previous and actual discovered catalogs are identical.
=================== 1 failed, 35 passed, 2 skipped in 38.08s ===================

@evantahler
Copy link
Contributor

evantahler commented Feb 2, 2023

/test connector=connectors/source-postgres

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

	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       141      5    96%   87, 93, 239, 243-244
	 source_acceptance_test/conftest.py                     211     95    55%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-285, 293-306, 311-317, 324-335, 342-358
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              494    117    76%   55, 60, 99-110, 115-122, 126-127, 131-132, 382, 402, 440, 478-495, 508-519, 523-528, 534, 567-572, 610-617, 660-662, 665, 730-738, 750-753, 758, 814-815, 821, 824, 892-902, 915-940
	 source_acceptance_test/tests/test_incremental.py       160     14    91%   58-65, 70-83, 246
	 source_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 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/connector_runner.py       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 source_acceptance_test/utils/json_schema_helper.py     114     13    89%   31-32, 39, 42, 66-69, 97, 121, 203-205
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1708    341    80%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [2] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
SKIPPED [2] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:509: The previous and actual discovered catalogs are identical.
SKIPPED [2] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_incremental.py:22: `future_state` has a bypass reason, skipping.
================== 68 passed, 6 skipped in 735.33s (0:12:15) ===================

Copy link
Contributor

@evantahler evantahler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm testing some connectors ^ ... let's see how they do

Comment on lines 359 to 370
(
{"type": "object"},
[
AirbyteMessage(type=Type.RECORD, record=AirbyteRecordMessage(stream="test_stream", data={"aa": 23}, emitted_at=111)),
AirbyteMessage(type=Type.RECORD, record=AirbyteRecordMessage(stream="test_stream", data={"aa": 24}, emitted_at=111)),
],
[
AirbyteMessage(type=Type.RECORD, record=AirbyteRecordMessage(stream="test_stream", data={"aa": 23}, emitted_at=111)),
AirbyteMessage(type=Type.RECORD, record=AirbyteRecordMessage(stream="test_stream", data={"aa": 24}, emitted_at=112)),
],
pytest.raises(AssertionError, match="emitted_at should increase on subsequent runs")
),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 this is the main assertion

@bnchrch
Copy link
Contributor Author

bnchrch commented Feb 2, 2023

/test connector=connectors/source-pokeapi

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

	 Name                                                 Stmts   Miss  Cover   Missing
	 ----------------------------------------------------------------------------------
	 source_acceptance_test/base.py                          12      4    67%   16-19
	 source_acceptance_test/config.py                       141      5    96%   87, 93, 239, 243-244
	 source_acceptance_test/conftest.py                     211     95    55%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-285, 293-306, 311-317, 324-335, 342-358
	 source_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 source_acceptance_test/tests/test_core.py              494    117    76%   55, 60, 99-110, 115-122, 126-127, 131-132, 382, 402, 440, 478-495, 508-519, 523-528, 534, 567-572, 610-617, 660-662, 665, 730-738, 750-753, 758, 814-815, 821, 824, 892-902, 915-940
	 source_acceptance_test/tests/test_incremental.py       160     14    91%   58-65, 70-83, 246
	 source_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 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/connector_runner.py       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 source_acceptance_test/utils/json_schema_helper.py     114     13    89%   31-32, 39, 42, 66-69, 97, 121, 203-205
	 ----------------------------------------------------------------------------------
	 TOTAL                                                 1708    341    80%

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.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:509: The previous and actual discovered catalogs are identical.
======================== 13 passed, 3 skipped in 12.44s ========================

@bnchrch
Copy link
Contributor Author

bnchrch commented Feb 2, 2023

/test connector=connectors/source-github

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

Build Failed

Test summary info:

=========================== short test summary info ============================
FAILED test_core.py::TestConnection::test_check[inputs0] - Failed: Timeout >3...
FAILED test_core.py::TestConnection::test_check[inputs1] - Failed: Timeout >3...
FAILED test_core.py::TestDiscovery::test_discover[inputs0] - Failed: Timeout ...
FAILED test_core.py::TestDiscovery::test_discover[inputs1] - Failed: Timeout ...
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
SKIPPED [2] ../usr/local/lib/python3.9/site-packages/source_acceptance_test/tests/test_core.py:509: The previous and actual discovered catalogs are identical.
============= 4 failed, 41 passed, 3 skipped in 547.99s (0:09:07) ==============

@bnchrch bnchrch force-pushed the bechurch/22240-add-emitted-at-sat branch from 1454711 to 9cd8923 Compare February 2, 2023 19:44
@bnchrch
Copy link
Contributor Author

bnchrch commented Feb 2, 2023

/test connector=connectors/source-alloydb

🕑 connectors/source-alloydb https://github.com/airbytehq/airbyte/actions/runs/4078094045
✅ connectors/source-alloydb https://github.com/airbytehq/airbyte/actions/runs/4078094045
No Python unittests run

Build Passed

Test summary info:

All Passed

@bnchrch
Copy link
Contributor Author

bnchrch commented Feb 2, 2023

/test connector=connectors/source-bigquery

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

	 Name                                                    Stmts   Miss  Cover   Missing
	 -------------------------------------------------------------------------------------
	 connector_acceptance_test/base.py                          12      4    67%   16-19
	 connector_acceptance_test/config.py                       141      5    96%   87, 93, 239, 243-244
	 connector_acceptance_test/conftest.py                     211     95    55%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-285, 293-306, 311-317, 324-335, 342-358
	 connector_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 connector_acceptance_test/tests/test_core.py              494    117    76%   55, 60, 99-110, 115-122, 126-127, 131-132, 382, 402, 440, 478-495, 508-519, 523-528, 534, 567-572, 610-617, 660-662, 665, 730-738, 750-753, 758, 814-815, 821, 824, 892-902, 915-940
	 connector_acceptance_test/tests/test_incremental.py       160     14    91%   58-65, 70-83, 246
	 connector_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 connector_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 connector_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 connector_acceptance_test/utils/connector_runner.py       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 connector_acceptance_test/utils/json_schema_helper.py     114     13    89%   31-32, 39, 42, 66-69, 97, 121, 203-205
	 -------------------------------------------------------------------------------------
	 TOTAL                                                    1708    341    80%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:509: The previous and actual discovered catalogs are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_incremental.py:30: `future_state` not specified, skipping.
=================== 35 passed, 2 skipped in 63.33s (0:01:03) ===================

@bnchrch
Copy link
Contributor Author

bnchrch commented Feb 2, 2023

/test connector=connectors/source-google-sheets

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

Name                                                Stmts   Miss  Cover
-----------------------------------------------------------------------
google_sheets_source/models/spreadsheet_values.py      12      0   100%
google_sheets_source/models/spreadsheet.py             34      0   100%
google_sheets_source/models/__init__.py                 2      0   100%
google_sheets_source/__init__.py                        2      0   100%
google_sheets_source/helpers.py                       139     26    81%
google_sheets_source/client.py                         23      6    74%
google_sheets_source/google_sheets_source.py          107     85    21%
-----------------------------------------------------------------------
TOTAL                                                 319    117    63%
	 Name                                                    Stmts   Miss  Cover   Missing
	 -------------------------------------------------------------------------------------
	 connector_acceptance_test/base.py                          12      4    67%   16-19
	 connector_acceptance_test/config.py                       141      5    96%   87, 93, 239, 243-244
	 connector_acceptance_test/conftest.py                     211     95    55%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-285, 293-306, 311-317, 324-335, 342-358
	 connector_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 connector_acceptance_test/tests/test_core.py              494    117    76%   55, 60, 99-110, 115-122, 126-127, 131-132, 382, 402, 440, 478-495, 508-519, 523-528, 534, 567-572, 610-617, 660-662, 665, 730-738, 750-753, 758, 814-815, 821, 824, 892-902, 915-940
	 connector_acceptance_test/tests/test_incremental.py       160     14    91%   58-65, 70-83, 246
	 connector_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 connector_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 connector_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 connector_acceptance_test/utils/connector_runner.py       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 connector_acceptance_test/utils/json_schema_helper.py     114     13    89%   31-32, 39, 42, 66-69, 97, 121, 203-205
	 -------------------------------------------------------------------------------------
	 TOTAL                                                    1708    341    80%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: Incremental sync are not supported on this connector
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:509: The previous and actual discovered catalogs are identical.
================== 35 passed, 3 skipped in 171.67s (0:02:51) ===================

@bnchrch
Copy link
Contributor Author

bnchrch commented Feb 2, 2023

/test connector=connectors/source-metabase

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

	 Name                                                    Stmts   Miss  Cover   Missing
	 -------------------------------------------------------------------------------------
	 connector_acceptance_test/base.py                          12      4    67%   16-19
	 connector_acceptance_test/config.py                       141      5    96%   87, 93, 239, 243-244
	 connector_acceptance_test/conftest.py                     211     95    55%   36, 42-44, 49, 54, 77, 83, 89-91, 110, 115-117, 123-125, 131-132, 137-138, 143, 149, 158-167, 173-178, 193, 217, 248, 254, 262-267, 275-285, 293-306, 311-317, 324-335, 342-358
	 connector_acceptance_test/plugin.py                        69     25    64%   22-23, 31, 36, 120-140, 144-148
	 connector_acceptance_test/tests/test_core.py              494    117    76%   55, 60, 99-110, 115-122, 126-127, 131-132, 382, 402, 440, 478-495, 508-519, 523-528, 534, 567-572, 610-617, 660-662, 665, 730-738, 750-753, 758, 814-815, 821, 824, 892-902, 915-940
	 connector_acceptance_test/tests/test_incremental.py       160     14    91%   58-65, 70-83, 246
	 connector_acceptance_test/utils/asserts.py                 39      2    95%   62-63
	 connector_acceptance_test/utils/common.py                  94     10    89%   16-17, 32-38, 72, 75
	 connector_acceptance_test/utils/compare.py                 62     23    63%   21-51, 68, 97-99
	 connector_acceptance_test/utils/connector_runner.py       133     33    75%   24-27, 46-47, 50-54, 57-58, 73-75, 78-80, 83-85, 88-90, 93-95, 124-125, 159-161, 208
	 connector_acceptance_test/utils/json_schema_helper.py     114     13    89%   31-32, 39, 42, 66-69, 97, 121, 203-205
	 -------------------------------------------------------------------------------------
	 TOTAL                                                    1708    341    80%

Build Passed

Test summary info:

=========================== short test summary info ============================
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/plugin.py:63: Skipping TestIncremental.test_two_sequential_reads: This connector does not implement incremental sync
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:100: The previous and actual specifications are identical.
SKIPPED [1] ../usr/local/lib/python3.9/site-packages/connector_acceptance_test/tests/test_core.py:509: The previous and actual discovered catalogs are identical.
======================== 34 passed, 3 skipped in 33.72s ========================

Comment on lines 858 to 861
# Compare two lists, ignoring order of elements.
diff = DeepDiff(first_read_records_data, second_read_records_data, ignore_order=True, report_repetition=True)

assert diff == {}, f"records should be the same on subsequent runs. Diff: {diff}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this "two subsequent runs" logic is already happening here. I would suggest adding your new assertions in TestFullRefresh.test_sequentials_reads to avoid code duplication and limit the number of READ attempts. Let me know if you think otherwise.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

limit the number of READ attempts

I think thats a good reason to change. Ill put up a commit shortly

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually @alafanechere .

How much of an impact do reads have to run time?

Should we always been trying to optomize for a low number of reads to run the SAT suite?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It highly depends on the connector under test but its definitely one of the most time-consuming step of the test.

Should we always been trying to optomize for a low number of reads to run the SAT suite?

I'd say we shall indeed optimize to lower the number of reads and implement caching whenever possible.

@bnchrch bnchrch force-pushed the bechurch/22240-add-emitted-at-sat branch from 9cd8923 to e52850b Compare February 7, 2023 02:27
@bnchrch bnchrch requested a review from alafanechere February 7, 2023 17:41
Copy link
Contributor

@alafanechere alafanechere left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the changes, only minor suggestions 😄

@bnchrch bnchrch force-pushed the bechurch/22240-add-emitted-at-sat branch from 5d0592f to a2c42d6 Compare February 8, 2023 23:46
@bnchrch
Copy link
Contributor Author

bnchrch commented Feb 9, 2023

/test connector=bases/connector-acceptance-test

🕑 bases/connector-acceptance-test https://github.com/airbytehq/airbyte/actions/runs/4130001012
✅ bases/connector-acceptance-test https://github.com/airbytehq/airbyte/actions/runs/4130001012
No Python unittests run

Build Passed

Test summary info:

All Passed

@bnchrch bnchrch merged commit 465e646 into master Feb 9, 2023
@bnchrch bnchrch deleted the bechurch/22240-add-emitted-at-sat branch February 9, 2023 16:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[SAT] Assert that record emitted_at values increase between subsequent runs
3 participants