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

Raise test coverage to 100% #92

Open
jugmac00 opened this issue Nov 28, 2020 · 6 comments
Open

Raise test coverage to 100% #92

jugmac00 opened this issue Nov 28, 2020 · 6 comments

Comments

@jugmac00
Copy link
Collaborator

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
Copy link
Collaborator Author

I tried to come up with a test for...

def systime(byte_arr, offset=0):
ft = uint64(byte_arr, offset)
try:
return datetime.utcfromtimestamp((ft - EPOCH_AS_FILETIME) / HUNDREDS_OF_NANOSECONDS)
except: # noqa: E722
microseconds = ft / 10
return (datetime(1601, 1, 1) + timedelta(microseconds=microseconds))

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?

@petri
Copy link
Member

petri commented Nov 28, 2020

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

@Beercow
Copy link
Contributor

Beercow commented Nov 29, 2020 via email

@jugmac00
Copy link
Collaborator Author

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:

def systime(byte_arr, offset=0):
ft = uint64(byte_arr, offset)
try:
return datetime.utcfromtimestamp((ft - EPOCH_AS_FILETIME) / HUNDREDS_OF_NANOSECONDS)
except: # noqa: E722
microseconds = ft / 10
return (datetime(1601, 1, 1) + timedelta(microseconds=microseconds))

@petri
Copy link
Member

petri commented Dec 12, 2020

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
Copy link
Contributor

Beercow commented Dec 12, 2020

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

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

No branches or pull requests

3 participants