Skip to content

Tests#153

Open
kvlaskarolina wants to merge 6 commits intomasterfrom
test
Open

Tests#153
kvlaskarolina wants to merge 6 commits intomasterfrom
test

Conversation

@kvlaskarolina
Copy link
Copy Markdown
Contributor

@kvlaskarolina kvlaskarolina commented Jan 9, 2026

cd backend
pytest printing/tests/printing/processing/test_pages.py --cov
pytest printing/tests/printing/processing/test_imposition.py --cov
pytest printing/tests/printing/processing/test_final_pages.py --cov
pytest printing/tests/printing/processing/test_converter.py --cov

Copy link
Copy Markdown
Contributor

@piotr-3-janiszewski piotr-3-janiszewski left a comment

Choose a reason for hiding this comment

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

I think your tests are great! The code isn't hard to read, both the structure and naming makes sense. I personally (might be totally wrong, but I) think that there are many places in your code where adding a fixture might enable you to cover more edge cases without much effort. Great job 🥇 🎆 💯 👍 🏆

Comment thread backend/printing/tests/printing/processing/test_final_pages.py
Comment thread backend/printing/tests/printing/processing/test_final_pages.py Outdated
)

@pytest.fixture
def mock_page_sizes():
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe we could add more edge cases? For example a square page? Or a very long page?

from pypdf import PdfReader, PdfWriter, PageObject

from printing.processing.pages import PageSize, PageSizes, PageOrientation

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The following two tests could be merged into one with a fixture and could cover more edge cases. Like being a square.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree, it's worth testing the page size utility functions on different sizes, especially covering the cases:

  • width/height < 1/2
  • width/height = 1/2
  • width/height > 1/2, < 1
  • width/height = 1
  • width/height > 1, < 2
  • width/height = 2
  • width/height > 2
    The first and last case are especially confusing and thus error-prone, because splitting such pages along either edge does not change the page orientation. Make sure to test the page splitting utilities as well.

Comment thread backend/printing/tests/printing/processing/test_pages.py Outdated
Copy link
Copy Markdown
Member

@dominik-korsa dominik-korsa left a comment

Choose a reason for hiding this comment

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

See the comments for some quick notes, please also wait for #154 to get merged to main an update this branch afterwards, since the changes from #154 are relevant to this PR

Comment thread backend/.coverage
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

.coverage files should be ignored, this has been done in #154

Comment thread backend/printing/tests/comftest.py Outdated
Comment thread backend/printing/tests/printing/processing/test_pages.py Outdated
Comment thread backend/printing/tests/printing/processing/test_final_pages.py Outdated
@kvlaskarolina kvlaskarolina changed the title Testy Tests Jan 12, 2026
Comment thread backend/printing/tests/comftest.py Outdated
Comment thread backend/printing/tests/printing/processing/test_pages.py Outdated
from pypdf import PdfReader, PdfWriter, PageObject

from printing.processing.pages import PageSize, PageSizes, PageOrientation

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree, it's worth testing the page size utility functions on different sizes, especially covering the cases:

  • width/height < 1/2
  • width/height = 1/2
  • width/height > 1/2, < 1
  • width/height = 1
  • width/height > 1, < 2
  • width/height = 2
  • width/height > 2
    The first and last case are especially confusing and thus error-prone, because splitting such pages along either edge does not change the page orientation. Make sure to test the page splitting utilities as well.

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.

3 participants