Standard Tests: allow specifying exact record matches #2186 #2960
Conversation
keu
requested review from
sherifnada,
yevhenii-ldv and
vitaliizazmic
and removed request for
ChristopheDuong and
cgardens
| path = getattr(inputs, "expected_records_path") | ||
| if not path: | ||
| return [] | ||
|
|
||
| with open(str(base_path / path)) as f: | ||
| return [AirbyteMessage.parse_raw(line) for line in f] | ||
| return [AirbyteRecordMessage.parse_raw(line) for line in f] |
There was a problem hiding this comment.
why not make this AirbyteMessage in case we want to do some state as well?
There was a problem hiding this comment.
my logic here was the following:
- it is expected
records - STATE objects are kind of black boxes IIRC
- I don't think we should assert on LOG messages
- it is easier to provide records, ideally just
dataattribute, but we need to know the stream name
+ extra_fields + exact_order + extra_records
keu
force-pushed
the
keu/standard-python-tests-records-assert
branch
from
365d9eb
to
7822264
Compare
| @@ -13,6 +13,11 @@ tests: | |||
| - config_path: "secrets/config.json" | |||
| configured_catalog_path: "sample_files/configured_catalog.json" | |||
| validate_output_from_all_streams: yes | |||
| expect_records: | |||
There was a problem hiding this comment.
why not call it expected_records? it follows the expected/actual naming convention for tests
There was a problem hiding this comment.
originally it was so, but in the test we have expected_records fixture that an actual records we expect, and config class inputs.expected_records (inputs.expected_records.extra_fields, etc) which might cause confusion.
docs/contributing-to-airbyte/building-new-connector/source-acceptance-tests.md
Outdated
Show resolved
Hide resolved
docs/contributing-to-airbyte/building-new-connector/source-acceptance-tests.md
Outdated
Show resolved
Hide resolved
| | `expect_records.exact_order` | boolean | False | Ensure that records produced in exact same order| | ||
| | `expect_records.extra_records` | boolean | True | Allow connector to produce extra records, but still enforce all records from the expected file to be produced| | ||
|
|
||
| `expect_records` is a nested configuration, if omitted - the part of the test responsible for record matching will be skipped. Due to the fact that we can't identify records without primary keys matching flags support only the following combinations: |
There was a problem hiding this comment.
are Airbyte field e.g: emitted_at counted?
There was a problem hiding this comment.
no, we explicitly remove metadata, only record itself used for comparision
There was a problem hiding this comment.
the only reason I pack expected records into AirbyteRecordMessages and not just dicts is that we need the name of the stream and I don't want to have separate file for each stream
airbyte-integrations/bases/source-acceptance-test/source_acceptance_test/tests/test_core.py
Outdated
Show resolved
Hide resolved
…eptance-tests.md Co-authored-by: Sherif A. Nada <snadalive@gmail.com>
…tance_test/tests/test_core.py Co-authored-by: Sherif A. Nada <snadalive@gmail.com>
…eptance-tests.md Co-authored-by: Sherif A. Nada <snadalive@gmail.com>
closes #2186
What
Describe what the change is solving
It helps to add screenshots if it affects the frontend.
How
Describe the solution
Recommended reading order
test.javacomponent.ts