Formalizing physical test suites for web tests? Alternatives?

63 views
Skip to first unread message

Xianzhu Wang

unread,
Oct 17, 2019, 1:57:25 PM10/17/19
to blink-dev
Hi,

We have been supporting physical test suites for a long time, but it's not well known perhaps because for now the configuration is hard-coded in python source code.

Like a virtual test suite, a physical test suite is a subset of web tests that runs with specific command line flags. The difference is that physical tests run with their original path names, without "virtual/prefix".

I think a physical test suite has the following advantages:
  1. It costs less. It still requires dedicated content_shell worker process like virtual test suites, but it runs only once. *
  2. It avoids configuration at multiple places (e.g. VirtualTestSuites to define the virtual test suite, and TestExpectations/NeverFixTests to skip the base test suite) when the tests are only meaningful with specific flags. *
  3. When we are about to launch a feature (after we have tested the feature with virtual test suites and/or --additional-driver-flag), we can use physical test suites to enable the feature in smaller batches (than enabling the feature for all tests at once).
  4. The filenames of baselines (if any) are shorter, which can prevent too-long filenames for baselines under virtual test directories.
  5. We can have a central place for fixed additional flags which are currently implemented at different places not easy to find, e.g. for --enable-experimental-web-platform-features and mock scrollbars for compositing tests.
* Actually advantages 1 and 2 can be achieved with virtual test cases by putting test cases under the virtual test directory. This method could be an alternative to physical test suites, but it doesn't have the other advantages. It also needs to be better documented.

Physical test suites apparently have drawbacks:
  1. Developers need to know another type of test configuration.
  2. It's not obvious to developers that a test runs with specific flags without looking at the physical test suite configuration.
I'm thinking of formalizing it with a config file web_tests/PhysicalTestSuites (CL) to make it easier to use, and convert some virtual test suites into physical test suites to simplify test configurations. Do you think this is good or bad for blink?

Thanks,
Xianzhu

Chris Harrelson

unread,
Oct 17, 2019, 2:57:09 PM10/17/19
to Xianzhu Wang, blink-dev
I would lean towards deleting physical test suites as a concept, to reduce the number of concepts developers have to know about. The two examples of physical test suites right now appear straightforward to make virtual.

I also don't like that certain directories end up magically with special flags.

--
You received this message because you are subscribed to the Google Groups "blink-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to blink-dev+...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CADBxrifP8ESMZSR1CfQM-beEVyQfcHmfO8guvvSW9%3DxfCA1c_w%40mail.gmail.com.

Philip Rogers

unread,
Oct 17, 2019, 2:57:42 PM10/17/19
to Xianzhu Wang, blink-dev
This is a great idea! I like reducing the number of places we apply directory-specific settings. It's mega confusing to not be able to reproduce a test failure when loading the test directly.

What if we unified on using just VirtualTestSuites, removing PhysicalTestSuites too? This would mean developers only need to understand one directory-specific configuration system instead of two, and it's listed in the test's name. I think the issue of multiple configuration places (advantage 2) is important because of the complexity in how these configurations interact. For example, whether PhysicalTestSuite flags apply to VirtualTestSuites is not obvious. There is a similar issue with how flag-specific expectations interact. PhysicalTestSuites and mock scrollbars are actually only used for a small number of tests which could either be rebaselined or switched to virtual test suites.

On Thu, Oct 17, 2019 at 10:57 AM Xianzhu Wang <wangx...@chromium.org> wrote:

Xianzhu Wang

unread,
Oct 17, 2019, 6:16:36 PM10/17/19
to Philip Rogers, blink-dev
VirtualTestSuites has a limitation: "pure virtual" test suite doesn't work for WPT tests. "Pure virtual" means that we put test cases under the virtual test directory, which is a trick to let the test run only under the virtual suite. Because of this limitation, when some WPT tests require specific runtime flags (e.g. they use an experimental feature), we need to create a virtual test suite, and skip the base tests in NeverFixTests or TestExpectations [1]. Another way is to modify bot scripts to add the flags, but this doesn't support local runs.

I just learned that we are moving WPT tests away from run_web_tests.py. Will the new design support additional flags?

[1] this might be the way to go if we decide to ditch physical test suites.

Stephen McGruer

unread,
Oct 17, 2019, 6:53:48 PM10/17/19
to Xianzhu Wang, Philip Rogers, blink-dev, ecosyst...@chromium.org
+ecosystem-infra (specifically lpz@ who will likely have the most context)

Xianzhu Wang

unread,
Oct 21, 2019, 2:06:57 PM10/21/19
to Stephen McGruer, Luke Z, Philip Rogers, blink-dev, ecosyst...@chromium.org
(lpz@ I have a question for you at the bottom of this email.)

Thanks everyone for the feedback.

Now I think we probably should not have another type of test suite, but support some features of physical test suite under the current virtual test suite framework.

My proposals are:

1. Support virtual test suite without a "base" for tests that need special flags, like the following in VirtualTestSuites (e.g. for tests that require overlay scrollbars):
   {
     "prefix": "overlay-scrollbars",
     "args": ["--enable-features=OverlayScrollbar"],
   }

  This basically an alternative of physical test suite that requires the "virtual" prefix to explicitly indicate that the tests run with the special args, instead of magically applying the args.

   Alternatively, we can use a special "base" (which requires no modification to the current blinkpy code), like
   {
     "prefix": "overlay-scrollbars",
     "base": "tests",
     "args": ["--enable-features=OverlayScrollbar"],
   }

2. Move base tests that run also under a virtual test suite but are only meaningful under the virtual test suite, into the virtual test directory. This avoids multi-place test configurations, and shortens virtual test and baseline path names, to reduce the chance of too-long filenames on Windows.
    For example,
  • Move all tests that are valid only under virtual/not-site-per-process into the virtual directory to simply test configurations;
  • Move paint/dark-mode to virtual/dark-mode-*, so that virtual/dark-mode/paint/dark-mode/grayscale-images/ will become virtual/dark-mode-grascale-images/ which has much shorter path names, and avoid running dark-mode tests under non-dark-mode configuration.
However, virtual test suites seem still awkward for WPT tests that require special flags. lpz@, is there a plan to address the issue in the new run_wpt_tests.py design?

Thanks,
Xianzhu

Philip Rogers

unread,
Oct 21, 2019, 6:17:20 PM10/21/19
to Xianzhu Wang, Stephen McGruer, Luke Z, blink-dev, ecosyst...@chromium.org
I think this proposal is a reasonable way forward and will simplify our testing configurations.

For #1, if this is only needed for overlay scrollbars, virtual/overlay-scrollbars/compositing might be the simplest approach? #2 sounds like a nice cleanup overall.

Xianzhu Wang

unread,
Oct 21, 2019, 6:27:00 PM10/21/19
to Philip Rogers, Stephen McGruer, Luke Z, blink-dev, ecosyst...@chromium.org
On Mon, Oct 21, 2019 at 3:17 PM Philip Rogers <p...@chromium.org> wrote:
I think this proposal is a reasonable way forward and will simplify our testing configurations.

For #1, if this is only needed for overlay scrollbars, virtual/overlay-scrollbars/compositing might be the simplest approach? #2 sounds like a nice cleanup overall.

Overlay scrollbar is just an example of physical tests under a virtual test suite. The two choices are 1) omitting "base" which can save a level of directory, and 2) using "tests" as a common container directory for all physical tests. An additional level of directory "test" seems not a problem, and can be a good pattern for physical tests. Without it, we will need to list every directory under virtual/xxx in VirtualTestSuites.

Dirk Pranke

unread,
Oct 21, 2019, 6:33:55 PM10/21/19
to Xianzhu Wang, Philip Rogers, Stephen McGruer, Luke Z, blink-dev, ecosystem-infra
I don't actually understand the proposals fully, possibly because (unless I'm missing something) "overlay-scrollbars" isn't currently either a physical or virtual suite.

Does it make sense to rephrase it in terms of something that does exist, like hdr/ ?

I don't understand how (1) would work, in the sense of how you can omit a "base"; base tells you which directories need the flags, right?

Unless you're suggesting that we should support a mode where we re-run *all* of the tests with a flag? If you're looking to do that, we should simply run a different test step, like we did for site-per-process. But, running all of the tests is an extremely expensive thing so I hope we're not planning to do that normally.

-- Dirk

Xianzhu Wang

unread,
Oct 21, 2019, 7:12:02 PM10/21/19
to Dirk Pranke, Philip Rogers, Stephen McGruer, Luke Z, blink-dev, ecosystem-infra
The proposal is to use the current feature of virtual test suite that allows physical tests to be under virtual test directory, and the tests will run with the specified args only.

Using "hdr" as an example, there would be an entry in VirtualTestSuite like:
Choice 1 (which needs a change of run_web_tests.py script, to let a virtual test suite without a "base" not match any physical directory):
  {
     prefix: "hdr",
     args: "--force-color-profile=scrgb-linear"]
  }
and all current tests under hdr/ should be moved into virtual/hdr/ directory.

Choice 2 (which doesn't need any change of scripts):
  {
     prefix: "hdr",
     base: "tests", // Which matches no base physical directory.
     args: "--force-color-profile=scrgb-linear"]
  }
and all current hdr/ tests should be moved into virtual/hdr/tests/ directory.

Actually the above is equivalent to the following PhysicalTestSuite entry in base.py:
  PhysicalTestSuite('virtual/hdr/tests/', ['--force-color-profile=scrgb-linear'])

My overlay scrollbar example was based on a hypothetical PhysicalTestSuite entry:
  PhysicalTestSuite('overlay-scrollbar/', ['--enable-feature=OverlayScrollbar'])

Dirk Pranke

unread,
Oct 21, 2019, 7:49:03 PM10/21/19
to Xianzhu Wang, Philip Rogers, Stephen McGruer, Luke Z, blink-dev, ecosystem-infra
Got it, thanks.

Given that, I'd actually be more in favor of #1 than #2, the fake "tests" directory feels unnecessary. 

But, it does seem like either approach is problematic for replacing "physical" test suites for WPT, but we don't have any of those today, right? I don't really see an issue with virtual test suites for WPT; it seems like we could make them work the same way non-WPT virtual suites work.

-- Dirk

Xianzhu Wang

unread,
Oct 21, 2019, 8:30:44 PM10/21/19
to Dirk Pranke, Philip Rogers, Stephen McGruer, Luke Z, blink-dev, ecosystem-infra
On Mon, Oct 21, 2019 at 4:48 PM Dirk Pranke <dpr...@chromium.org> wrote:
Got it, thanks.

Given that, I'd actually be more in favor of #1 than #2, the fake "tests" directory feels unnecessary. 
 
But, it does seem like either approach is problematic for replacing "physical" test suites for WPT, but we don't have any of those today, right? I don't really see an issue with virtual test suites for WPT; it seems like we could make them work the same way non-WPT virtual suites work.

Yes, we are only using virtual-suite-and-skipping-base method for the WPT tests that only work with specific flags. I hope the new run_wpt_tests.py will provide a better support of the feature.

Xianzhu Wang

unread,
Oct 31, 2019, 10:57:58 PM10/31/19
to Dirk Pranke, Philip Rogers, Stephen McGruer, Luke Z, blink-dev, ecosystem-infra
Updates:

PhysicalTestSuites are removed and replaced with virtual test suites with improved support of real tests under virtual test directories.

VirtualTestSuites config file is now in the following format:
[
  ...
   {
    "prefix": "threaded-prefer-compositing",
    "bases": ["fast/scroll-behavior",
              "fast/scrolling"],
    "args": ["--enable-threaded-compositing",
             "--enable-prefer-compositing-to-lcd-text"]
  },
  ...
]

"bases" can be empty to mimic a physical test suite. For example:
   {
    "prefix": "text-antialias",
    "bases": [],
    "args": ["--enable-font-antialiasing"]
   },
replaced the old PhysicalTestSuite:
   PhysicalTestSuite('fast/text', ['--enable-font-antialiasing']),
with fast/text directory renamed to virtual/text-antialias.

An unfortunate confusing outcome is the fake nested virtual test suite like:
  {
    "prefix": "controls-refresh-hc",
    "bases": ["virtual/controls-refresh/color-scheme"],
    "args": ["--enable-features=FormControlsRefresh",
             "--force-high-contrast",
             "--enable-blink-features=ForcedColors"]
  },
This doesn't mean the virtual suite virtual/controls-refresh-hc inherits virtual/controls-refresh, but means that we'll run the real tests under virtual/controls-refresh/color-scheme directory with the specified args, for virtual test names like virtual/controls-refresh-hc/virtual/controls-refresh/color-scheme/... .

Dirk Pranke

unread,
Nov 6, 2019, 9:17:46 PM11/6/19
to Xianzhu Wang, Philip Rogers, Stephen McGruer, Luke Z, blink-dev, ecosystem-infra
Hm. I wonder if we should rename "virtual" to "flag-specific" or something?

Xianzhu Wang

unread,
Nov 6, 2019, 9:57:10 PM11/6/19
to Dirk Pranke, Philip Rogers, Stephen McGruer, Luke Z, blink-dev, ecosystem-infra
I think flag-specific is a good name, but we may need to integrate the virtual test suite feature and the flag-specific feature with their different use cases still well supported. The latter is useful to test work-in-process features with all web tests.

Dirk Pranke

unread,
Nov 7, 2019, 4:10:39 PM11/7/19
to Xianzhu Wang, Philip Rogers, Stephen McGruer, Luke Z, blink-dev, ecosystem-infra
True, I didn't mean to suggest we do anything with the flag-specific stuff, so that would be confusing. Maybe 'variant' or 'variants' instead, or something ...

-- Dirk
Reply all
Reply to author
Forward
0 new messages