-
Notifications
You must be signed in to change notification settings - Fork 93
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
feat: foreign key on delete cascade action testing and samples #910
Conversation
Here is the summary of changes. You are about to add 3 region tags.
This comment is generated by snippet-bot.
|
def test_create_table_with_foreign_key_delete_cascade(capsys, instance_id, sample_database): | ||
snippets.create_table_with_foreign_key_delete_cascade(instance_id, sample_database.database_id) | ||
out, _ = capsys.readouterr() | ||
assert "Created Customers and ShoppingCarts table with FKShoppingCartsCustomerId" in out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
May be instead of checking the output message we should verify that the table was actually created/ altered by the sample or not ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For IT, yes but for samples we just need the to check if an error is not thrown
|
||
|
||
@pytest.mark.dependency(name="alter_table_with_foreign_key_delete_cascade", | ||
depends=["create_table_with_foreign_key_delete_cascade"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi, As we are adding Foreign Key Delete Cascade
constraint , this should Not depend on sample which has already added the constraints while creating the tables, isn't it ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is common in samples to have this dependency to reduce the time to run these
Co-authored-by: Vishwaraj Anand <vishwaraj.anand00@gmail.com>
Co-authored-by: Vishwaraj Anand <vishwaraj.anand00@gmail.com>
tests/system/test_database_api.py
Outdated
|
||
|
||
DBAPI_OPERATION_TIMEOUT = 240 # seconds | ||
FKDCA_CUSTOMERS_COLUMNS = ("CustomerId", "CustomerName") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Some places we are using FKADC
and other places we are using FKDCA
. It will be good to use same terminology everywhere.
No description provided.