Bug 235370

Summary:

Move tests to pyfakefs from FileSystemMock

Product:

WebKit

Reporter:

Sam Sneddon [:gsnedders] <gsnedders>

Component:

Tools / Tests

Assignee:

Sam Sneddon [:gsnedders] <gsnedders>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

ap, ews-watchlist, glenn, jbedard, webkit-bug-importer

Priority:

Keywords:

InRadar

Version:

WebKit Nightly Build

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://bugs.webkit.org/show_bug.cgi?id=236043

Bug Depends on:

Bug Blocks:

220421

Attachments:

Description	Flags
Patch	none

Description Sam Sneddon [:gsnedders] 2022-01-19 11:18:54 PST

see https://github.com/jmcgeheeiv/pyfakefs

This has a bunch of advantages over our current FileSystemMock:

Avoids us having to maintain our own re-implementation (which currently has a variety of issues)

Can configure it to emulate Windows/Linux/macOS behaviour on other systems, allowing us to test things still work on Windows more easily

Provides a path away from our current FileSystem class towards using the standard library functions directly

Comment 1 Radar WebKit Bug Importer 2022-01-19 12:24:04 PST

<rdar://problem/87785236>

Comment 2 Sam Sneddon [:gsnedders] 2022-01-28 13:19:04 PST

Created attachment 450272 [details]
Patch

Comment 3 Jonathan Bedard 2022-02-01 08:04:29 PST

Comment on attachment 450272 [details]
Patch

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

> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_finder_legacy_unittest.py:57
> +        self.setUpPyfakefs()

I agree that using pyfakefs through the TestCaseMixin is the right thing to do for this class. I'm curious if you looked at the Patcher and considered it at all? Or was the Mixin just the natural choice because this particular set of tests was already using setup.

Comment 4 Sam Sneddon [:gsnedders] 2022-02-01 08:32:02 PST

Comment on attachment 450272 [details]
Patch

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

>> Tools/Scripts/webkitpy/layout_tests/controllers/layout_test_finder_legacy_unittest.py:57
>> +        self.setUpPyfakefs()
> 
> I agree that using pyfakefs through the TestCaseMixin is the right thing to do for this class. I'm curious if you looked at the Patcher and considered it at all? Or was the Mixin just the natural choice because this particular set of tests was already using setup.

Using Patcher requires reimplementing a lot of the TestCaseMixin machinery for very little gain.

Comment 5 EWS 2022-02-01 08:36:48 PST

Committed r288878 (246630@main): <https://commits.webkit.org/246630@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 450272 [details].