Create directory if the given gtest_output path is in an unexisting directory.

Create directory if the given gtest_output path is in an unexisting directory.

BUG=42781
TEST=Run one of out-of-process-tests (e.g. browser_tests) with --gtest_output=xml:unexisting_dir/output.xml parameter and see if the specified unexisting_dir is successfully created (if it's in a writable location).

Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=48468

[Paweł Hajdan Jr., 2010-05-27 11:30:36]

Could you provide some rationale why we should do it? I'd prefer to avoid
"magically" created directories. Usually when the user supplies a directory, it
is expected to exist. When it doesn't, it's usually a mistake, and just failing
instead of silently "correcting" it helps with detection.

[kinuko, 2010-05-27 12:38:43]

On 2010/05/27 11:30:36, Paweł Hajdan Jr. wrote:
> Could you provide some rationale why we should do it? I'd prefer to avoid
> "magically" created directories. Usually when the user supplies a directory,
it
> is expected to exist. When it doesn't, it's usually a mistake, and just
failing
> instead of silently "correcting" it helps with detection.

I wouldn't usually do this either, but the original gtest seems to do so (e.g.
tries to create directories recursively) if --gtest_output parameter value
contains directories.  Because this part (xml output part) is basically an
'emulation' of the original gtest behavior I didn't want to make them behave
differently.
I added a comment to the line.

[Paweł Hajdan Jr., 2010-05-27 12:49:42]

Thanks for explaining. Just one simple suggestion to log a warning in that case.

http://codereview.chromium.org/2302003/diff/4001/5001
File chrome/test/test_launcher/test_runner.cc (right):

http://codereview.chromium.org/2302003/diff/4001/5001#newcode76
chrome/test/test_launcher/test_runner.cc:76: if
(file_util::CreateDirectory(path.DirName())) {
How about:
if (dir doesn't exist) {
  LOG(WARNING) << (the directory doesn't exist);
  // Create the directory if necessary (gtest does the same).
  file_util::CreateDirectory(...);
}

[kinuko, 2010-05-27 13:12:00]

Adding a warning sounds good, updated the patch.

http://codereview.chromium.org/2302003/diff/4001/5001
File chrome/test/test_launcher/test_runner.cc (right):

http://codereview.chromium.org/2302003/diff/4001/5001#newcode76
chrome/test/test_launcher/test_runner.cc:76: if
(file_util::CreateDirectory(path.DirName())) {
On 2010/05/27 12:49:42, Paweł Hajdan Jr. wrote:
> How about:
> if (dir doesn't exist) {
>   LOG(WARNING) << (the directory doesn't exist);
>   // Create the directory if necessary (gtest does the same).
>   file_util::CreateDirectory(...);
> }

Done.

[Paweł Hajdan Jr., 2010-05-27 14:58:44]

After fixing one comment LGTM.

http://codereview.chromium.org/2302003/diff/13001/14001
File chrome/test/test_launcher/test_runner.cc (right):

http://codereview.chromium.org/2302003/diff/13001/14001#newcode77
chrome/test/test_launcher/test_runner.cc:77: << std::endl;
Please add info to the warning that the directory will be created.

[kinuko, 2010-05-28 02:09:21]

Thanks for your review, fixed the WARNING text.  Will be submitting.

On 2010/05/27 14:58:44, Paweł Hajdan Jr. wrote:
> After fixing one comment LGTM.
> 
> http://codereview.chromium.org/2302003/diff/13001/14001
> File chrome/test/test_launcher/test_runner.cc (right):
> 
> http://codereview.chromium.org/2302003/diff/13001/14001#newcode77
> chrome/test/test_launcher/test_runner.cc:77: << std::endl;
> Please add info to the warning that the directory will be created.