Code review comment for lp:~vila/bzr/1116079-gzip-compat

John A Meinel (jameinel) wrote :

Hash: SHA1

On 2013-07-09 12:04, Vincent Ladeuil wrote:
> Vincent Ladeuil has proposed merging
> lp:~vila/bzr/1116079-gzip-compat into lp:bzr.
> Requested reviews: bzr-core (bzr-core) Related bugs: Bug #1116079
> in Bazaar: "Test
> bzrlib.tests.test_tuned_gzip.TestToGzip.test_enormous_chunk fails -
> potential regression in python2.7 2.7.3-15ubuntu1"
> For more details, see:
> has changed in 2.7, AFAIU, we don't really need
> anymore but the deprecation has never been
> completed.
> This fix does mainly two things:
> - fix assertToGzip so the failing test_enormous_chunks doesn't
> flood the ouput with 256*1024 'a large string\n' twice, i.e.
> 7.864.320 bytes ! I suspect the test writer never had this test
> fail...
> - catch up with internal design evolution.
> That's the minimal effort to get the test suite passing.

+ lraw, ldecoded = len(raw_bytes), len(decoded)
+ self.assertEqual(lraw, ldecoded,
+ 'Expecting data length %d, got %d' % (lraw,
+ self.assertEqual(raw_bytes, decoded)

Why not turn that into:

if raw_bytes != decoded:"Raw bytes did not match (not outputting due to size)")

Someone who wants to investigate can do a debug print, but we won't
dump 7MB that nobody can actively use if the length happens to match.

I would personally be fine just dropping tuned_gzip altogether in
favor of just using upstream's gzip (since it is only deprecated
formats anyway).

But this change is ok, too.

 review: approve


Version: GnuPG v1.4.13 (Cygwin)
Comment: Using GnuPG with Thunderbird -


review: Approve

« Back to merge proposal