Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(1)

Issue 27240: Make startup_tests build and run on Linux (except reference tests). (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 years, 2 months ago by Paweł Hajdan Jr.
Modified:
6 years ago
Reviewers:
agl, sgk
CC:
chromium-reviews_googlegroups.com
Visibility:
Public.

Description

Make startup_tests build and run on Linux (except reference tests).

Patch Set 1 #

Total comments: 2

Patch Set 2 : small updates #

Patch Set 3 : forgot about test_file_util_posix.cc #

Total comments: 5

Patch Set 4 : sync with trunk #

Patch Set 5 : small changes, remove bad code from scons #

Patch Set 6 : fix mac build #

Patch Set 7 : more fixes #

Patch Set 8 : cleaned up, without process_util changes #

Patch Set 9 : small bugfix in process_util #

Unified diffs Side-by-side diffs Delta from patch set Stats (+262 lines, -125 lines) Patch
M base/base.scons View 1 chunk +1 line, -0 lines 0 comments Download
M base/process_util_posix.cc View 7 1 chunk +1 line, -1 line 0 comments Download
A base/test_file_util_posix.cc View 1 chunk +118 lines, -0 lines 0 comments Download
M chrome/browser/browser_init.h View 1 chunk +0 lines, -2 lines 0 comments Download
M chrome/browser/browser_init.cc View 1 2 3 4 5 6 9 chunks +7 lines, -10 lines 0 comments Download
M chrome/browser/browser_main.cc View 1 2 3 4 5 6 1 chunk +0 lines, -1 line 0 comments Download
M chrome/chrome.gyp View 6 7 8 1 chunk +2 lines, -0 lines 0 comments Download
M chrome/common/chrome_constants.cc View 1 2 3 4 5 6 1 chunk +4 lines, -0 lines 0 comments Download
M chrome/test/automation/automation.scons View 1 chunk +0 lines, -1 line 0 comments Download
M chrome/test/automation/automation_proxy.h View 2 chunks +5 lines, -0 lines 0 comments Download
M chrome/test/automation/automation_proxy.cc View 1 1 chunk +11 lines, -0 lines 0 comments Download
M chrome/test/automation/browser_proxy.cc View 7 chunks +10 lines, -3 lines 0 comments Download
M chrome/test/startup/startup_test.cc View 1 2 3 4 6 chunks +26 lines, -30 lines 0 comments Download
M chrome/test/startup/startup_tests.scons View 1 2 3 4 4 chunks +19 lines, -4 lines 0 comments Download
M chrome/test/ui/ui_test.h View 1 2 3 4 5 6 3 chunks +1 line, -11 lines 0 comments Download
M chrome/test/ui/ui_test.cc View 1 2 3 4 5 6 7 15 chunks +52 lines, -60 lines 0 comments Download
M chrome/test/ui/ui_tests.scons View 3 chunks +5 lines, -2 lines 0 comments Download
Trybot results:
Commit queue not available (can’t edit this change).

Messages

Total messages: 12 (0 generated)
Paweł Hajdan Jr.
This contains parts of my other reviews, which I couldn't commit because of closed tree: ...
8 years, 2 months ago (2009-02-26 22:31:36 UTC) #1
agl
http://codereview.chromium.org/27240/diff/1/12 File chrome/test/automation/automation_proxy.cc (right): http://codereview.chromium.org/27240/diff/1/12#newcode411 Line 411: map.push_back(std::pair<int, int>(src_fd, dest_fd)); std::make_pair is probably better here. ...
8 years, 2 months ago (2009-02-27 03:28:41 UTC) #2
Paweł Hajdan Jr.
On 2009/02/27 03:28:41, agl wrote: > http://codereview.chromium.org/27240/diff/1/12 > File chrome/test/automation/automation_proxy.cc (right): > > http://codereview.chromium.org/27240/diff/1/12#newcode411 > ...
8 years, 2 months ago (2009-02-27 17:10:36 UTC) #3
Paweł Hajdan Jr.
agl: I forgot to add base/test_file_util.cc. Please make sure to review it.
8 years, 2 months ago (2009-02-27 17:23:23 UTC) #4
agl
http://codereview.chromium.org/27240/diff/1033/1035 File base/test_file_util_posix.cc (right): http://codereview.chromium.org/27240/diff/1033/1035#newcode47 Line 47: DCHECK(suffix[0] == '/'); DCHECK_EQ http://codereview.chromium.org/27240/diff/1033/1035#newcode54 Line 54: if ...
8 years, 2 months ago (2009-02-27 18:16:04 UTC) #5
Paweł Hajdan Jr.
Patch updated. I'll do above fixes in a follow-up, both in {test_,}file_util_posix.cc.
8 years, 2 months ago (2009-02-27 22:17:12 UTC) #6
sgk
Hmm, Depends() should do exactly what you want. If there's a dependency on the chrome ...
8 years, 2 months ago (2009-02-27 23:14:28 UTC) #7
Paweł Hajdan Jr.
agl: patch updated. sgk: Thanks for explaining. I removed dependency-related code for now. I would ...
8 years, 2 months ago (2009-03-02 17:40:29 UTC) #8
agl
LGTM
8 years, 2 months ago (2009-03-02 18:39:25 UTC) #9
Paweł Hajdan Jr.
sgk: ping. I'm waiting for your input. Please say if you think I can commit ...
8 years, 2 months ago (2009-03-03 10:09:29 UTC) #10
Paweł Hajdan Jr.
agl: Please take another look. I had to change few things after syncing with trunk ...
8 years, 2 months ago (2009-03-04 20:55:08 UTC) #11
Paweł Hajdan Jr.
8 years, 2 months ago (2009-03-05 09:45:49 UTC) #12
agl: I just made one change in process_util (it was my code I recently
commited): when status was -1, I should return true in CrashAwareSleep instead
of false (because that means the process may exist). It was breaking
startup_tests for Linux and I didn't wan't to further block this patch, so I
commited.

I'll submit a follow-up patch for process_util_posix to correct a small issue in
it (this "may" in explanation above).
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld 650457f06