Fixing AuthenticationError when running test-webkitpy as a non-committer.
Created attachment 181855 [details] Patch
Attachment 181855 [details] did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Tools/ChangeLog', u'Tools/Scripts/webkitpy..." exit_code: 1 /mnt/git/webkit-style-queue/Tools/Scripts/webkitpy/thirdparty/autoinstalled/pylint/lint.py:393: DeprecationWarning: disable-msg is deprecated, replace it by disable (/mnt/git/webkit-style-queue/Tools/Scripts/webkitpy/common/checkout/scm/svn.py, line # ignore false positives for methods implemented in the mixee class. pylint: disable-msg=E1101 ) DeprecationWarning) Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py:304: [SCMTest._shared_test_changed_files] Instance of 'SCMTest' has no 'scm' member [pylint/E1101] [5] Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py:308: [SCMTest._shared_test_changed_files] Instance of 'SCMTest' has no 'scm' member [pylint/E1101] [5] Total errors found: 2 in 4 files If any of these errors are false positives, please file a bug against check-webkit-style.
Comment on attachment 181855 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181855&action=review > Tools/ChangeLog:7 > + Could you explain a bit more here about what sort of changes you're making here? It's not immediately clear if this is all search-replace or one-off-fixes or what? > Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py:128 > +class SVNTestRepository(object): Welcome to our cave. Do you like our charcoal buffalo? > Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py:345 > + self.assertEqual(sorted(changed_files), sorted(["test_dir/test_file3", "test_file"])) I wonder if fancy new unitests has a sort-for-you method. Not that this is hard. We've talked of moving to unittests2 since we can't quite drop 2.6 yet. (We're held back by the last version of Mac OS X which Apple WebKit supports, as we try not to have contributors have to install any more than absolutely necessary). > Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py:1055 > + self.scm = detect_scm_system(self.git_checkout_path) I believe there is a newer SCMDetector object which is smarter than this free-function. But that's another change for another time. :) > Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py:1555 > + cwd = tempfile.mkdtemp(suffix="git_test_with_mock") Why do the mock tests need a real tempfile? Don't they work from a MockFileSystem()?
Mostly I just need more context for the cleanup. Or it's possible there is a search-replace part of this which would be trivial to land first? (even if it didn't change behavior/fix tests)
Comment on attachment 181855 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181855&action=review >> Tools/ChangeLog:7 >> + > > Could you explain a bit more here about what sort of changes you're making here? It's not immediately clear if this is all search-replace or one-off-fixes or what? There is a couple of things which I've changed; * Sort the list inputs into a deterministic order (see the added sorted()) * Change all tests to use self.scm rather then a local scm * Make the self.scm object override the svn_auth_realm >> Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py:345 >> + self.assertEqual(sorted(changed_files), sorted(["test_dir/test_file3", "test_file"])) > > I wonder if fancy new unitests has a sort-for-you method. Not that this is hard. We've talked of moving to unittests2 since we can't quite drop 2.6 yet. (We're held back by the last version of Mac OS X which Apple WebKit supports, as we try not to have contributors have to install any more than absolutely necessary). Yes, they have an "assertXXXEqual" which doesn't compare order. >> Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py:1055 >> + self.scm = detect_scm_system(self.git_checkout_path) > > I believe there is a newer SCMDetector object which is smarter than this free-function. But that's another change for another time. :) Agreed >> Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py:1555 >> + cwd = tempfile.mkdtemp(suffix="git_test_with_mock") > > Why do the mock tests need a real tempfile? Don't they work from a MockFileSystem()? cwd can't be none. Some of these tests are actually still broken.
Comment on attachment 181855 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181855&action=review >>> Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py:1555 >>> + cwd = tempfile.mkdtemp(suffix="git_test_with_mock") >> >> Why do the mock tests need a real tempfile? Don't they work from a MockFileSystem()? > > cwd can't be none. Some of these tests are actually still broken. I see, Git is still a real object and uses a real FileSystem (because it doesn't even take a FileSystem parameter!) so it needs a real path?
Created attachment 182098 [details] Patch
Comment on attachment 182098 [details] Patch LGTM.
Comment on attachment 182098 [details] Patch Rejecting attachment 182098 [details] from commit-queue. Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', u'--status-host=queues.webkit.org', ..." exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 13309 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 53>At revision 13309. ________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' ________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' Updating webkit projects from gyp files... Full output: http://queues.webkit.org/results/15808013
The following ChangeLog files contain OOPS: trunk/Tools/ChangeLog Please don't ever say "OOPS" in a ChangeLog file.
Comment on attachment 182098 [details] Patch Your patch has two ChangeLog entries in one ChangeLog. Please use one ChangeLog entry per patch.
Created attachment 182241 [details] Patch
Before: Ran 1733 tests in 40.638s FAILED (failures=0, errors=22) After: Ran 1733 tests in 43.084s FAILED (failures=1, errors=8)
Comment on attachment 182241 [details] Patch LGTM.
Comment on attachment 182241 [details] Patch Clearing flags on attachment: 182241 Committed r139399: <http://trac.webkit.org/changeset/139399>
All reviewed patches have been landed. Closing bug.