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

Fix comparing _EOF with itself #20

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 8 additions & 7 deletions dahuffman/huffmancodec.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,17 +21,18 @@ class _EndOfFileSymbol:
def __repr__(self) -> str:
return "_EOF"

def __eq__(self, other) -> bool:
return other.__class__ == self.__class__

# Because _EOF will be compared with normal symbols (strings, bytes),
# we have to provide a minimal set of comparison methods.
# We'll make _EOF smaller than the rest (meaning lowest frequency)
# we have to provide a minimal set of comparison methods. We'll make
# _EOF smaller than all values other than itself since there will
# always be 1 and only 1 end of file per file.
def __lt__(self, other) -> bool:
return True
return self != other

def __gt__(self, other) -> bool:
return False

def __eq__(self, other) -> bool:
return other.__class__ == self.__class__
return self == other

def __hash__(self) -> int:
return hash(self.__class__)
Expand Down
35 changes: 35 additions & 0 deletions tests/test_eof.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import pytest

from dahuffman.huffmancodec import _EOF


@pytest.fixture(params=(
b"\0",
b"abc",
"abc",
0,
100,
-1000,
False,
True,
None,
[],
(),
{},
[-100, 0, 10],
(-100, 10))
)
def to_compare(request):
Copy link
Owner

Choose a reason for hiding this comment

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

What I meant in a previous comment was pytest allows you to define the fixture as a method of TestEndOfFileSymbol (instead of a module level function), which makes it available only to other methods of that class

return request.param


class TestEndOfFileSymbol:

def test_eq(self):
assert _EOF == _EOF
Copy link
Owner

Choose a reason for hiding this comment

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

maybe also test equality _EOF == _EndOfFileSymbol()


def test_lt(self, to_compare):
assert _EOF < to_compare

def test_gt(self, to_compare):
assert to_compare > _EOF
Copy link
Owner

Choose a reason for hiding this comment

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

I would also assert for !=, and maybe just put everything in same test function to keep it simple:

assert _EOF < to_compare
assert _EOF <= to_compare
assert to_compare > _EOF
assert to_compare >= _EOF
assert to_compare != _EOF
assert not (to_compare == _EOF)

Loading