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

bugfix: use correct time; fix brittle unit test #1336

Merged

Conversation

jzacsh
Copy link
Collaborator

@jzacsh jzacsh commented Feb 23, 2024

this is fixing a single time-mishandling issue that uncovered two orthogonal problems:

    1. "correct time" issue: json parsing dropped timezone/offset specifiers as we were using a hand-crafted formatter string. Switching us to a standard library one that accepts both Z alias (for greenwhich) and offsets (like +00:00). Gphotos api docs[a] (inlined into the code) shows we use the former, which is what the unit test uses as well.
    • this probably meant time metadata for content transferred was off by some number of due to timezone being dropped
    1. "brittle unit test": unit test was relying on machine-specific gradle test behavior, so an out of the box git checkout master had two failing tests for me[b] but not for our github ci/cd.
    • this was due to use of toString() (in our assertEquals) which relies on your locale
    • fixed now as we use java.time.Instant#equals instead.

[a]: https://developers.google.com/photos/library/reference/rest/v1/mediaItems#mediametadata lists creationTime as:

string (Timestamp format)

Time when the media item was first created (not when it was uploaded
to Google Photos).

A timestamp in RFC3339 UTC "Zulu" format, with nanosecond resolution
and up to nine fractional digits. Examples: "2014-10-02T15:01:23Z" and
"2014-10-02T15:01:23.045123456Z".

[b]: local unit test failure looked like this for me (I'm in midwest US):

org.opentest4j.AssertionFailedError:
    expected: <Mon Oct 02 22:33:38 UTC 2023>
    but was: <Mon Oct 02 22:33:38 CDT 2023>

this is fixing a single time-mishandling issue that uncovered two
orthogonal problems:

- 1. "correct time" issue: json parsing dropped timezone/offset
  specifiers as we were using a hand-crafted formatter string. Switching
  us to a standard library one that accepts both `Z` alias (for
  greenwhich) and offsets (like `+00:00`). Gphotos api docs[a] (inlined
  into the code) shows we use the former, which is what the unit test
  uses as well.
  - this probably meant time metadata for content transferred was off by
    some number of due to timezone being dropped
- 2. "brittle unit test": unit test was relying on machine-specific
  `gradle test` behavior, so an out of the box `git checkout master` had
  two failing tests for me[b] but not for our github ci/cd.
   - this was due to use of `toString()` (in our assertEquals) which relies on your locale
   - fixed now as we use `java.time.Instant#equals` instead.

[a]: https://developers.google.com/photos/library/reference/rest/v1/mediaItems#mediametadata lists `creationTime` as:
> string (Timestamp format)
>
> Time when the media item was first created (not when it was uploaded
> to Google Photos).
>
> A timestamp in RFC3339 UTC "Zulu" format, with nanosecond resolution
> and up to nine fractional digits. Examples: "2014-10-02T15:01:23Z" and
> "2014-10-02T15:01:23.045123456Z".

[b]: local unit test failure looked like this for me (I'm in midwest US):
```
org.opentest4j.AssertionFailedError:
    expected: <Mon Oct 02 22:33:38 UTC 2023>
    but was: <Mon Oct 02 22:33:38 CDT 2023>
```
@aksingh737 aksingh737 merged commit ce93fb6 into dtinit:master Feb 23, 2024
5 checks passed
@jzacsh jzacsh deleted the fix-incorrect-creationTime-handling branch February 23, 2024 23:32
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

Successfully merging this pull request may close these issues.

2 participants