webkit_layout_tests with --additional-driver-flag=--enable-features=NetworkService doesn't quite work |
|||||
Issue descriptionTo test the NetworkService we do: blink/tools/run_web_tests.py --additional-driver-flag=--enable-features=NetworkService This doesn't quite work, as the test driver may also append command line flags when running particular tests. For instance: ./tools/run_web_tests.py --additional-driver-flag=--enable-features=NetworkService virtual/reporting-api/external/wpt/content-security-policy/reporting-api/reporting-api-works-on-frame-src.https.sub.html Will end up invoking: content_shell --enable-features=NetworkService --run-web-tests ... --enable-features=Reporting The problem here is we have two --enable-features rather than a single one coalesced with commas. That means at runtime base::FeatureList::IsEnabled() will only return true for Reporting and not NetworkService. Any web_test which species custom --enable-features is impacted by this. For instance search for --enable-features in this file: https://cs.chromium.org/chromium/src/third_party/blink/web_tests/VirtualTestSuites
,
Nov 27
+Dirk for his thoughts
,
Nov 27
+nednguyen, +robertma, who work on this code these days. Personally, I'm not a fan of how base::CommandLine works here, but I'm also not gonna spend any energy arguing that we should change it :). I think it's fine to fix this in run_web_tests.py one way or another. eroman@'s suggestion in #c1 is probably the right way to do it. I've thought of a couple of other approaches off the top of my head (like adding a first-class --enable-features flag to run_web_tests.py), but I don't think the ones that I've thought of will really interact all that well with the other mechanisms like the VirtualTestSuites.
,
Nov 27
Thanks Dirk! I did a POC for the run_web_tests.py version: https://chromium-review.googlesource.com/c/chromium/src/+/1351906 Appears to work for my use-case at least, but may need to be refactored by someone who knows python :)
,
Nov 27
,
Nov 29
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/80939788be512dba94c38f88a8a3c80377789a35 commit 80939788be512dba94c38f88a8a3c80377789a35 Author: Eric Roman <eroman@chromium.org> Date: Thu Nov 29 03:04:19 2018 Coalesce --enable-features and --disable-features in run_web_tests.py Bug: 908654 Change-Id: I184bbfd27ff1116bccb1e5cb9a9ef1e19cacd26d Reviewed-on: https://chromium-review.googlesource.com/c/1351906 Commit-Queue: Eric Roman <eroman@chromium.org> Reviewed-by: Dirk Pranke <dpranke@chromium.org> Cr-Commit-Position: refs/heads/master@{#612007} [modify] https://crrev.com/80939788be512dba94c38f88a8a3c80377789a35/third_party/blink/tools/blinkpy/web_tests/port/driver.py [modify] https://crrev.com/80939788be512dba94c38f88a8a3c80377789a35/third_party/blink/tools/blinkpy/web_tests/port/driver_unittest.py
,
Nov 29
Was this expected to make some virtual tests fail with network service? See issue https://crbug.com/910011
,
Nov 29
Yes, those failures are expected (those tests weren't previously running under NetworkService) |
|||||
►
Sign in to add a comment |
|||||
Comment 1 by eroman@chromium.org
, Nov 27