New issue
Advanced search Search tips

Issue 796975 link

Starred by 1 user
Status: Fixed
Owner:
jbroman@chromium.org
Closed: Dec 2017
Cc:
dcheng@chromium.org
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 2
Type: Task

Blocking:
issue 796973



Sign in to add a comment

third_party/WebKit/Source/controller/tests/RunAllTests.cpp shouldn't include content/test

Project Member Reported by jam@chromium.org, Dec 21 2017 
Chrome Version       : 64.0.3282.39
OS Version: 6.1 (Windows 7, Windows Server 2008 R2)
URLs (if applicable) :
Other browsers tested:
  Add OK or FAIL after other browsers where you have tested this issue:
     Safari:
    Firefox:
    IE/Edge:

What steps will reproduce the problem?
1.
2.
3.

What is the expected result?


What happens instead of that?


Please provide any additional information below. Attach a screenshot if
possible.

UserAgentString: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.39 Safari/537.36

Comment 1 by jbroman@chromium.org, Dec 21 2017 

Doing this is the point of content/test/blink_test_environment.*. This is a fairly narrow dependency that has existed for as long as I can remember.

There are certainly contrived ways of eliminating it (like making the target in third_party/WebKit/ expose the rest of runHelper, and moving the "main" method and the webkit_unit_tests target into content/test/), but that doesn't really seem meaningfully better than just allowing this dependency.

Or is moving that file to content/public/test/ sufficient (i.e. the dependency is okay, it's just the path that's objectionable)?

Comment 2 by jam@chromium.org, Dec 22 2017 

sorry I just saw this comment. SG
Project Member

Comment 3 by bugdroid1@chromium.org, Dec 22 2017 

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/409a9cc31dd21844df0cfed7e336263d2bf44c8e

commit 409a9cc31dd21844df0cfed7e336263d2bf44c8e
Author: Jeremy Roman <jbroman@chromium.org>
Date: Fri Dec 22 18:36:13 2017

content: Move blink_test_environment.* to content/public/test/.

Bug:  796975 
Change-Id: I61fc830dd8a6fdba1ed0eacd1d33416553b57e45
Reviewed-on: https://chromium-review.googlesource.com/842844
Reviewed-by: John Abd-El-Malek <jam@chromium.org>
Commit-Queue: Jeremy Roman <jbroman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#526025}
[rename] https://crrev.com/409a9cc31dd21844df0cfed7e336263d2bf44c8e/content/public/test/blink_test_environment.cc
[rename] https://crrev.com/409a9cc31dd21844df0cfed7e336263d2bf44c8e/content/public/test/blink_test_environment.h
[modify] https://crrev.com/409a9cc31dd21844df0cfed7e336263d2bf44c8e/content/test/BUILD.gn
[modify] https://crrev.com/409a9cc31dd21844df0cfed7e336263d2bf44c8e/third_party/WebKit/Source/controller/tests/DEPS
[modify] https://crrev.com/409a9cc31dd21844df0cfed7e336263d2bf44c8e/third_party/WebKit/Source/controller/tests/RunAllTests.cpp
[modify] https://crrev.com/409a9cc31dd21844df0cfed7e336263d2bf44c8e/third_party/WebKit/Source/platform/heap/DEPS
[modify] https://crrev.com/409a9cc31dd21844df0cfed7e336263d2bf44c8e/third_party/WebKit/Source/platform/heap/RunAllTests.cpp
[modify] https://crrev.com/409a9cc31dd21844df0cfed7e336263d2bf44c8e/third_party/WebKit/Source/platform/testing/BlinkFuzzerTestSupport.cpp
[modify] https://crrev.com/409a9cc31dd21844df0cfed7e336263d2bf44c8e/third_party/WebKit/Source/platform/testing/DEPS

Comment 4 by jbroman@chromium.org, Dec 22 2017

Status: Fixed (was: Assigned)
►

Sign in to add a comment