Bug 52691 – new-run-webkit-tests: remove use of os.walk, use mock filesystem for better unit testing

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Dirk Pranke]

new-run-webkit-tests: remove use of os.walk, use mock filesystem for better unit testing

[Dirk Pranke]

Created attachment 79369 [details]
Patch

[Dirk Pranke]

Created attachment 79375 [details]
revise approach to files_under() so we can handle reftests properly

[Mihai Parparita]

Comment on attachment 79375 [details]
revise approach to files_under() so we can handle reftests properly

View in context: https://bugs.webkit.org/attachment.cgi?id=79375&action=review

> Tools/Scripts/webkitpy/common/system/filesystem.py:76
> +        dirs_to_skip = dirs_to_skip or []

You can use [] as the default argument value to the function, since you're not modifying the list.

> Tools/Scripts/webkitpy/common/system/filesystem_mock.py:75
> +        dirs_to_skip = dirs_to_skip or []

Same here.

> Tools/Scripts/webkitpy/common/system/filesystem_mock.py:100
> +                    continue

This "continue" will only apply to the loop over dirs_to_skip, but I think you want to continue the iteration in the self.files loop.

[Dirk Pranke]

(In reply to comment #3)
> (From update of attachment 79375 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=79375&action=review
> 
> > Tools/Scripts/webkitpy/common/system/filesystem.py:76
> > +        dirs_to_skip = dirs_to_skip or []
> 
> You can use [] as the default argument value to the function, since you're not modifying the list.
>

Fair enough. Done.
 
> > Tools/Scripts/webkitpy/common/system/filesystem_mock.py:75
> > +        dirs_to_skip = dirs_to_skip or []
> 
> Same here.
>

Ditto. 
 
> > Tools/Scripts/webkitpy/common/system/filesystem_mock.py:100
> > +                    continue
> 
> This "continue" will only apply to the loop over dirs_to_skip, but I think you want to continue the iteration in the self.files loop.

Good catch. Fixed.

[Dirk Pranke]

Created attachment 79462 [details]
update w/ feedback from mihaip's review

[Dirk Pranke]

Created attachment 79470 [details]
pull strcats out of loop in MockFileSystem.files_under()

[Dirk Pranke]

Comment on attachment 79470 [details]
pull strcats out of loop in MockFileSystem.files_under()

Clearing flags on attachment: 79470

Committed r76155: <http://trac.webkit.org/changeset/76155>

[Dirk Pranke]

All reviewed patches have been landed.  Closing bug.

[Eric Seidel (no email)]

This broke tests.

======================================================================
ERROR: test_files_in_namelist (webkitpy.common.system.directoryfileset_unittest.DirectoryFileSetTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/Tools/Scripts/webkitpy/common/system/directoryfileset_unittest.py", line 42, in test_files_in_namelist
    self.assertTrue('some-file' in self._fileset.namelist())
  File "/home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/Tools/Scripts/webkitpy/common/system/directoryfileset.py", line 61, in namelist
    return map(self._drop_directory_prefix, self._files_in_directory())
  File "/home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/Tools/Scripts/webkitpy/common/system/directoryfileset.py", line 52, in _files_in_directory
    return self._filesystem.files_under(self._path)
  File "/home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/Tools/Scripts/webkitpy/common/system/filesystem_mock.py", line 85, in files_under
    if self.basename(path) in dirs_to_skip:
TypeError: basename() takes exactly 1 argument (2 given)

----------------------------------------------------------------------

[Dirk Pranke]

(In reply to comment #9)
> This broke tests.
> 
> ======================================================================
> ERROR: test_files_in_namelist (webkitpy.common.system.directoryfileset_unittest.DirectoryFileSetTest)
> ----------------------------------------------------------------------
> Traceback (most recent call last):
>   File "/home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/Tools/Scripts/webkitpy/common/system/directoryfileset_unittest.py", line 42, in test_files_in_namelist
>     self.assertTrue('some-file' in self._fileset.namelist())
>   File "/home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/Tools/Scripts/webkitpy/common/system/directoryfileset.py", line 61, in namelist
>     return map(self._drop_directory_prefix, self._files_in_directory())
>   File "/home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/Tools/Scripts/webkitpy/common/system/directoryfileset.py", line 52, in _files_in_directory
>     return self._filesystem.files_under(self._path)
>   File "/home/webkitbuildbot/slaves/release32bit/buildslave/qt-linux-release/build/Tools/Scripts/webkitpy/common/system/filesystem_mock.py", line 85, in files_under
>     if self.basename(path) in dirs_to_skip:
> TypeError: basename() takes exactly 1 argument (2 given)
> 
> ----------------------------------------------------------------------

Fixed in r76158.

[WebKit Review Bot]

http://trac.webkit.org/changeset/76155 might have broken GTK Linux 32-bit Release
The following tests are not passing:
fast/forms/input-text-scroll-left-on-blur.html
fast/forms/plaintext-mode-2.html