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

Remove duplicate tests #890

Merged
merged 2 commits into from Feb 27, 2019
Merged

Remove duplicate tests #890

merged 2 commits into from Feb 27, 2019

Conversation

jbrockmendel
Copy link
Contributor

Add a check so that duplicates don't sneak back in.

Rename TestInputFormats to the more accurate TestInputTypes.

A couple more test-cleanup PRs coming up (one with an actual bugfix), trying to keep them small and orthogonal.

@@ -180,7 +182,7 @@ def test_ybd(self):
assert res == actual


class TestInputFormats(object):
class TestInputTypes(object):
Copy link
Member

Choose a reason for hiding this comment

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

Why Types instead of Formats?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because whats being tested here is different types: bytes vs unicode vs bytearray vs StringIO...

Copy link
Member

Choose a reason for hiding this comment

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

Ah, I was a bit thrown by the fact that the first test is just an empty string. I see now.

@jbrockmendel
Copy link
Contributor Author

Looks like no rebase needed.

@pganssle
Copy link
Member

@jbrockmendel Can you add a changelog?

@jbrockmendel
Copy link
Contributor Author

done

@pganssle pganssle changed the title CLN: Remove duplicate tests Remove duplicate tests Feb 27, 2019
Since all the TestInputFormats tests are about different input *types*,
TestInputTypes is a more descriptive name.
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.

None yet

2 participants