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 some missing tests #896

Merged
merged 11 commits into from
Apr 24, 2024
Merged

Add some missing tests #896

merged 11 commits into from
Apr 24, 2024

Conversation

sh-rp
Copy link
Collaborator

@sh-rp sh-rp commented Jan 16, 2024

Tasks

  • - test basic pyarrow lib
  • - test pyarrow normalize
  • - define columns and then load arrow table with incorrect order
  • - fix _dlt_load_id added only if needed
  • - test discard value and discard row in nested json: discard row in child table that has another child table should discard data from descendants
  • - test removal of x-normalizer tags (also when {})
  • - pydantic nested model test
  • - test freeze exception with Pydantic

original ticket: #787

Copy link

netlify bot commented Jan 16, 2024

Deploy Preview for dlt-hub-docs ready!

Name Link
🔨 Latest commit 607a3af
🔍 Latest deploy log https://app.netlify.com/sites/dlt-hub-docs/deploys/6627c68e5182a70008df5e13
😎 Deploy Preview https://deploy-preview-896--dlt-hub-docs.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@sh-rp sh-rp changed the title 787 - add missing tests Pydantic improvements Jan 16, 2024
@sh-rp sh-rp closed this Jan 16, 2024
@sh-rp sh-rp changed the title Pydantic improvements Add some missing tests Jan 17, 2024
@sh-rp sh-rp reopened this Jan 17, 2024
@sh-rp sh-rp force-pushed the d#/787-missing-tests branch 3 times, most recently from 60f9097 to 31b5ae8 Compare January 17, 2024 14:05
@sh-rp
Copy link
Collaborator Author

sh-rp commented Jan 17, 2024

@rudolfix questions:

  • what exactly needs to be tested on the pydantic nested models?
  • fix _dlt_load_id added only if needed -> what is the indent behavior here?

@sh-rp sh-rp force-pushed the d#/787-missing-tests branch from a03fc46 to 5adf9c5 Compare January 17, 2024 15:32
@rudolfix rudolfix self-requested a review January 22, 2024 10:34
@sh-rp sh-rp linked an issue Jan 22, 2024 that may be closed by this pull request
9 tasks
@sh-rp sh-rp force-pushed the d#/787-missing-tests branch from 9c14c69 to e183b1b Compare January 30, 2024 20:07
@sh-rp sh-rp marked this pull request as ready for review February 1, 2024 17:38
@sh-rp sh-rp self-assigned this Mar 18, 2024
@rudolfix rudolfix assigned rudolfix and unassigned sh-rp Mar 18, 2024
Copy link
Collaborator

@rudolfix rudolfix left a comment

Choose a reason for hiding this comment

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

LGTM! 2 tests need upgrade after few months of work

{"col1": 1},
]
)
columns = [new_column("_dlt_something", "bigint"), new_column("col2", "text")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

because of SCD2 only _dlt_versions and loads are removed. other columns may be added. check the normalize and pls fix the test

@@ -708,3 +709,32 @@ def assert_timestamp_data_type(load_storage: LoadStorage, data_type: TDataType)
event_schema = load_storage.normalized_packages.load_schema(loads[0])
# in raw normalize timestamp column must not be coerced to timestamp
assert event_schema.get_table_columns("event")["timestamp"]["data_type"] == data_type


def test_removal_of_normalizer_schema_section(raw_normalize: Normalize) -> None:
Copy link
Collaborator

Choose a reason for hiding this comment

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

so this test must be a little extended.

  1. we remove evolove-columns-once
  2. but we add seen-data to all tables that had data items (see shcema utils: has_table_seen_data)
    this is a good test, needs to be upgraded

@sh-rp sh-rp added ci full run the full load tests on pr and removed ci full run the full load tests on pr labels Apr 23, 2024
Copy link
Collaborator

@rudolfix rudolfix left a comment

Choose a reason for hiding this comment

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

LGTM!

@sh-rp sh-rp merged commit edcedd5 into devel Apr 24, 2024
48 checks passed
@rudolfix rudolfix deleted the d#/787-missing-tests branch May 17, 2024 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

implement missing arrow and data contracts tests
2 participants