New Git Workflow: chrome/test/data/layout_tests are (mostly) empty

[Dmitry Lomov]

Hi folks,

it looks like the new git workflow does not populate chrome/test/data/layout_tests, hence chromium ui_tests do not work in the new git workflow. 

Intentional? Unintentional? What is the right way to fix?

Thanks,

Dmitry

[Elliot Glaysher (Chromium)]

While there are plans to prune the new git repository of layout tests,
old .exe files, and other cruft, I don't believe this has happened
yet. I just performed a new git checkout and received 1.21 GiB of
objects while the proposed pruned repository is a little over 300
megabytes. (Also, chrome/test/data/layout_tests/ wasn't on the list of
directories to prune from the git repository.)

-- Elliot

> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
> http://groups.google.com/a/chromium.org/group/chromium-dev
>

[Sreeram Ramachandran]

I didn't have the layout_tests either, so I just "fixed" it by doing:
$ cd chrome/test/data/layout_tests
$ ln -s ../../../../third_party/WebKit/LayoutTests

[Nicolas Sylvain]

On Tue, Oct 18, 2011 at 6:56 PM, Dmitry Lomov <dsl...@google.com> wrote:

We should make the test look in the real layout tests directory first. It is really bad to have to duplicate all those

layout tests data.  This duplication is not supported on the new git workflow (and hopefully we won't have too).

sreeram's suggestion should work until there is a real fix.

Nicolas

[Dmitry Lomov]

On Wed, Oct 19, 2011 at 1:02 PM, Nicolas Sylvain <nsyl...@chromium.org> wrote:

On Tue, Oct 18, 2011 at 6:56 PM, Dmitry Lomov <dsl...@google.com> wrote:

Hi folks,

it looks like the new git workflow does not populate chrome/test/data/layout_tests, hence chromium ui_tests do not work in the new git workflow. 

Intentional? Unintentional? What is the right way to fix?

We should make the test look in the real layout tests directory first. It is really bad to have to duplicate all those

layout tests data.  This duplication is not supported on the new git workflow (and hopefully we won't have too).

Is there a bug? 

[Dmitry Lomov]

Opened http://code.google.com/p/chromium/issues/detail?id=100937.

[Sheridan Rawlins]

crrev.com/106424 and crrev.com/107031 recently left me unable to build
as it appears as though .DEPS.git doesn't honor either subpath include
or src/lib_json...

DEPS:
"src/third_party/jsoncpp/source/include":
(Var("sourceforge_url") % {"repo": "jsoncpp"}) +
"/trunk/jsoncpp/include@" + Var("jsoncpp_revision"),

"src/third_party/jsoncpp/source/src/lib_json":
(Var("sourceforge_url") % {"repo": "jsoncpp"}) +
"/trunk/jsoncpp/src/lib_json@" + Var("jsoncpp_revision"),

.DEPS.git
'src/third_party/jsoncpp/source/include':
Var('git_url') + '/external/jsoncpp/
jsoncpp.git@586229b92307944dd750801893c8c7c624547c8a',
'src/third_party/jsoncpp/source/src/lib_json':
Var('git_url') + '/external/jsoncpp/
jsoncpp.git@586229b92307944dd750801893c8c7c624547c8a',

I believe that rsleevi will revert crrev.com/107031 and we'll need to
fix this as well before moving it forward again.

On Oct 19, 2:14 pm, Dmitry Lomov <dsl...@chromium.org> wrote:
> Openedhttp://code.google.com/p/chromium/issues/detail?id=100937.

[Nicolas Sylvain]

On Mon, Oct 24, 2011 at 10:03 PM, Sheridan Rawlins <s...@chromium.org> wrote:

crrev.com/106424 and crrev.com/107031 recently left me unable to build
as it appears as though .DEPS.git doesn't honor either subpath include
or src/lib_json...

This should be fixed now.

Nicolas

[Takashi Toyoshima]

Greetings,

I'll land another change related this topic.
http://codereview.chromium.org/8641002/

Dmitry fixed ui_layout_test. But other disabled tests also need the same hack.
I added chrome::DIR_LAYOUT_TESTS key for PathService::Get() then use it
from each tests to choose suitable directory.

By the way, new git workflow will not allow duplicate layout tests in
chrome/test/data/layout_tests. But, to my understanding, our test bots
never have third_party/WebKit/LayoutTests because its size is too huge.
Must we dispose chrome/test/data/layout_tests in spite of that?
Or bots will continue to use svn, then it could not be a problem?

I feel it's confusing that the source tree structure is different
between git and svn.

For now, I'll land this change to enable some disabled tests.
But, still we need some discussion on this topic?

--
Takashi Toyoshima
Software Engineer, Google

[Nicolas Sylvain]

On Tue, Nov 22, 2011 at 10:48 AM, Takashi Toyoshima <toyo...@google.com> wrote:

Greetings,

I'll land another change related this topic.
http://codereview.chromium.org/8641002/

Dmitry fixed ui_layout_test. But other disabled tests also need the same hack.
I added chrome::DIR_LAYOUT_TESTS key for PathService::Get() then use it
from each tests to choose suitable directory.

By the way, new git workflow will not allow duplicate layout tests in
chrome/test/data/layout_tests. But, to my understanding, our test bots
never have third_party/WebKit/LayoutTests because its size is too huge.
Must we dispose chrome/test/data/layout_tests in spite of that?
Or bots will continue to use svn, then it could not be a problem?

I feel it's confusing that the source tree structure is different
between git and svn.

The only reason why we had  "chrome/test/data/layout_tests" is because we did not want

to fetch "third_party/WebKit/LayoutTests" on the bots because it took too long. This is not

a problem with Git. (And GIT in general does not work well with fetching sub-repos like that).

We should fix all places in the code where we use "chrome/test/data/layout_tests" and fallback on 

"third_party/WebKit/LayoutTests" if it's not there.

Thanks

Nicolas

[Takashi Toyoshima]

Thanks Nicolas,

So, my change looks good except for the fallback order.
I'll make another change to swap path search order.

[Nicolas Sylvain]

On Tue, Nov 22, 2011 at 4:41 PM, Takashi Toyoshima <toyo...@google.com> wrote:

Thanks Nicolas,

So, my change looks good except for the fallback order.
I'll make another change to swap path search order.

Thanks for making the change.

The fallback order does not really matter. Either way should work just as fine.

Thanks

Nicolas 

[Takashi Toyoshima]

OK, I keep current order.

Thanks,

[Satoru Takabayashi]

How do we test changes in src/DEPS with the new git workflow?

I've added a new dependency to src/DEPS locally, but gclient sync didn't fetch the new dependency. Turned out gclient sync honored src/.DEPS.git instead, and the file says "DO NOT EDIT".

Per git log, .DEPS.git seems to be updated automatically by chrome-admin bot. Is there any way to update this file locally, so we can test changes in src/DEPS?

Satoru

[Chase Phillips]

Hi Satoru,

On Fri, Jan 6, 2012 at 10:03 AM, Satoru Takabayashi <sat...@chromium.org> wrote:

How do we test changes in src/DEPS with the new git workflow?

See the docs at http://code.google.com/p/chromium/wiki/UsingNewGit#Rolling_DEPS.

 

I've added a new dependency to src/DEPS locally, but gclient sync didn't fetch the new dependency. Turned out gclient sync honored src/.DEPS.git instead, and the file says "DO NOT EDIT".

Per git log, .DEPS.git seems to be updated automatically by chrome-admin bot. Is there any way to update this file locally, so we can test changes in src/DEPS?

The "DO NOT EDIT" is to discourage anyone from editing the file and checking their change back in which will just get overwritten by the automated deps2git system.  You can, if needed, edit the file locally and test changes with it.

Chase

[Satoru Takabayashi]

On Fri, Jan 6, 2012 at 4:54 PM, Chase Phillips <c...@google.com> wrote:

Hi Satoru,

On Fri, Jan 6, 2012 at 10:03 AM, Satoru Takabayashi <sat...@chromium.org> wrote:

How do we test changes in src/DEPS with the new git workflow?

See the docs at http://code.google.com/p/chromium/wiki/UsingNewGit#Rolling_DEPS.

Thanks. However, the extra manual steps described here look complicated and error-prone. Wouldn't it be a better solution to provide the script that converts DEPS to .DEPS.git?

I've added a new dependency to src/DEPS locally, but gclient sync didn't fetch the new dependency. Turned out gclient sync honored src/.DEPS.git instead, and the file says "DO NOT EDIT".

Per git log, .DEPS.git seems to be updated automatically by chrome-admin bot. Is there any way to update this file locally, so we can test changes in src/DEPS?

The "DO NOT EDIT" is to discourage anyone from editing the file and checking their change back in which will just get overwritten by the automated deps2git system.  You can, if needed, edit the file locally and test changes with it.

Maybe we should update the comment and add a link to the docs.

Satoru

[Chase Phillips]

On Fri, Jan 6, 2012 at 5:06 PM, Satoru Takabayashi <sat...@chromium.org> wrote:

On Fri, Jan 6, 2012 at 4:54 PM, Chase Phillips <c...@google.com> wrote:

Hi Satoru,

On Fri, Jan 6, 2012 at 10:03 AM, Satoru Takabayashi <sat...@chromium.org> wrote:

How do we test changes in src/DEPS with the new git workflow?

See the docs at http://code.google.com/p/chromium/wiki/UsingNewGit#Rolling_DEPS.

Thanks. However, the extra manual steps described here look complicated and error-prone. Wouldn't it be a better solution to provide the script that converts DEPS to .DEPS.git?

That solution would be fine if we can make the conversion script available externally.  If we can do that, we will.

 

I've added a new dependency to src/DEPS locally, but gclient sync didn't fetch the new dependency. Turned out gclient sync honored src/.DEPS.git instead, and the file says "DO NOT EDIT".

Per git log, .DEPS.git seems to be updated automatically by chrome-admin bot. Is there any way to update this file locally, so we can test changes in src/DEPS?

The "DO NOT EDIT" is to discourage anyone from editing the file and checking their change back in which will just get overwritten by the automated deps2git system.  You can, if needed, edit the file locally and test changes with it.

Maybe we should update the comment and add a link to the docs.

Changing the comment to add a link to the docs is probably worthwhile.  I pointed you to the script offline.  Feel free to make that change and upload a CL for review to nsylvain or myself.

Chase