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.

Sign up for GitHub

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

Fix comparing _EOF with itself #20

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
86fd5f8
Make _EOF equal itself
pushfoo Oct 26, 2023
60de253
Add tests for _EOF
pushfoo Oct 26, 2023
703d4c2
Simplify EOF test fixtures
pushfoo Oct 27, 2023
a5723cb
Group EOF comparison tests into a class
pushfoo Oct 27, 2023
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
View file
Open in desktop
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
30 changes: 30 additions & 0 deletions tests/test_eof.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import pytest

from dahuffman.huffmancodec import _EOF


def test_eq():
Copy link
Owner

Choose a reason for hiding this comment

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

I don't think it's necessary to put these tests in a separate file.
I would just add them in the existing test_dahuffman.py file
Maybe move the tests and fixtures inside a class, e.g. TestEndOfFileSymbol. I'm guessing you used a separate file to keep the fixtures away from existing tests, but you can also achieve that by scoping them to a single class

assert _EOF == _EOF


@pytest.fixture(params=(b'a', b'\0', b'_EOF'))
def raw_value(request):
return request.param


@pytest.fixture(params=(lambda b: b.decode('utf-8'), bytes, tuple))
def of_type(request):
return request.param


@pytest.fixture
def to_compare(raw_value, of_type):
Copy link
Owner

Choose a reason for hiding this comment

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

I feel this construct of three fixtures is a bit convoluted and misses some useful things to test against.
I would just make a straight list of values to use, e.g.

[
 b"\0",
 b"abc",
 "abc",
 0, 
 100,
 -1000,
  False,
  True,
  None,
  [],
  (),
  {},
  [-100, 0, 10],
  (-100, 10),
  _EndOfFileSymbol()
]

return of_type(raw_value)


def test_lt(to_compare):
assert _EOF < to_compare


def test_gt(to_compare):
assert to_compare > _EOF