Bug 264750 – WebKitFinder and TestPort disagree about layout_tests_dir

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Sam Sneddon [:gsnedders]]

import unittest

from webkitpy.common.host_mock import MockHost
from webkitpy.common.webkit_finder import WebKitFinder
from webkitpy.port.test import TestPort


class TestPortTest(unittest.TestCase):

    def test_webkit_finder_base(self):
        host = MockHost()
        port = TestPort(host)
        fs = host.filesystem

        finder = WebKitFinder(fs)
        self.assertEqual(finder.layout_tests_dir(), port.layout_tests_dir())

E       AssertionError: '/mock-checkout/LayoutTests' != '/test.checkout/LayoutTests'
E       - /mock-checkout/LayoutTests
E       ?  ^^^^^
E       + /test.checkout/LayoutTests
E       ?  ^^^^^

[Radar WebKit Bug Importer]

<rdar://problem/118339735>

[Sam Sneddon [:gsnedders]]

Note if you replace port with host.port_factory.get() then this does work; so it's specifically about the interaction between TestPort (which tries to pretend it has its own directory paths) and the MockFileSystem.

[Sam Sneddon [:gsnedders]]

Excluding tests:

```
% rg '(mock-checkout)' Tools --glob '!*_unittest.py*' --glob '!*_integrationtest.py' --sort=path --no-heading
Tools/Scripts/webkitpy/common/checkout/scm/scm_mock.py:37:        self.checkout_root = "/mock-checkout"
Tools/Scripts/webkitpy/common/system/filesystem_mock.py:97:        return "/mock-checkout/Tools/Scripts/" + module_name.replace('.', '/') + ".py"
Tools/Scripts/webkitpy/port/darwin_testcase.py:60:            "MOCK popen: ['Tools/Scripts/run-safari', '--release', '--no-saved-state', '-NSOpen', 'test.html'], cwd=/mock-checkout\n",
Tools/Scripts/webkitpy/port/ios_testcase.py:41:        self.assertEqual(search_path[-1], '/mock-checkout/LayoutTests/platform/ios')
Tools/Scripts/webkitpy/port/ios_testcase.py:42:        self.assertEqual(search_path[-2], '/mock-checkout/LayoutTests/platform/ios-wk1')
Tools/Scripts/webkitpy/port/port_testcase.py:536:        self.assertEqual(port.path_to_test_expectations_file(), '/mock-checkout/LayoutTests/platform/testwebkitport/TestExpectations')
Tools/Scripts/webkitpy/port/port_testcase.py:540:        self.assertEqual(port.path_to_test_expectations_file(), '/mock-checkout/LayoutTests/platform/testwebkitport/TestExpectations')
Tools/Scripts/webkitpy/port/port_testcase.py:543:        port.host.filesystem.files['/mock-checkout/LayoutTests/platform/testwebkitport/TestExpectations'] = 'some content'
Tools/Scripts/webkitpy/port/port_testcase.py:545:        self.assertEqual(port.path_to_test_expectations_file(), '/mock-checkout/LayoutTests/platform/testwebkitport/TestExpectations')
Tools/Scripts/webkitpy/port/port_testcase.py:579:        host.filesystem.write_text_file('/mock-checkout/LayoutTests/platform/testwebkitport/TestExpectations',
Tools/Scripts/webkitpy/port/port_testcase.py:593:            "MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'MOCK_ENVIRON_COPY': '1'}\n",
Tools/Scripts/webkitpy/port/port_testcase.py:602:            '''MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'MOCK_ENVIRON_COPY': '1'}
Tools/Scripts/webkitpy/port/port_testcase.py:603:MOCK run_command: ['Tools/Scripts/build-webkittestrunner', '--release'], cwd=/mock-checkout, env={'MOCK_ENVIRON_COPY': '1'}
Tools/Scripts/webkitpy/port/port_testcase.py:613:            '''MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'MOCK_ENVIRON_COPY': '1'}
Tools/Scripts/webkitpy/port/port_testcase.py:626:            '''MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release'], cwd=/mock-checkout, env={'MOCK_ENVIRON_COPY': '1'}
Tools/Scripts/webkitpy/port/port_testcase.py:689:        self.assertEqual(port._path_to_apache_config_file(), '/mock-checkout/LayoutTests/http/conf/httpd.conf')
Tools/Scripts/webkitpy/port/test.py:298:TEST_DIR = '/mock-checkout'
Tools/Scripts/webkitpy/port/watch_testcase.py:47:        self.assertEqual(search_path[-1], '/mock-checkout/LayoutTests/platform/wk2')
Tools/Scripts/webkitpy/port/watch_testcase.py:48:        self.assertEqual(search_path[-2], '/mock-checkout/LayoutTests/platform/watchos')
Tools/Scripts/webkitpy/tool/commands/queuestest.py:74:        checkout_dir = '/mock-checkout'

% rg '(test\.checkout)' Tools --glob '!*_unittest.py*' --glob '!*_integrationtest.py' --sort=path --no-heading
Tools/Scripts/webkitpy/port/test.py:296:LAYOUT_TEST_DIR = '/test.checkout/LayoutTests'
Tools/Scripts/webkitpy/port/test.py:297:PERF_TEST_DIR = '/test.checkout/PerformanceTests'
Tools/Scripts/webkitpy/port/test.py:477:        return '/test.checkout'
Tools/Scripts/webkitpy/port/test.py:653:        test_world_leaks_output = """TEST: file:///test.checkout/LayoutTests/failures/expected/leak.html
Tools/Scripts/webkitpy/port/test.py:654:ABANDONED DOCUMENT: file:///test.checkout/LayoutTests/failures/expected/leak.html
Tools/Scripts/webkitpy/port/test.py:655:TEST: file:///test.checkout/LayoutTests/failures/unexpected/leak.html
Tools/Scripts/webkitpy/port/test.py:656:ABANDONED DOCUMENT: file:///test.checkout/LayoutTests/failures/expected/flaky-leak.html
Tools/Scripts/webkitpy/port/test.py:657:TEST: file:///test.checkout/LayoutTests/failures/unexpected/flaky-leak.html
Tools/Scripts/webkitpy/port/test.py:658:ABANDONED DOCUMENT: file:///test.checkout/LayoutTests/failures/expected/leak.html
Tools/Scripts/webkitpy/port/test.py:659:TEST: file:///test.checkout/LayoutTests/failures/unexpected/leak.html
Tools/Scripts/webkitpy/port/test.py:660:ABANDONED DOCUMENT: file:///test.checkout/LayoutTests/failures/expected/leak-subframe.html
Tools/Scripts/webkitpy/port/test.py:661:TEST: file:///test.checkout/LayoutTests/failures/expected/leaky-reftest.html
Tools/Scripts/webkitpy/port/test.py:662:ABANDONED DOCUMENT: file:///test.checkout/LayoutTests/failures/expected/leaky-reftest.html"""
Tools/Scripts/webkitpy/test/main.py:320:        "webkitscmpy.test.checkout_unittest",
```

[Sam Sneddon [:gsnedders]]

We have a comment in test.py:

```
# Here we use a non-standard location for the layout tests, to ensure that
# this works. The path contains a '.' in the name because we've seen bugs
# related to this before.

LAYOUT_TEST_DIR = '/test.checkout/LayoutTests'
PERF_TEST_DIR = '/test.checkout/PerformanceTests'
TEST_DIR = '/mock-checkout'
```

Which is an assumption that webkit_finder.py violates.

Ironically, the only place that uses WebKitFinder.layout_tests_dir is:

port/base.py
693:        return self._webkit_finder.layout_tests_dir()

[Sam Sneddon [:gsnedders]]

That said, port.base has a comment saying:

>     # FIXME: update callers to create a finder and call it instead of these next five routines (which should be protected).

So maybe it's simply not intended to allow ports to override the layout test directory any more?

That said, AFAICT no port ever in the tree ever did override it (I'd thought Chromium did, but it seemingly didn't), so maybe it's just needlessly generic?

If that's the case, then we can fix the difference by just dropping support for other paths.

[Sam Sneddon [:gsnedders]]

Okay, so while no _port_ overrides it, `port.base` does handle `--layout-tests-directory`, which _is_ used.

So we probably should push everything to go via the port, because otherwise we risk accidentally using different paths.

[Sam Sneddon [:gsnedders]]

Pull request: https://github.com/WebKit/WebKit/pull/20551

[EWS]

Committed 270880@main (cb302cd1f44f): <https://commits.webkit.org/270880@main>

Reviewed commits have been landed. Closing PR #20551 and removing active labels.