florianfesti / boxes

Boxes.py - laser cutting boxes and more
GNU General Public License v3.0
973 stars 354 forks source link

Unable to run pre-commit on master #688

Closed gauthier12 closed 2 months ago

gauthier12 commented 3 months ago

Describe the bug Hi I am unable to run pre-commit, even on a clean master branch. I think there is two parts, the first, easy to fix is to update the svg files but I have a strange problem where pyupgrade keeps adding blank line in boxes/generators/trafficlight.py. If I try do commit the modified version by pyupgrade, next time, pyupgrade will add a new line. I am not able to find if it is a problem with pyupgrade or with the file trafficlight.py. Does anyone had the same problem ? cheers

To Reproduce

 git clone https://github.com/florianfesti/boxes.git
cd boxes
pre-commit run --all-files

Expected behavior pre-commit report no error

Screenshots or Drawings

pre-commit run --all-files
trim trailing whitespace.................................................Passed
fix end of files.........................................................Passed
check yaml...............................................................Passed
check toml...........................................(no files to check)Skipped
check for added large files..............................................Passed
fix requirements.txt.....................................................Passed
check for case conflicts.................................................Passed
detect private key.......................................................Passed
check that executables have shebangs.....................................Passed
check that scripts with shebangs are executable..........................Passed
type annotations not comments............................................Passed
check for eval().........................................................Passed
rst ``code`` is two backticks............................................Passed
rst directives end with two colons.......................................Passed
rst ``inline code`` next to normal text..................................Passed
pyupgrade................................................................Failed
- hook id: pyupgrade
- exit code: 1
- files were modified by this hook

Rewriting boxes/generators/trafficlight.py

autoflake................................................................Passed
isort....................................................................Passed
mypy.....................................................................Passed
mypy.....................................................................Passed
mypy.....................................................................Passed
mypy.....................................................................Passed
mypy.....................................................................Passed
mypy.....................................................................Passed
mypy.....................................................................Passed
mypy.....................................................................Passed
rstcheck.................................................................Passed
shellcheck...............................................................Passed
codespell................................................................Passed
pytest...................................................................Failed
- hook id: pytest
- exit code: 1

============================= test session starts ==============================
platform linux -- Python 3.12.4, pytest-8.2.2, pluggy-1.5.0
rootdir: /home/gauthier/project/boxes2
collected 163 items                                                            

tests/test_svg.py ................F............F............s........... [ 33%]
........ss.................F.......................F...............F.... [ 77%]
............ss..s....................                                    [100%]

=================================== FAILURES ===================================
_____________________ TestSVG.test_generator[BrickSorter] ______________________

self = <test_svg.TestSVG object at 0x74e817ec44a0>
generator = <class 'boxes.generators.brick_sorter.BrickSorter'>
capsys = <_pytest.capture.CaptureFixture object at 0x74e8127bda30>

    @pytest.mark.parametrize(
        "generator",
        all_generators,
        ids=idfunc.__func__,
    )
    def test_generator(self, generator: type[boxes.Boxes], capsys) -> None:
        boxName = generator.__name__
        if boxName in self.avoidGenerator:
            pytest.skip("Skipped generator")
        box = generator()
        box.parseArgs("")
        box.metadata["reproducible"] = True
        box.open()
        box.render()
        boxData = box.close()

        out, err = capsys.readouterr()

        assert 100 < boxData.__sizeof__(), "No data generated."
        assert 0 == len(out), "Console output generated."
        assert 0 == len(err), "Console error generated."

        # Use external library lxml as cross-check.
        assert self.is_valid_xml_by_lxml(boxData.getvalue()) is True, "Invalid XML according to library lxml."

        file = Path(__file__).resolve().parent / 'data' / (boxName + '.svg')
        file.write_bytes(boxData.getvalue())

        # Use example data from repository as reference data.
        referenceData = Path(__file__).resolve().parent.parent / 'examples' / (boxName + '.svg')
        assert referenceData.exists() is True, "Reference file for comparison does not exist."
        assert referenceData.is_file() is True, "Reference file for comparison does not exist."
>       assert referenceData.read_bytes() == boxData.getvalue(), "SVG files are not equal. If change is intended, please update example files."
E       AssertionError: SVG files are not equal. If change is intended, please update example files.
E       assert b'<?xml versi...n</g>\n</svg>' == b'<?xml versi...n</g>\n</svg>'
E         
E         At index 878 diff: b' ' != b'e'
E         Use -v to get more diff

tests/test_svg.py:83: AssertionError
______________________ TestSVG.test_generator[Console2_0] ______________________

self = <test_svg.TestSVG object at 0x74e817ec4da0>
generator = <class 'boxes.generators.console2.Console2'>
capsys = <_pytest.capture.CaptureFixture object at 0x74e80b596b10>

    @pytest.mark.parametrize(
        "generator",
        all_generators,
        ids=idfunc.__func__,
    )
    def test_generator(self, generator: type[boxes.Boxes], capsys) -> None:
        boxName = generator.__name__
        if boxName in self.avoidGenerator:
            pytest.skip("Skipped generator")
        box = generator()
        box.parseArgs("")
        box.metadata["reproducible"] = True
        box.open()
        box.render()
        boxData = box.close()

        out, err = capsys.readouterr()

        assert 100 < boxData.__sizeof__(), "No data generated."
        assert 0 == len(out), "Console output generated."
        assert 0 == len(err), "Console error generated."

        # Use external library lxml as cross-check.
        assert self.is_valid_xml_by_lxml(boxData.getvalue()) is True, "Invalid XML according to library lxml."

        file = Path(__file__).resolve().parent / 'data' / (boxName + '.svg')
        file.write_bytes(boxData.getvalue())

        # Use example data from repository as reference data.
        referenceData = Path(__file__).resolve().parent.parent / 'examples' / (boxName + '.svg')
        assert referenceData.exists() is True, "Reference file for comparison does not exist."
        assert referenceData.is_file() is True, "Reference file for comparison does not exist."
>       assert referenceData.read_bytes() == boxData.getvalue(), "SVG files are not equal. If change is intended, please update example files."
E       AssertionError: SVG files are not equal. If change is intended, please update example files.
E       assert b'<?xml versi...n</g>\n</svg>' == b'<?xml versi...n</g>\n</svg>'
E         
E         At index 827 diff: b"'" != b's'
E         Use -v to get more diff

tests/test_svg.py:83: AssertionError
________________________ TestSVG.test_generator[Matrix] ________________________

self = <test_svg.TestSVG object at 0x74e817ec74a0>
generator = <class 'boxes.generators.matrix.Matrix'>
capsys = <_pytest.capture.CaptureFixture object at 0x74e8127de780>

    @pytest.mark.parametrize(
        "generator",
        all_generators,
        ids=idfunc.__func__,
    )
    def test_generator(self, generator: type[boxes.Boxes], capsys) -> None:
        boxName = generator.__name__
        if boxName in self.avoidGenerator:
            pytest.skip("Skipped generator")
        box = generator()
        box.parseArgs("")
        box.metadata["reproducible"] = True
        box.open()
        box.render()
        boxData = box.close()

        out, err = capsys.readouterr()

        assert 100 < boxData.__sizeof__(), "No data generated."
        assert 0 == len(out), "Console output generated."
        assert 0 == len(err), "Console error generated."

        # Use external library lxml as cross-check.
        assert self.is_valid_xml_by_lxml(boxData.getvalue()) is True, "Invalid XML according to library lxml."

        file = Path(__file__).resolve().parent / 'data' / (boxName + '.svg')
        file.write_bytes(boxData.getvalue())

        # Use example data from repository as reference data.
        referenceData = Path(__file__).resolve().parent.parent / 'examples' / (boxName + '.svg')
        assert referenceData.exists() is True, "Reference file for comparison does not exist."
        assert referenceData.is_file() is True, "Reference file for comparison does not exist."
>       assert referenceData.read_bytes() == boxData.getvalue(), "SVG files are not equal. If change is intended, please update example files."
E       AssertionError: SVG files are not equal. If change is intended, please update example files.
E       assert b'<?xml versi...n</g>\n</svg>' == b'<?xml versi...n</g>\n</svg>'
E         
E         At index 1970 diff: b' ' != b'e'
E         Use -v to get more diff

tests/test_svg.py:83: AssertionError
_______________________ TestSVG.test_generator[RackBox] ________________________

self = <test_svg.TestSVG object at 0x74e817ee86e0>
generator = <class 'boxes.generators.rackbox.RackBox'>
capsys = <_pytest.capture.CaptureFixture object at 0x74e80b575f10>

    @pytest.mark.parametrize(
        "generator",
        all_generators,
        ids=idfunc.__func__,
    )
    def test_generator(self, generator: type[boxes.Boxes], capsys) -> None:
        boxName = generator.__name__
        if boxName in self.avoidGenerator:
            pytest.skip("Skipped generator")
        box = generator()
        box.parseArgs("")
        box.metadata["reproducible"] = True
        box.open()
        box.render()
        boxData = box.close()

        out, err = capsys.readouterr()

        assert 100 < boxData.__sizeof__(), "No data generated."
        assert 0 == len(out), "Console output generated."
        assert 0 == len(err), "Console error generated."

        # Use external library lxml as cross-check.
        assert self.is_valid_xml_by_lxml(boxData.getvalue()) is True, "Invalid XML according to library lxml."

        file = Path(__file__).resolve().parent / 'data' / (boxName + '.svg')
        file.write_bytes(boxData.getvalue())

        # Use example data from repository as reference data.
        referenceData = Path(__file__).resolve().parent.parent / 'examples' / (boxName + '.svg')
        assert referenceData.exists() is True, "Reference file for comparison does not exist."
        assert referenceData.is_file() is True, "Reference file for comparison does not exist."
>       assert referenceData.read_bytes() == boxData.getvalue(), "SVG files are not equal. If change is intended, please update example files."
E       AssertionError: SVG files are not equal. If change is intended, please update example files.
E       assert b'<?xml versi...n</g>\n</svg>' == b'<?xml versi...n</g>\n</svg>'
E         
E         At index 18544 diff: b'4' != b'1'
E         Use -v to get more diff

tests/test_svg.py:83: AssertionError
______________________ TestSVG.test_generator[Console2_1] ______________________

self = <test_svg.TestSVG object at 0x74e817ee92e0>
generator = <class 'boxes.generators.console2.Console2'>
capsys = <_pytest.capture.CaptureFixture object at 0x74e81cbc7f50>

    @pytest.mark.parametrize(
        "generator",
        all_generators,
        ids=idfunc.__func__,
    )
    def test_generator(self, generator: type[boxes.Boxes], capsys) -> None:
        boxName = generator.__name__
        if boxName in self.avoidGenerator:
            pytest.skip("Skipped generator")
        box = generator()
        box.parseArgs("")
        box.metadata["reproducible"] = True
        box.open()
        box.render()
        boxData = box.close()

        out, err = capsys.readouterr()

        assert 100 < boxData.__sizeof__(), "No data generated."
        assert 0 == len(out), "Console output generated."
        assert 0 == len(err), "Console error generated."

        # Use external library lxml as cross-check.
        assert self.is_valid_xml_by_lxml(boxData.getvalue()) is True, "Invalid XML according to library lxml."

        file = Path(__file__).resolve().parent / 'data' / (boxName + '.svg')
        file.write_bytes(boxData.getvalue())

        # Use example data from repository as reference data.
        referenceData = Path(__file__).resolve().parent.parent / 'examples' / (boxName + '.svg')
        assert referenceData.exists() is True, "Reference file for comparison does not exist."
        assert referenceData.is_file() is True, "Reference file for comparison does not exist."
>       assert referenceData.read_bytes() == boxData.getvalue(), "SVG files are not equal. If change is intended, please update example files."
E       AssertionError: SVG files are not equal. If change is intended, please update example files.
E       assert b'<?xml versi...n</g>\n</svg>' == b'<?xml versi...n</g>\n</svg>'
E         
E         At index 827 diff: b"'" != b's'
E         Use -v to get more diff

tests/test_svg.py:83: AssertionError
=========================== short test summary info ============================
FAILED tests/test_svg.py::TestSVG::test_generator[BrickSorter] - AssertionError: SVG files are not equal. If change is intended, please upda...
FAILED tests/test_svg.py::TestSVG::test_generator[Console2_0] - AssertionError: SVG files are not equal. If change is intended, please upda...
FAILED tests/test_svg.py::TestSVG::test_generator[Matrix] - AssertionError: SVG files are not equal. If change is intended, please upda...
FAILED tests/test_svg.py::TestSVG::test_generator[RackBox] - AssertionError: SVG files are not equal. If change is intended, please upda...
FAILED tests/test_svg.py::TestSVG::test_generator[Console2_1] - AssertionError: SVG files are not equal. If change is intended, please upda...
=================== 5 failed, 152 passed, 6 skipped in 2.98s ===================
git diff boxes/generators/trafficlight.py
diff --git a/boxes/generators/trafficlight.py b/boxes/generators/trafficlight.py
index a9394b5..10e13c2 100644
--- a/boxes/generators/trafficlight.py
+++ b/boxes/generators/trafficlight.py
@@ -45,6 +45,7 @@ When turned by 90°, it can be also used to create a bottle holder."""

+
     def __init__(self) -> None:
         Boxes.__init__(self)

Additional context

marauder37 commented 3 months ago

FWIW, I can't reproduce this, it works for me as long as I generate the missing example SVGs.

You've tested with a clean repo but have you tried it with a fresh virtualenv? Does it happen with Python <3.12? Just guesses

florianfesti commented 2 months ago

OK, I fixed the example SVGs in f6b96ed5b5da4dc92364c91e17af0d21e33df26a.

The new line in traffic light still is an issue. Looks like pyupgrade does not agree between Python 3.8 and 3.12

https://github.com/florianfesti/boxes/actions/runs/9811458664/job/27093651415

florianfesti commented 2 months ago

OK, looks like 3dfaa24ea8eac73eaf4813374e02317c2248e522 and f3c6a44028409c8bdcb30846efb8f456c035383d fix this issue. I still think this is actually a bug in the pyupgrade plugin. Anyone interested please feel free to look deeper and report to pyupgrade upstream. I am just happy the test suite passes for now.

(Locally isort is unhappy with scripts/boxes_proxy.py but I'll ignore that for now)

florianfesti commented 2 months ago

Oh, and thanks for reporting and testing!