Skip to content

Refactor test for ADD NON NULL column and change default expected behavior#16885

Merged
findepi merged 3 commits intomasterfrom
findepi/add-non-null-test
Apr 14, 2023
Merged

Refactor test for ADD NON NULL column and change default expected behavior#16885
findepi merged 3 commits intomasterfrom
findepi/add-non-null-test

Conversation

@findepi
Copy link
Copy Markdown
Member

@findepi findepi commented Apr 5, 2023

Separately test case when table is empty and when it is not.
Also, change default expected behavior: adding a NON NULL column to a
non-empty table should generally fail, since DEFAULT value was not
provided.

@findepi findepi added test no-release-notes This pull request does not require release notes entry labels Apr 5, 2023
@cla-bot cla-bot bot added the cla-signed label Apr 5, 2023
@findepi findepi marked this pull request as draft April 5, 2023 08:17
@findepi findepi force-pushed the findepi/add-non-null-test branch from f04ad64 to 237ea21 Compare April 5, 2023 08:17
@findepi findepi changed the title Split test for ADD NON NULL column into two Refactor test for ADD NON NULL column and change default expected behavior Apr 5, 2023
@findepi findepi force-pushed the findepi/add-non-null-test branch 2 times, most recently from 5f2ff3b to fb60700 Compare April 5, 2023 08:22
@github-actions github-actions bot added the iceberg Iceberg connector label Apr 5, 2023
@findepi findepi force-pushed the findepi/add-non-null-test branch from fb60700 to e47bb4e Compare April 5, 2023 08:43
@findepi findepi marked this pull request as ready for review April 5, 2023 08:43
@findepi findepi requested review from ebyhr and hashhar April 5, 2023 08:43
}

@Test
public void testAddNotNullColumn()
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

i intentionally didn't use testAddNotNullColumnToNonEmptyTable to help identity overrides which became redundant now that we changed what behavior we expect by default.

@findepi findepi force-pushed the findepi/add-non-null-test branch from e47bb4e to 32168bb Compare April 11, 2023 13:07
@findepi
Copy link
Copy Markdown
Member Author

findepi commented Apr 11, 2023

CI ignite #16671 #16980 (fix: #16963)

@findepi findepi force-pushed the findepi/add-non-null-test branch 6 times, most recently from 3eac870 to c927843 Compare April 12, 2023 18:54
findepi added 3 commits April 13, 2023 14:16
Previously, when list has wrong length, their contents were not provided
in the failure message:

    java.lang.AssertionError: lists don't have the same size expected [4] but found [5]
Separately test case when table is empty and when it is not.
Also, change default expected behavior: adding a NON NULL column to a
non-empty table should generally fail, since DEFAULT value was not
provided.
@findepi findepi force-pushed the findepi/add-non-null-test branch from c927843 to eefc7dd Compare April 13, 2023 12:18
@findepi
Copy link
Copy Markdown
Member Author

findepi commented Apr 14, 2023

CI #17033

@findepi findepi merged commit 1fe2b4e into master Apr 14, 2023
@findepi findepi deleted the findepi/add-non-null-test branch April 14, 2023 11:35
@github-actions github-actions bot added this to the 414 milestone Apr 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed iceberg Iceberg connector no-release-notes This pull request does not require release notes entry test

Development

Successfully merging this pull request may close these issues.

3 participants