Code review comment for lp:~louis/apport/apport-unpack-extract

Hello Martin,

All the changes that you requested have been implemented except for one that I don't understand :

> +
> + def test_extract(self):
> + '''extract() with various binary elements.'''
> +
> + # create a test report with binary elements
> + large_val = b'A' * 5000000
> +
> + pr = problem_report.ProblemReport()
> + pr['Txt'] = 'some text'
> + pr['MoreTxt'] = 'some more text'
> + pr['Foo'] = problem_report.CompressedValue(b'FooFoo!')
> + pr['Uncompressed'] = bin_data
> + pr['Bin'] = problem_report.CompressedValue()
> + pr['Bin'].set_value(bin_data)
> + pr['Large'] = problem_report.CompressedValue(large_val)
> + pr['Multiline'] = problem_report.CompressedValue(b'\1\1\1\n\2\2\n\3\3\3')

> This is really binary, so I would just drop this bit.

I'm not sure of what should be dropped.

The other slight change is the request to replace item by key : the key variable was already in use in the method, so I replaced item by bin_key to discriminate.

I also improved the test by avoiding repetition and testing the exceptions.

Let me know if there is anything else to be done.

Kind regards,

...Louis