adding round trip test for lzma

[Cheukting]

Adding the round trip test for lzma. Only option that is testing is the compresslever (preset) however, according to the documentation there are other options such as format=FORMAT_XZ, check=-1 Shall I add that? I know I can use st.build to add them (cannot use sampled from because some options are only available to certain settings)

[Cheukting]

Travis failed but I have run the tests locally right after I have clone the repo (before changing anything) and it failed, so hope it's not because of me 😛

[pganssle]

@Cheukting I believe this is failing because of the formatting check. If you run tox -e check locally it will reformat your code and you can commit that. (On mobile I can't really see the difference in the Travis diff, so I assume it's a whitespace thing).

[Zac-HD]

there are other options such as format=FORMAT_XZ, check=-1 Shall I add that? I know I can use st.builds() to add them (cannot use st.sampled_from() because some options are only available to certain settings)

Yes, please do! You can actually use st.sampled_from(), and then use hypothesis.assume() inside the test to reject any invalid combinations.

[Cheukting]

I break it into 2 cases, I think that's easier than using assume. However, I got this error:

======================================================================
ERROR: test_lzma_round_trip_format_xz (tests.test_compression.TestLZMA)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/cheuktingho/Contribution/stdlib-property-tests/tests/test_compression.py", line 53, in test_lzma_round_trip_format_xz
    payload=st.binary(),
  File "/Users/cheuktingho/.pyenv/versions/stdlib-test/lib/python3.7/site-packages/hypothesis/core.py", line 1102, in wrapped_test
    raise the_error_hypothesis_found
  File "/Users/cheuktingho/Contribution/stdlib-property-tests/tests/test_compression.py", line 62, in test_lzma_round_trip_format_xz
    payload, format="FORMAT_XZ", check=check, preset=compresslevel
  File "/Users/cheuktingho/.pyenv/versions/3.7.3/lib/python3.7/lzma.py", line 318, in compress
    comp = LZMACompressor(format, check, preset, filters)
TypeError: an integer is required (got type str)

Which I don't understand when I look at the source code: https://github.com/python/cpython/blob/c1f1ddf30a595c2bfa3c06e54fb03fa212cd28b5/Lib/zipfile.py#L612

Can't see where the arguments are taken here.

[Zac-HD]

Nice, that looks good.

The 'bug' in your test is that format="FORMAT_XZ" should be format=lzma.FORMAT_XZ - it's a module-level constant (which happens to be an integer imported from the _lzma module, but that's not relevant to us).

As a style issue, I think it would be easier for readers to compare these two tests if we pass the same arguments to each. That means adding format=st.just(lzma.FORMAT_XZ) to the first; though I think I prefer to omit the check argument (i.e. keep the status quo) over check=st.just(-1), since there doesn't seem to be a named constant for that default argument.

[Zac-HD]

Main hint is to pip install tox and then run tox locally - it'll do all the formatting for you, as well as installing all the dependencies in a virtual environment and running the tests.

[Zac-HD]

I suggest using @st.composite to write a custom strategy lzma_filters(), which should make reading the error messages a lot easier, at least.

After that I'd make the strategy less general, until the test passes, and just leave a # TODO: comment block explaining that we're only testing a small subset of possible filters. That's a much better starting point for future testers than the there-is-no-test status quo!

[Cheukting]

Yes, I can rewrite the filter strategy. Will try another push today.

[Cheukting]

I have rewritten the filter strategy, will try to loosen the test to make it pass.

[Zac-HD]

Rebase on master to get rid of this bit.

[Zac-HD]

Cross-reference: HypothesisWorks/hypothesis#2430

[Cheukting]

refactor again, will need some thought on the test cases. Will continue later today

[Zac-HD]

🎉

Thanks @Cheukting!