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: diffing excessively large snapshot lines #778

Merged
merged 4 commits into from
Jul 20, 2023
Merged

Conversation

iamogbz
Copy link
Collaborator

@iamogbz iamogbz commented Jul 20, 2023

Description

Limit the amount of characters used when comparing line diffs.

Can be validated by changing DIFF_LINE_WIDTH_LIMIT to a value like 1000000 and running inv test -t test_diff_large_lines

  • Also fixes test_unicode

Related Issues

Checklist

  • This PR has sufficient documentation.
  • This PR has sufficient test coverage.
  • This PR title satisfies semantic convention.

Additional Comments

def test_large_binary_diff(snapshot):
    b = b"\x01" * 1000000
    assert b == snapshot.use_extension(SingleFileSnapshotExtension)
(syrupy-py3.11) ➜  syrupy git:(test-single-file-fix) ✗ inv test -vv -t test_large_binary_diff
============================== test session starts ==============================
platform darwin -- Python 3.11.4, pytest-7.4.0, pluggy-1.2.0 -- /Users/Emmanuel/Sources/github/tophat/syrupy/.venv/bin/python
cachedir: .pytest_cache
benchmark: 4.0.0 (defaults: timer=time.perf_counter disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /Users/Emmanuel/Sources/github/tophat/syrupy
configfile: pyproject.toml
plugins: syrupy-4.0.7, xdist-3.3.1, benchmark-4.0.0
collected 259 items / 258 deselected / 1 selected                               

tests/syrupy/extensions/test_single_file.py::test_large_binary_diff FAILED

=================================== FAILURES ====================================
____________________________ test_large_binary_diff _____________________________

snapshot = SnapshotAssertion(name='snapshot', num_executions=0)

    def test_large_binary_diff(snapshot):
        b = b"\x01" * 1000000
>       assert b == snapshot.use_extension(SingleFileSnapshotExtension)
E       AssertionError: assert [+ received] == [- snapshot]
E         Snapshot 'test_large_binary_diff' does not exist!
E         + b'\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x01\x...

tests/syrupy/extensions/test_single_file.py:77: AssertionError
---------------------------- snapshot report summary ----------------------------
1 snapshot failed.
============================ short test summary info ============================
FAILED tests/syrupy/extensions/test_single_file.py::test_large_binary_diff - AssertionError: assert [+ received] == [- snapshot]
======================= 1 failed, 258 deselected in 0.21s =======================

Follow up to #776

@iamogbz iamogbz requested a review from noahnu July 20, 2023 18:50
Comment on lines -49 to +50
def test_diff_large_lines(self, Reporter, osenv):
n_count = 5000
def test_diff_large_lines(self, Reporter, snapshot, osenv):
n_count = 1000000
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

5000 was not large enough apparently

@iamogbz iamogbz marked this pull request as ready for review July 20, 2023 18:56
assert snapshot_utf8 == "apple"
assert snapshot_utf8 == content
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops

@noahnu noahnu merged commit 64b4265 into main Jul 20, 2023
@noahnu noahnu deleted the test-single-file-fix branch July 20, 2023 21:35
tophat-opensource-bot pushed a commit that referenced this pull request Jul 20, 2023
## [4.0.8](v4.0.7...v4.0.8) (2023-07-20)

### Bug Fixes

* diffing excessively large snapshot lines ([#778](#778)) ([64b4265](64b4265))
@tophat-opensource-bot
Copy link
Contributor

🎉 This PR is included in version 4.0.8 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tests hang when diffing moderately sized binary file
3 participants