- Revision
- 186870
- Author
- [email protected]
- Date
- 2015-07-15 16:13:56 -0700 (Wed, 15 Jul 2015)
Log Message
Many test failures in scm_unittest.py
https://bugs.webkit.org/show_bug.cgi?id=143967
Patch by Dean Johnson <[email protected]> on 2015-07-15
Reviewed by Daniel Bates.
* Scripts/webkitpy/common/checkout/scm/detection.py:
(SCMDetector.detect_scm_system): Paths with symlinks are now resolved to
absolute canonical file paths. Two mutually exclusive issues cause this
to be a problem.
1) Python's os.path.relpath() function does not return correct relative
paths between two filepaths that point to the same file, if symlinks are
involved.
On Mac, /tmp points to /private/tmp
ex. os.path.relpath('/tmp', '/private/tmp')
returns '../../tmp'
What we want is actually just '.'
2) Git does not allow file paths to trace outside of
the Git repository. This means that if you have a repository in
/tmp and you refer to that repository when invoking a git command as
../tmp, Git will produce errors about working outside of the repository.
* Scripts/webkitpy/common/checkout/scm/git.py: Over time Git has changed
its default behavior and such, needed to be updated.
(Git.changed_files): '--' was added into the command so that patch_directories
were taken as positional arguments.
* Scripts/webkitpy/common/checkout/scm/scm_unittest.py: Changed tests and setup
in the GitSVNTest class to more closely emulate the version of Git that the tests
assumed a system had. Also fixed a small side-effect from the absolute canonical
path fix in SCMDetector.detect_scm_system
(SVNTestRepository.setup): A relative filepath was previously passed as the
checkout root to SCMDetector.detect_scm_system, but is now cleaned to
an absolute canonical path before being passed in. The failing test
was a "sanity check" that the svn.checkout_root and scm.checkout_root
were the same.
(GitSVNTest._setup_git_checkout):
1) Added "--prefix ''" option to `git clone` since Git changed its default
behavior in version 2.0.
2) The branch master was renamed to trunk to more closely emulate what
tests expected when they were written.
(GitSVNTest.test_changed_files_local_plus_working_copy): Two of the three
original tests failed because the tests expected the trunk branch
to produce its parent's commit, whereas Git merely provides the HEAD commit
for a given branch (trunk in this case). Based on other tests written
in the same commit, it appears these tests were failing from the point they
were written.
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (186869 => 186870)
--- trunk/Tools/ChangeLog 2015-07-15 23:04:41 UTC (rev 186869)
+++ trunk/Tools/ChangeLog 2015-07-15 23:13:56 UTC (rev 186870)
@@ -1,3 +1,50 @@
+2015-07-15 Dean Johnson <[email protected]>
+
+ Many test failures in scm_unittest.py
+ https://bugs.webkit.org/show_bug.cgi?id=143967
+
+ Reviewed by Daniel Bates.
+
+ * Scripts/webkitpy/common/checkout/scm/detection.py:
+ (SCMDetector.detect_scm_system): Paths with symlinks are now resolved to
+ absolute canonical file paths. Two mutually exclusive issues cause this
+ to be a problem.
+ 1) Python's os.path.relpath() function does not return correct relative
+ paths between two filepaths that point to the same file, if symlinks are
+ involved.
+ On Mac, /tmp points to /private/tmp
+ ex. os.path.relpath('/tmp', '/private/tmp')
+ returns '../../tmp'
+ What we want is actually just '.'
+ 2) Git does not allow file paths to trace outside of
+ the Git repository. This means that if you have a repository in
+ /tmp and you refer to that repository when invoking a git command as
+ ../tmp, Git will produce errors about working outside of the repository.
+ * Scripts/webkitpy/common/checkout/scm/git.py: Over time Git has changed
+ its default behavior and such, needed to be updated.
+ (Git.changed_files): '--' was added into the command so that patch_directories
+ were taken as positional arguments.
+ * Scripts/webkitpy/common/checkout/scm/scm_unittest.py: Changed tests and setup
+ in the GitSVNTest class to more closely emulate the version of Git that the tests
+ assumed a system had. Also fixed a small side-effect from the absolute canonical
+ path fix in SCMDetector.detect_scm_system
+ (SVNTestRepository.setup): A relative filepath was previously passed as the
+ checkout root to SCMDetector.detect_scm_system, but is now cleaned to
+ an absolute canonical path before being passed in. The failing test
+ was a "sanity check" that the svn.checkout_root and scm.checkout_root
+ were the same.
+ (GitSVNTest._setup_git_checkout):
+ 1) Added "--prefix ''" option to `git clone` since Git changed its default
+ behavior in version 2.0.
+ 2) The branch master was renamed to trunk to more closely emulate what
+ tests expected when they were written.
+ (GitSVNTest.test_changed_files_local_plus_working_copy): Two of the three
+ original tests failed because the tests expected the trunk branch
+ to produce its parent's commit, whereas Git merely provides the HEAD commit
+ for a given branch (trunk in this case). Based on other tests written
+ in the same commit, it appears these tests were failing from the point they
+ were written.
+
2015-07-14 Anders Carlsson <[email protected]>
Assertions.h should include ExportMacros.h
Modified: trunk/Tools/Scripts/webkitpy/common/checkout/scm/detection.py (186869 => 186870)
--- trunk/Tools/Scripts/webkitpy/common/checkout/scm/detection.py 2015-07-15 23:04:41 UTC (rev 186869)
+++ trunk/Tools/Scripts/webkitpy/common/checkout/scm/detection.py 2015-07-15 23:13:56 UTC (rev 186870)
@@ -63,16 +63,16 @@
return scm_system
def detect_scm_system(self, path, patch_directories=None):
- absolute_path = self._filesystem.abspath(path)
+ real_path = self._filesystem.realpath(path)
if patch_directories == []:
patch_directories = None
- if SVN.in_working_directory(absolute_path, executive=self._executive):
- return SVN(cwd=absolute_path, patch_directories=patch_directories, filesystem=self._filesystem, executive=self._executive)
+ if SVN.in_working_directory(real_path, executive=self._executive):
+ return SVN(cwd=real_path, patch_directories=patch_directories, filesystem=self._filesystem, executive=self._executive)
- if Git.in_working_directory(absolute_path, executive=self._executive):
- return Git(cwd=absolute_path, patch_directories=patch_directories, filesystem=self._filesystem, executive=self._executive)
+ if Git.in_working_directory(real_path, executive=self._executive):
+ return Git(cwd=real_path, patch_directories=patch_directories, filesystem=self._filesystem, executive=self._executive)
return None
Modified: trunk/Tools/Scripts/webkitpy/common/checkout/scm/git.py (186869 => 186870)
--- trunk/Tools/Scripts/webkitpy/common/checkout/scm/git.py 2015-07-15 23:04:41 UTC (rev 186869)
+++ trunk/Tools/Scripts/webkitpy/common/checkout/scm/git.py 2015-07-15 23:13:56 UTC (rev 186870)
@@ -216,7 +216,7 @@
def changed_files(self, git_commit=None):
# FIXME: --diff-filter could be used to avoid the "extract_filenames" step.
- status_command = [self.executable_name, 'diff', '-r', '--name-status', "--no-renames", "--no-ext-diff", "--full-index", self.merge_base(git_commit)]
+ status_command = [self.executable_name, 'diff', '-r', '--name-status', "--no-renames", "--no-ext-diff", "--full-index", self.merge_base(git_commit), '--']
status_command.extend(self._patch_directories)
# FIXME: I'm not sure we're returning the same set of files that SVN.changed_files is.
# Added (A), Copied (C), Deleted (D), Modified (M), Renamed (R)
Modified: trunk/Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py (186869 => 186870)
--- trunk/Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py 2015-07-15 23:04:41 UTC (rev 186869)
+++ trunk/Tools/Scripts/webkitpy/common/checkout/scm/scm_unittest.py 2015-07-15 23:13:56 UTC (rev 186870)
@@ -185,7 +185,7 @@
if not cached_svn_repo_path:
cached_svn_repo_path = cls._setup_mock_repo()
- test_object.temp_directory = tempfile.mkdtemp(suffix="svn_test")
+ test_object.temp_directory = os.path.realpath(tempfile.mkdtemp(suffix="svn_test"))
test_object.svn_repo_path = os.path.join(test_object.temp_directory, "repo")
test_object.svn_repo_url = "file://%s" % test_object.svn_repo_path
test_object.svn_checkout_path = os.path.join(test_object.temp_directory, "checkout")
@@ -1035,8 +1035,9 @@
def _setup_git_checkout(self):
self.git_checkout_path = tempfile.mkdtemp(suffix="git_test_checkout")
# --quiet doesn't make git svn silent, so we use run_silent to redirect output
- run_silent(['git', 'svn', 'clone', '-T', 'trunk', self.svn_repo_url, self.git_checkout_path])
+ run_silent(['git', 'svn', 'clone', '-T', 'trunk', '--prefix', '', self.svn_repo_url, self.git_checkout_path])
os.chdir(self.git_checkout_path)
+ run_silent(['git', 'branch', '-m', 'trunk'])
def _tear_down_git_checkout(self):
# Change back to a valid directory so that later calls to os.getcwd() do not fail.
@@ -1406,12 +1407,21 @@
self.assertIn('test_file_commit2', files)
# working copy should *not* be in the list.
- files = self.scm.changed_files('trunk..')
+ files = self.scm.changed_files('trunk..') # git diff trunk..HEAD
+ self.assertNotIn('test_file_commit1', files)
+ self.assertNotIn('test_file_commit2', files)
+
+ files = self.scm.changed_files('trunk^..') # git diff trunk^..HEAD
self.assertIn('test_file_commit1', files)
self.assertNotIn('test_file_commit2', files)
# working copy *should* be in the list.
- files = self.scm.changed_files('trunk....')
+ # .... is a hack for working copy changes to be included
+ files = self.scm.changed_files('trunk....') # git diff trunk
+ self.assertNotIn('test_file_commit1', files)
+ self.assertIn('test_file_commit2', files)
+
+ files = self.scm.changed_files('trunk^....') # git diff trunk^
self.assertIn('test_file_commit1', files)
self.assertIn('test_file_commit2', files)
