koodaamo / tnefparse

a TNEF decoding library written in python, without external dependencies
GNU Lesser General Public License v3.0
49 stars 37 forks source link

Raise test coverage to 100% #92

Open jugmac00 opened 3 years ago

jugmac00 commented 3 years ago

While test coverage is already at a very good 93%, the PR #84 alone would have brought two regressions (changed log level + no more help text), as there are still some lines without coverage.

I suggest to fill the coverage gaps before continuing to work on the current open PRs.

jugmac00 commented 3 years ago

I tried to come up with a test for... https://github.com/koodaamo/tnefparse/blob/d29d976e58c4faab789e6a47f726334b442a727a/tnefparse/util.py#L41-L47

I even wrote a helper to iterate over date/time ranges, but I found no value which fails for the try block and succeeds for the except block.

The change was introduced in https://github.com/koodaamo/tnefparse/pull/54/commits/be35566012772caf7cbf70c4c11b94b242a77d41

@Beercow @petri Maybe you know or remember why this try/except statement was introduced in the first place?

petri commented 3 years ago

I don't remember. But I bet this is related: https://stackoverflow.com/questions/10849717/what-is-the-significance-of-january-1-1601

Beercow commented 3 years ago

It has to do with an error where microseconds are too large

http://statgen.us/files/software/seqpower/usr/local/lib/anaconda2/pkgs/pillow-3.1.1-py27_0/lib/python2.7/site-packages/PIL/OleFileIO.py

Thank you in advance,

Brian Maloney, GCIH, GCFE, GCFA

On Nov 28, 2020, at 12:42 PM, Jürgen Gmach notifications@github.com wrote:

 I tried to come up with a test for... https://github.com/koodaamo/tnefparse/blob/d29d976e58c4faab789e6a47f726334b442a727a/tnefparse/util.py#L41-L47

I even wrote a helper to iterate over date/time ranges, but I found no value which fails for the try block and succeeds for the except block.

The change was introduced in be35566

@Beercow @petri Maybe you know or remember why this try/except statement was introduced in the first place?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

jugmac00 commented 3 years ago

Hi @Beercow

thank you very much for getting back to me.

As you seem to have some more domain knowledge in this topic, could you give me a microsecond number which is too large, but fits the condition in the except branch?

        microseconds = ft / 10
        return (datetime(1601, 1, 1) + timedelta(microseconds=microseconds))

As I wrote above, I did not find a number on my own.

Here is the complete code snippet: https://github.com/koodaamo/tnefparse/blob/f81554d410a00c93c7713663baf17a0b7016e452/tnefparse/util.py#L41-L47

petri commented 3 years ago

Maybe this is simply to overcome the fact that the max for python timestamps and dates is different? https://stackoverflow.com/questions/39153700/why-cant-pythons-datetime-max-survive-a-round-trip-through-timestamp-fromtim

Beercow commented 3 years ago

Unfortunately, I do not have the file I was working with anymore. Trying to find one that brings up the error.