Bug 223006 - REGRESSION: two webkitscmpy.test.svn_unittest.TestRemoteSvn tests are flaky failures
Summary: REGRESSION: two webkitscmpy.test.svn_unittest.TestRemoteSvn tests are flaky f...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: Other
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2021-03-09 16:31 PST by Ryan Haddad
Modified: 2021-03-12 11:05 PST (History)
7 users (show)

See Also:


Attachments
Patch (1.89 KB, patch)
2021-03-10 13:37 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Ryan Haddad 2021-03-09 16:31:56 PST
The following two webkitscmpy tests appear to have become flaky failures recently

https://results.webkit.org/?suite=webkitpy-tests&suite=webkitpy-tests&test=webkitscmpy.test.svn_unittest.TestRemoteSvn.test_identifier&test=webkitscmpy.test.svn_unittest.TestRemoteSvn.test_non_cannonical_identifiers

[1956/1972] webkitscmpy.test.svn_unittest.TestRemoteSvn.test_non_cannonical_identifiers erred:
  Traceback (most recent call last):
    File "/Volumes/Data/worker/catalina-release-tests-wk2/build/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/svn_unittest.py", line 305, in test_non_cannonical_identifiers
      self.assertEqual('2@trunk', str(remote.Svn(self.remote).commit(identifier='0@branch-a')))
    File "/Volumes/Data/worker/catalina-release-tests-wk2/build/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/remote/svn.py", line 374, in commit
      info = self.info(cached=True, branch=branch, revision=revision)
    File "/Volumes/Data/worker/catalina-release-tests-wk2/build/Tools/Scripts/libraries/webkitcorepy/webkitcorepy/decorators.py", line 55, in decorator
      value = function(*args, **kwargs)
    File "/Volumes/Data/worker/catalina-release-tests-wk2/build/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/remote/svn.py", line 124, in info
      }, data='<propfind xmlns="DAV:">\n'
    File "/Volumes/Data/worker/catalina-release-tests-wk2/build/Tools/Scripts/libraries/webkitcorepy/webkitcorepy/mocks/requests_.py", line 120, in <lambda>
      patch('requests.request', new=lambda *args, **kwargs: Session().request(*args, **kwargs)),
    File "/Volumes/Data/worker/catalina-release-tests-wk2/build/Tools/Scripts/libraries/webkitcorepy/webkitcorepy/mocks/requests_.py", line 115, in request
      return this.request(method, url, **kwargs)
    File "/Volumes/Data/worker/catalina-release-tests-wk2/build/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/mocks/remote/svn.py", line 263, in request
      datetime.fromtimestamp(commit.timestamp, tz=tzoffset(None, -7 * 60 * 60)).strftime('%Y-%m-%dT%H:%M:%S.103754Z'),
    File "/Volumes/Data/worker/catalina-release-tests-wk2/build/Tools/Scripts/libraries/autoinstalled/python-3/dateutil/tz/_common.py", line 140, in fromutc
      raise TypeError("fromutc() requires a datetime argument")
  TypeError: fromutc() requires a datetime argument


[1954/1972] webkitscmpy.test.svn_unittest.TestRemoteSvn.test_identifier erred:
  Traceback (most recent call last):
    File "/Volumes/Data/worker/bigsur-debug-tests-wk1/build/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/test/svn_unittest.py", line 294, in test_identifier
      self.assertEqual(1, remote.Svn(self.remote).commit(identifier='1@trunk').revision)
    File "/Volumes/Data/worker/bigsur-debug-tests-wk1/build/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/remote/svn.py", line 374, in commit
      info = self.info(cached=True, branch=branch, revision=revision)
    File "/Volumes/Data/worker/bigsur-debug-tests-wk1/build/Tools/Scripts/libraries/webkitcorepy/webkitcorepy/decorators.py", line 55, in decorator
      value = function(*args, **kwargs)
    File "/Volumes/Data/worker/bigsur-debug-tests-wk1/build/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/remote/svn.py", line 117, in info
      response = requests.request(
    File "/Volumes/Data/worker/bigsur-debug-tests-wk1/build/Tools/Scripts/libraries/webkitcorepy/webkitcorepy/mocks/requests_.py", line 120, in <lambda>
      patch('requests.request', new=lambda *args, **kwargs: Session().request(*args, **kwargs)),
    File "/Volumes/Data/worker/bigsur-debug-tests-wk1/build/Tools/Scripts/libraries/webkitcorepy/webkitcorepy/mocks/requests_.py", line 115, in request
      return this.request(method, url, **kwargs)
    File "/Volumes/Data/worker/bigsur-debug-tests-wk1/build/Tools/Scripts/libraries/webkitscmpy/webkitscmpy/mocks/remote/svn.py", line 263, in request
      datetime.fromtimestamp(commit.timestamp, tz=tzoffset(None, -7 * 60 * 60)).strftime('%Y-%m-%dT%H:%M:%S.103754Z'),
    File "/Volumes/Data/worker/bigsur-debug-tests-wk1/build/Tools/Scripts/libraries/autoinstalled/python-3/dateutil/tz/_common.py", line 140, in fromutc
      raise TypeError("fromutc() requires a datetime argument")
  TypeError: fromutc() requires a datetime argument
Comment 1 Ryan Haddad 2021-03-09 16:33:04 PST
Maybe related to https://trac.webkit.org/changeset/274127/webkit?
Comment 2 Jonathan Bedard 2021-03-09 17:05:32 PST
Do not understand how this can possibly be flakey, taking a closer look.
Comment 3 Lauro Moura 2021-03-10 12:18:48 PST
I also getting these locally kinda often, and a webkitpy patch failed once in EWS earlier today.

Adding some prints to `dateutil` where it is triggered (_common.py:140), the `dt` parameter is indeed a `datetime.datetime` object, but the imported `datetime` name in this module sometimes points to `FakeDateTime` (which is a subclass of `datetime.datetime`), making the `isinstance` call to return False.
Comment 4 Jonathan Bedard 2021-03-10 12:25:22 PST
(In reply to Lauro Moura from comment #3)
> I also getting these locally kinda often, and a webkitpy patch failed once
> in EWS earlier today.
> 
> Adding some prints to `dateutil` where it is triggered (_common.py:140), the
> `dt` parameter is indeed a `datetime.datetime` object, but the imported
> `datetime` name in this module sometimes points to `FakeDateTime` (which is
> a subclass of `datetime.datetime`), making the `isinstance` call to return
> False.

I have a partial change locally that just abandons timezone shenanigans entirely and prints the time as utc shifted 7 hours (which is California time for our SVN server, which is the whole point of all of this anyways)
Comment 5 Jonathan Bedard 2021-03-10 13:37:53 PST
Created attachment 422865 [details]
Patch
Comment 6 Lauro Moura 2021-03-10 13:50:21 PST
(In reply to Jonathan Bedard from comment #4)
> I have a partial change locally that just abandons timezone shenanigans
> entirely and prints the time as utc shifted 7 hours (which is California
> time for our SVN server, which is the whole point of all of this anyways)

Nice, hopefully this will be enough for this error.

Given the flakiness and the import issue, it kinda looks like a test patching datetime to mock it has this mocked datetime "leaking" to other tests not expecting it later, no?

Btw, I found this old freezegun patch with isinstance issues when working together with dateutil: https://github.com/spulec/freezegun/commit/0453e5f47efd1d0d79af973e5ddcf9fa83b2ca40
Comment 7 Lauro Moura 2021-03-11 21:03:00 PST
Tested here a few dozen times and no more flakiness nor regresssions.
Comment 8 EWS 2021-03-12 09:18:42 PST
commit-queue failed to commit attachment 422865 [details] to WebKit repository. To retry, please set cq+ flag again.
Comment 9 EWS 2021-03-12 11:04:07 PST
Committed r274366: <https://commits.webkit.org/r274366>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 422865 [details].
Comment 10 Radar WebKit Bug Importer 2021-03-12 11:05:20 PST
<rdar://problem/75369005>