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 coverage tests for tables.py #568

Closed
wants to merge 2 commits into from

Conversation

canlasla
Copy link

@canlasla canlasla commented Dec 7, 2022

[x] Wrote test for feature
[ ] Added changes to CHANGELOG.md

Changes proposed:
Added coverage tests to tables.py for issue #476

Copy link
Member

@adnanhemani adnanhemani 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 this!

@@ -1157,6 +1157,9 @@ def test_remove_column_type(table):
# Create #
##########

def test_empty_labels_none():
Copy link
Member

Choose a reason for hiding this comment

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

I think you've actually stumbled on a bug here - this shouldn't have thrown an Error. Can you remove this test case and I'll create a ticket in the backlog to remove this method altogether.

Copy link
Member

Choose a reason for hiding this comment

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

Update: I've solved this bug in #570

@adnanhemani
Copy link
Member

Closing this review as none of the test cases added are applicable to the code as it stands now. Thank you for your efforts :)

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.

2 participants