New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

chromite's ParseDateTest.testBasicFunctionality fails in 2018

Project Member Reported by derat@chromium.org, Jan 1 2018

Issue description

chromite appears to have at least one unit test that isn't 2018-safe:

https://luci-milo.appspot.com/buildbot/chromeos/betty-paladin/1816 (and a bunch of other builders):

...
chromite-0.0.2-r3615: 00:22:22: ERROR: ### LOG: scripts/merge_logs_unittest
chromite-0.0.2-r3615: [chromite.scripts.merge_logs_unittest] ParseAutoservDateTest.testBasicFunctionality ... ok
chromite-0.0.2-r3615: [chromite.scripts.merge_logs_unittest] ParseChromeDateTest.testBasicFunctionality ... ok
chromite-0.0.2-r3615: [chromite.scripts.merge_logs_unittest] ParseDateTest.testBasicFunctionality ... FAIL
chromite-0.0.2-r3615: [chromite.scripts.merge_logs_unittest] ParseDateTest.testUTC ... ok
chromite-0.0.2-r3615: [chromite.scripts.merge_logs_unittest] ParsePowerdDateTest.testBasicFunctionality ... ok
chromite-0.0.2-r3615: 
chromite-0.0.2-r3615: ======================================================================
chromite-0.0.2-r3615: FAIL: [chromite.scripts.merge_logs_unittest] ParseDateTest.testBasicFunctionality
chromite-0.0.2-r3615: ----------------------------------------------------------------------
chromite-0.0.2-r3615: Traceback (most recent call last):
chromite-0.0.2-r3615:   File "/mnt/host/source/chromite/lib/timeout_util.py", line 191, in TimeoutWrapper
chromite-0.0.2-r3615:     return func(*args, **kwargs)
chromite-0.0.2-r3615:   File "/mnt/host/source/chromite/scripts/merge_logs_unittest.py", line 45, in testBasicFunctionality
chromite-0.0.2-r3615:     TZOFFSET))
chromite-0.0.2-r3615: AssertionError: datetime.datetime(2018, 7, 31, 9, 17, 2, tzinfo=tzfile('/usr/share/zoneinfo/America/Los_Angeles')) != datetime.datetime(2017, 7, 31, 9, 17, 2, tzinfo=tzoffset(None, -25200))
chromite-0.0.2-r3615: 
chromite-0.0.2-r3615: ----------------------------------------------------------------------
chromite-0.0.2-r3615: Ran 5 tests in 0.007s
chromite-0.0.2-r3615: 
chromite-0.0.2-r3615: FAILED (failures=1)

And yeah, I can see why that would fail:

    # Status Log
    dt = merge_logs.ParseDate('Jul 31 09:17:02')
    self.assertEqual(dt, datetime.datetime(2017, 7, 31, 9, 17, 2, 0,
                                           TZOFFSET))
 
Project Member

Comment 1 by bugdroid1@chromium.org, Jan 1 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/7a58e83834df0b3e74e8497b0dcd92a74f06df3c

commit 7a58e83834df0b3e74e8497b0dcd92a74f06df3c
Author: Daniel Erat <derat@chromium.org>
Date: Mon Jan 01 11:59:26 2018

chromite: Fix ParseDateTest.testBasicFunctionality for 2018.

Update merge_logs_unittest.py's
ParseDateTest.testBasicFunctionality test, which had a
hardcoded assumption that the current year is 2017.

I think it'll still be possible for the updated test to fail
if we cross a year boundary between its merge_logs.ParseDate
and datetime.datetime.now calls. That's unlikely to happen,
but the right fix is probably to make ParseDate also take
the current time.

BUG= chromium:798207 
TEST=test failed before; now passes
TBR=davidriley@chromium.org

Change-Id: I0f496150681cc0983dcbc7f63fab308d60ef1faa
Reviewed-on: https://chromium-review.googlesource.com/846708
Reviewed-by: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>

[modify] https://crrev.com/7a58e83834df0b3e74e8497b0dcd92a74f06df3c/scripts/merge_logs_unittest.py

Comment 2 by derat@chromium.org, Jan 1 2018

Status: Fixed (was: Started)
Feel free to reopen the bug and take ownership if you want to update ParseDate to take the current time.

Comment 3 by derat@chromium.org, Jan 2 2018

I suspect that the underlying ParseDate function may be broken in real usage too, assuming that it's ever used to interpret timestamps from the previous year.
Cc: jrbarnette@chromium.org dgarr...@chromium.org pprabhu@chromium.org
 Issue 798218  has been merged into this issue.

Comment 5 by derat@chromium.org, Jan 2 2018

Labels: Merge-Request-64
Got a request on the change to merge to M64.
Project Member

Comment 6 by sheriffbot@chromium.org, Jan 2 2018

Labels: -Merge-Request-64 Hotlist-Merge-Review Merge-Review-64
This bug requires manual review: M64 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), kbleicher@(ChromeOS), abdulsyed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: Merge-Request-63
Requesting that this be merged back to 63 so I can build moblab.
Blocking: 797187
Cc: ayatane@chromium.org
 Issue 798570  has been merged into this issue.
Status: Assigned (was: Fixed)
Looks like this still needs to be merged into 63
And 64 (but I think that merge requests are evaluated regardless of a bug's status).

If someone else wants to see the merges through, feel free to take this bug. I just submitted a fix because the CQ was broken and I wanted to land my changes. :-P
i don't think this particular bug really needs to go through TPM review considering it has no effect on the build/release images themselves.  just cherry pick it back.
Looks like you're doing all of the merges. Thanks!
Issue 798649 has been merged into this issue.
Consider this merge approved for the release branches, I just marked them all ready back to 62.
Cc: gkihumba@chromium.org kbleicher@chromium.org
Labels: -Hotlist-Merge-Review -Merge-Request-63 -Merge-Review-64 Merge-Approved-64 Merge-Approved-62 Merge-Approved-63
Project Member

Comment 18 by bugdroid1@chromium.org, Jan 3 2018

Labels: merge-merged-release-R63-10032.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/7ea6f6917b467ae9b83dc3ff8d14d750509b4ded

commit 7ea6f6917b467ae9b83dc3ff8d14d750509b4ded
Author: Daniel Erat <derat@chromium.org>
Date: Wed Jan 03 17:14:21 2018

chromite: Fix ParseDateTest.testBasicFunctionality for 2018.

Update merge_logs_unittest.py's
ParseDateTest.testBasicFunctionality test, which had a
hardcoded assumption that the current year is 2017.

I think it'll still be possible for the updated test to fail
if we cross a year boundary between its merge_logs.ParseDate
and datetime.datetime.now calls. That's unlikely to happen,
but the right fix is probably to make ParseDate also take
the current time.

BUG= chromium:798207 
TEST=test failed before; now passes
TBR=davidriley@chromium.org

Change-Id: I0f496150681cc0983dcbc7f63fab308d60ef1faa
Reviewed-on: https://chromium-review.googlesource.com/846708
Reviewed-by: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
(cherry picked from commit 7a58e83834df0b3e74e8497b0dcd92a74f06df3c)
Reviewed-on: https://chromium-review.googlesource.com/848635
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
Commit-Queue: Bernie Thompson <bhthompson@chromium.org>
Tested-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/7ea6f6917b467ae9b83dc3ff8d14d750509b4ded/scripts/merge_logs_unittest.py

Project Member

Comment 19 by bugdroid1@chromium.org, Jan 3 2018

Labels: merge-merged-release-R62-9901.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/2288de41078368a2ee4335fcff01430525840902

commit 2288de41078368a2ee4335fcff01430525840902
Author: Daniel Erat <derat@chromium.org>
Date: Wed Jan 03 17:14:56 2018

chromite: Fix ParseDateTest.testBasicFunctionality for 2018.

Update merge_logs_unittest.py's
ParseDateTest.testBasicFunctionality test, which had a
hardcoded assumption that the current year is 2017.

I think it'll still be possible for the updated test to fail
if we cross a year boundary between its merge_logs.ParseDate
and datetime.datetime.now calls. That's unlikely to happen,
but the right fix is probably to make ParseDate also take
the current time.

BUG= chromium:798207 
TEST=test failed before; now passes
TBR=davidriley@chromium.org

Change-Id: I0f496150681cc0983dcbc7f63fab308d60ef1faa
(cherry picked from commit 7a58e83834df0b3e74e8497b0dcd92a74f06df3c)
Reviewed-on: https://chromium-review.googlesource.com/848636
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
Commit-Queue: Bernie Thompson <bhthompson@chromium.org>
Tested-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/2288de41078368a2ee4335fcff01430525840902/scripts/merge_logs_unittest.py

Project Member

Comment 20 by bugdroid1@chromium.org, Jan 3 2018

Labels: merge-merged-release-R64-10176.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/2eb7e0ff5925fa193ae38546a66695f9aad5b0ba

commit 2eb7e0ff5925fa193ae38546a66695f9aad5b0ba
Author: Daniel Erat <derat@chromium.org>
Date: Wed Jan 03 17:14:59 2018

chromite: Fix ParseDateTest.testBasicFunctionality for 2018.

Update merge_logs_unittest.py's
ParseDateTest.testBasicFunctionality test, which had a
hardcoded assumption that the current year is 2017.

I think it'll still be possible for the updated test to fail
if we cross a year boundary between its merge_logs.ParseDate
and datetime.datetime.now calls. That's unlikely to happen,
but the right fix is probably to make ParseDate also take
the current time.

BUG= chromium:798207 
TEST=test failed before; now passes
TBR=davidriley@chromium.org

Change-Id: I0f496150681cc0983dcbc7f63fab308d60ef1faa
(cherry picked from commit 7a58e83834df0b3e74e8497b0dcd92a74f06df3c)
Reviewed-on: https://chromium-review.googlesource.com/848634
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
Commit-Queue: Bernie Thompson <bhthompson@chromium.org>
Tested-by: Bernie Thompson <bhthompson@chromium.org>

[modify] https://crrev.com/2eb7e0ff5925fa193ae38546a66695f9aad5b0ba/scripts/merge_logs_unittest.py

Issue 798702 has been merged into this issue.
Project Member

Comment 22 by bugdroid1@chromium.org, Jan 3 2018

Labels: merge-merged-firmware-fizz-10139.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/1e307ada3ad47475404846e8c1c4352aa63934e2

commit 1e307ada3ad47475404846e8c1c4352aa63934e2
Author: Daniel Erat <derat@chromium.org>
Date: Wed Jan 03 22:38:28 2018

chromite: Fix ParseDateTest.testBasicFunctionality for 2018.

Update merge_logs_unittest.py's
ParseDateTest.testBasicFunctionality test, which had a
hardcoded assumption that the current year is 2017.

I think it'll still be possible for the updated test to fail
if we cross a year boundary between its merge_logs.ParseDate
and datetime.datetime.now calls. That's unlikely to happen,
but the right fix is probably to make ParseDate also take
the current time.

BUG= chromium:798207 
TEST=test failed before; now passes
TBR=davidriley@chromium.org

Change-Id: I0f496150681cc0983dcbc7f63fab308d60ef1faa
(cherry picked from commit 7a58e83834df0b3e74e8497b0dcd92a74f06df3c)
Reviewed-on: https://chromium-review.googlesource.com/848637
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/1e307ada3ad47475404846e8c1c4352aa63934e2/scripts/merge_logs_unittest.py

Project Member

Comment 23 by bugdroid1@chromium.org, Jan 3 2018

Labels: merge-merged-firmware-coral-10068.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/819ac03630295cab709011a2e77318d97666824a

commit 819ac03630295cab709011a2e77318d97666824a
Author: Daniel Erat <derat@chromium.org>
Date: Wed Jan 03 22:38:31 2018

chromite: Fix ParseDateTest.testBasicFunctionality for 2018.

Update merge_logs_unittest.py's
ParseDateTest.testBasicFunctionality test, which had a
hardcoded assumption that the current year is 2017.

I think it'll still be possible for the updated test to fail
if we cross a year boundary between its merge_logs.ParseDate
and datetime.datetime.now calls. That's unlikely to happen,
but the right fix is probably to make ParseDate also take
the current time.

BUG= chromium:798207 
TEST=test failed before; now passes
TBR=davidriley@chromium.org

Change-Id: I0f496150681cc0983dcbc7f63fab308d60ef1faa
(cherry picked from commit 7a58e83834df0b3e74e8497b0dcd92a74f06df3c)
Reviewed-on: https://chromium-review.googlesource.com/848638
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/819ac03630295cab709011a2e77318d97666824a/scripts/merge_logs_unittest.py

Project Member

Comment 24 by bugdroid1@chromium.org, Jan 3 2018

Labels: merge-merged-firmware-scribe-10045.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/faab476f02e32fd5461ae80c4a6d63229a353794

commit faab476f02e32fd5461ae80c4a6d63229a353794
Author: Daniel Erat <derat@chromium.org>
Date: Wed Jan 03 22:38:32 2018

chromite: Fix ParseDateTest.testBasicFunctionality for 2018.

Update merge_logs_unittest.py's
ParseDateTest.testBasicFunctionality test, which had a
hardcoded assumption that the current year is 2017.

I think it'll still be possible for the updated test to fail
if we cross a year boundary between its merge_logs.ParseDate
and datetime.datetime.now calls. That's unlikely to happen,
but the right fix is probably to make ParseDate also take
the current time.

BUG= chromium:798207 
TEST=test failed before; now passes
TBR=davidriley@chromium.org

Change-Id: I0f496150681cc0983dcbc7f63fab308d60ef1faa
(cherry picked from commit 7a58e83834df0b3e74e8497b0dcd92a74f06df3c)
Reviewed-on: https://chromium-review.googlesource.com/848639
Reviewed-by: Dan Erat <derat@chromium.org>
Commit-Queue: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>

[modify] https://crrev.com/faab476f02e32fd5461ae80c4a6d63229a353794/scripts/merge_logs_unittest.py

Status: Fixed (was: Assigned)
Project Member

Comment 26 by bugdroid1@chromium.org, Jan 9 2018

Labels: merge-merged-factory-scarlet-10211.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/d22728ddb13f3536520b5621fe2ca93bf7acf35b

commit d22728ddb13f3536520b5621fe2ca93bf7acf35b
Author: Daniel Erat <derat@chromium.org>
Date: Tue Jan 09 07:45:38 2018

chromite: Fix ParseDateTest.testBasicFunctionality for 2018.

Update merge_logs_unittest.py's
ParseDateTest.testBasicFunctionality test, which had a
hardcoded assumption that the current year is 2017.

I think it'll still be possible for the updated test to fail
if we cross a year boundary between its merge_logs.ParseDate
and datetime.datetime.now calls. That's unlikely to happen,
but the right fix is probably to make ParseDate also take
the current time.

BUG= chromium:798207 
TEST=test failed before; now passes
TBR=davidriley@chromium.org

Change-Id: I0f496150681cc0983dcbc7f63fab308d60ef1faa
Reviewed-on: https://chromium-review.googlesource.com/846708
Reviewed-by: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
(cherry picked from commit 7a58e83834df0b3e74e8497b0dcd92a74f06df3c)
Reviewed-on: https://chromium-review.googlesource.com/856284
Reviewed-by: Youcheng Syu <youcheng@chromium.org>
Commit-Queue: Youcheng Syu <youcheng@chromium.org>
Tested-by: Youcheng Syu <youcheng@chromium.org>
Trybot-Ready: Youcheng Syu <youcheng@chromium.org>

[modify] https://crrev.com/d22728ddb13f3536520b5621fe2ca93bf7acf35b/scripts/merge_logs_unittest.py

Project Member

Comment 27 by bugdroid1@chromium.org, Jan 9 2018

Labels: merge-merged-factory-fizz-10167.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/cd2e1b3ff907634484603edf26febae98683baca

commit cd2e1b3ff907634484603edf26febae98683baca
Author: Daniel Erat <derat@chromium.org>
Date: Tue Jan 09 09:33:27 2018

chromite: Fix ParseDateTest.testBasicFunctionality for 2018.

Update merge_logs_unittest.py's
ParseDateTest.testBasicFunctionality test, which had a
hardcoded assumption that the current year is 2017.

I think it'll still be possible for the updated test to fail
if we cross a year boundary between its merge_logs.ParseDate
and datetime.datetime.now calls. That's unlikely to happen,
but the right fix is probably to make ParseDate also take
the current time.

BUG= chromium:798207 
TEST=test failed before; now passes
TBR=davidriley@chromium.org

Change-Id: I0f496150681cc0983dcbc7f63fab308d60ef1faa
Reviewed-on: https://chromium-review.googlesource.com/846708
Reviewed-by: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
(cherry picked from commit 7a58e83834df0b3e74e8497b0dcd92a74f06df3c)
Reviewed-on: https://chromium-review.googlesource.com/856179
Reviewed-by: Yilun Lin <yllin@chromium.org>
Commit-Queue: Yilun Lin <yllin@chromium.org>
Tested-by: Yilun Lin <yllin@chromium.org>

[modify] https://crrev.com/cd2e1b3ff907634484603edf26febae98683baca/scripts/merge_logs_unittest.py

Project Member

Comment 28 by bugdroid1@chromium.org, Jan 9 2018

Labels: merge-merged-stabilize-10032.56.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/a48c5560dc4c91f1cbd8bcf3256acc2e84437de8

commit a48c5560dc4c91f1cbd8bcf3256acc2e84437de8
Author: Daniel Erat <derat@chromium.org>
Date: Tue Jan 09 23:01:36 2018

chromite: Fix ParseDateTest.testBasicFunctionality for 2018.

Update merge_logs_unittest.py's
ParseDateTest.testBasicFunctionality test, which had a
hardcoded assumption that the current year is 2017.

I think it'll still be possible for the updated test to fail
if we cross a year boundary between its merge_logs.ParseDate
and datetime.datetime.now calls. That's unlikely to happen,
but the right fix is probably to make ParseDate also take
the current time.

BUG= chromium:798207 
TEST=test failed before; now passes
TBR=davidriley@chromium.org

Change-Id: I0f496150681cc0983dcbc7f63fab308d60ef1faa
Reviewed-on: https://chromium-review.googlesource.com/846708
Reviewed-by: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
(cherry picked from commit 7a58e83834df0b3e74e8497b0dcd92a74f06df3c)
Reviewed-on: https://chromium-review.googlesource.com/848635
Reviewed-by: Bernie Thompson <bhthompson@chromium.org>
Commit-Queue: Bernie Thompson <bhthompson@chromium.org>
Tested-by: Bernie Thompson <bhthompson@chromium.org>
(cherry picked from commit 7ea6f6917b467ae9b83dc3ff8d14d750509b4ded)
Reviewed-on: https://chromium-review.googlesource.com/857754
Reviewed-by: Yi Ren <renyi@google.com>
Reviewed-by: Brian Sheehan <bsheehan@google.com>
Tested-by: Brian Sheehan <bsheehan@google.com>
Commit-Queue: Yi Ren <renyi@google.com>
Commit-Queue: Brian Sheehan <bsheehan@google.com>

[modify] https://crrev.com/a48c5560dc4c91f1cbd8bcf3256acc2e84437de8/scripts/merge_logs_unittest.py

Project Member

Comment 29 by bugdroid1@chromium.org, Jan 12 2018

Labels: merge-merged-factory-coral-10122.B
The following revision refers to this bug:
  https://chromium.googlesource.com/chromiumos/chromite/+/a7350c072ed1811085bb62b6aec1aa4060c6d1cd

commit a7350c072ed1811085bb62b6aec1aa4060c6d1cd
Author: Daniel Erat <derat@chromium.org>
Date: Fri Jan 12 06:44:43 2018

chromite: Fix ParseDateTest.testBasicFunctionality for 2018.

Update merge_logs_unittest.py's
ParseDateTest.testBasicFunctionality test, which had a
hardcoded assumption that the current year is 2017.

I think it'll still be possible for the updated test to fail
if we cross a year boundary between its merge_logs.ParseDate
and datetime.datetime.now calls. That's unlikely to happen,
but the right fix is probably to make ParseDate also take
the current time.

BUG= chromium:798207 
TEST=test failed before; now passes
TBR=davidriley@chromium.org

Change-Id: I0f496150681cc0983dcbc7f63fab308d60ef1faa
Reviewed-on: https://chromium-review.googlesource.com/846708
Reviewed-by: Dan Erat <derat@chromium.org>
Tested-by: Dan Erat <derat@chromium.org>
(cherry picked from commit 7a58e83834df0b3e74e8497b0dcd92a74f06df3c)
Reviewed-on: https://chromium-review.googlesource.com/858876
Reviewed-by: Marco Chen <marcochen@chromium.org>
Commit-Queue: Marco Chen <marcochen@chromium.org>
Tested-by: Marco Chen <marcochen@chromium.org>

[modify] https://crrev.com/a7350c072ed1811085bb62b6aec1aa4060c6d1cd/scripts/merge_logs_unittest.py

Project Member

Comment 30 by sheriffbot@chromium.org, Jan 19 2018

Cc: haddowk@chromium.org bhthompson@google.com
This issue has been approved for a merge. Please merge the fix to any appropriate branches as soon as possible!

If all merges have been completed, please remove any remaining Merge-Approved labels from this issue.

Thanks for your time! To disable nags, add the Disable-Nags label.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot

Comment 31 by derat@chromium.org, Jan 19 2018

Labels: -Merge-Approved-62 -Merge-Approved-63 -Merge-Approved-64

Sign in to add a comment