Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Issue 116251 Be able to run tests from a read-only checkout.
Starred by 1 user Project Member Reported by maruel@google.com, Feb 29 2012 Back to list
Status: Assigned
Owner:
Components:
NextAction: ----
OS: All
Pri: 3
Type: Bug


Sign in to add a comment
Tests shouldn't be able to create any file in the current directory. Any file created should be in the temporary directly.
 
Project Member Comment 1 by bugdroid1@chromium.org, Mar 13 2012
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=126395

------------------------------------------------------------------------
r126395 | maruel@chromium.org | Tue Mar 13 07:37:55 PDT 2012

Changed paths:
 A http://src.chromium.org/viewvc/chrome/trunk/src/tools/isolate/strace_inputs.py?r1=126395&r2=126394&pathrev=126395

Add strace_inputs.py to strace a test executable and detect its dependencies.

It detects if all the files in a directories were read and put the dependency on
the directory entry instead of listing each individual files.

Detects all the temporary files that are created in the checkout directory and
that will need to be cleaned up to mark http://crbug.com/116251 fixed.

R=rogerta@chromium.org
BUG= 98640 
TEST=


Review URL: http://codereview.chromium.org/9689025
------------------------------------------------------------------------
Project Member Comment 2 by bugdroid1@chromium.org, Mar 10 2013
Blockedon: -chromium:98637 chromium:98637
Labels: -Area-Build -Type-Cleanup -Pri-2 Pri-3 Type-Bug Build
Project Member Comment 3 by bugdroid1@chromium.org, Jan 15 2014
------------------------------------------------------------------------
r244947 | maruel@chromium.org | 2014-01-15T18:48:42.280140Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/base_unittests.isolate?r1=244947&r2=244946&pathrev=244947
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/file_util_win.cc?r1=244947&r2=244946&pathrev=244947
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/file_util_unittest.cc?r1=244947&r2=244946&pathrev=244947

Make all the files mapped in when running base_unittests read only.

This means no file can be opened for write during the test run.

Make CopyFileUnsafe() reset the RO bit on Windows.
Add a tests that confirms the current CopyFile() behavior:
- On Windows, CopyFile() copies the ACL but now strips the READONLY bit.
- On OSX, CopyFile() copies the ACL.
- On anything else, ACL is not copied.

Rationale:
On anything-but-Windows, deleting a file require write access on the directory.
On Windows, deleting a file require not having the RO bit on the file.
CopyFile() affects the file but not the directory.

On isolated testing, the read only bit will be set on the file being copied,
causing the test to fail to delete the files.

This has wide implications in the unit tests. CopyFile() is mostly (but not
exclusively) used in unit tests.

R=thakis@chromium.org, vadimsh@chromium.org
BUG=116251

Review URL: https://codereview.chromium.org/136693004
------------------------------------------------------------------------
Project Member Comment 4 by bugdroid1@chromium.org, Jan 15 2014
------------------------------------------------------------------------
r244973 | maruel@chromium.org | 2014-01-15T20:38:25.088150Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/DEPS?r1=244973&r2=244972&pathrev=244973

Roll swarming_client @ 361bfda6c0e4e2469dd3dab8c2b741639cacb086.

This rolls a single important commit for read only support.

$ git log 8cacaaaffc..361bfda6 --date=short --format="%ad %ae %s" | sed 's/@chromium\.org//'
2014-01-15 maruel Change the default for isolate.py commands run & remap to copy files.

TBR=vadimsh@chromium.org
BUG=116251

Review URL: https://codereview.chromium.org/138063007
------------------------------------------------------------------------
Project Member Comment 5 by bugdroid1@chromium.org, Jan 15 2014
------------------------------------------------------------------------
r244979 | maruel@chromium.org | 2014-01-15T21:11:55.848509Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/file_util_unittest.cc?r1=244979&r2=244978&pathrev=244979

Fix CopyFileACL on Windows XP.

FILE_ATTRIBUTE_NOT_CONTENT_INDEXED doesn't exist on XP.

R=thakis@chromium.org
BUG=116251

Review URL: https://codereview.chromium.org/139373004
------------------------------------------------------------------------
Project Member Comment 6 by bugdroid1@chromium.org, Jan 15 2014
------------------------------------------------------------------------
r244986 | shashishekhar@chromium.org | 2014-01-15T22:14:44.084041Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/DEPS?r1=244986&r2=244985&pathrev=244986

Revert 244973 "Roll swarming_client @ 361bfda6c0e4e2469dd3dab8c2..."

> Roll swarming_client @ 361bfda6c0e4e2469dd3dab8c2b741639cacb086.
> 
> This rolls a single important commit for read only support.
> 
> $ git log 8cacaaaffc..361bfda6 --date=short --format="%ad %ae %s" | sed 's/@chromium\.org//'
> 2014-01-15 maruel Change the default for isolate.py commands run & remap to copy files.
> 
> TBR=vadimsh@chromium.org
> BUG=116251
> 
> Review URL: https://codereview.chromium.org/138063007

TBR=maruel@chromium.org

Review URL: https://codereview.chromium.org/140143002
------------------------------------------------------------------------
Project Member Comment 7 by bugdroid1@chromium.org, Jan 16 2014
------------------------------------------------------------------------
r245043 | maruel@chromium.org | 2014-01-16T00:59:54.776237Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/DEPS?r1=245043&r2=245042&pathrev=245043
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/base_unittests.isolate?r1=245043&r2=245042&pathrev=245043

Roll (bis) swarming_client @ 361bfda6c0e4e2469dd3dab8c2b741639cacb086.

This rolls a single important commit for read only support.

Remove read_only:1 from 'OS=="android"' in base_unittests.isolate. It is
currently not supported.

$ git log 8cacaaaffc..361bfda6 --date=short --format="%ad %ae %s" | sed 's/@chromium\.org//'
2014-01-15 maruel Change the default for isolate.py commands run & remap to copy files.

R=vadimsh@chromium.org
BUG=116251

Review URL: https://codereview.chromium.org/140333002
------------------------------------------------------------------------
Comment 8 by raymes@chromium.org, Jan 16 2014
start_crash_handler seemed to flake even with the above patch. See http://build.chromium.org/p/chromium.win/builders/Vista%20Tests%20%282%29/builds/40992
Project Member Comment 9 by bugdroid1@chromium.org, Feb 5 2014
------------------------------------------------------------------------
r249088 | maruel@chromium.org | 2014-02-05T19:55:52.079246Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/file_util_win.cc?r1=249088&r2=249087&pathrev=249088
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/file_util_unittest.cc?r1=249088&r2=249087&pathrev=249088
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/file_util.h?r1=249088&r2=249087&pathrev=249088
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/file_util_posix.cc?r1=249088&r2=249087&pathrev=249088

Make CopyDirectory() not copy the read only bit on Windows by reimplementing it.

Add CopyDirectoryACL test similar to CopyFileACL. Now both CopyDirectory and
CopyFile exhibits the same kind of behavior.

CopyDirectory is used in unit tests, on Windows installer and on OSX
web_application.

R=thakis@chromium.org,grt@chromium.org,rvargas@chromium.org
BUG=116251

Review URL: https://codereview.chromium.org/141273010
------------------------------------------------------------------------
Project Member Comment 10 by bugdroid1@chromium.org, Feb 6 2014
------------------------------------------------------------------------
r249258 | maruel@chromium.org | 2014-02-06T03:42:39.261611Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/file_util_unittest.cc?r1=249258&r2=249257&pathrev=249258
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/file_util.h?r1=249258&r2=249257&pathrev=249258
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/file_util_mac.mm?r1=249258&r2=249257&pathrev=249258

Modify CopyFileUnsafe() on OSX to stop copying ACL.

This makes CopyFile() and CopyDirectory() behavior consistent on OSX to the
other platforms. Stop copying the ACL on the files copied. This means the
destination file will inherit from the umask or inherited ACL from the
destination parent directory, which is sane.

This matches what is done on linux. This will fixes unit tests that copy files
from a read only directory into a temporary directory so they can be safely be
modified, otherwise the file acl would have to be modified everytime. While
base_unittests didn't care, it blew up disk cache tests in net_unittests.

R=thakis@chromium.org
BUG=116251, 335420 

Review URL: https://codereview.chromium.org/132183007
------------------------------------------------------------------------
Project Member Comment 11 by bugdroid1@chromium.org, Feb 6 2014
------------------------------------------------------------------------
r249500 | maruel@chromium.org | 2014-02-06T21:21:55.800992Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/net/net_unittests.isolate?r1=249500&r2=249499&pathrev=249500

Make all the files mapped in when running net_unittests read only.

This means no file can be opened for write during the test run.

R=vadimsh@chromium.org
BUG=116251
NOTRY=true

Review URL: https://codereview.chromium.org/133823003
------------------------------------------------------------------------
Blockedon: chromium:342913
Project Member Comment 13 by bugdroid1@chromium.org, Feb 13 2014
------------------------------------------------------------------------
r251054 | maruel@chromium.org | 2014-02-13T16:27:57.245347Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/unit_tests.isolate?r1=251054&r2=251053&pathrev=251054

Make all the files mapped in when running unit_tests read only on Linux and OSX.

This means no file can be opened for write during the test run.

Windows can't be done at the moment due to bug 342913.

R=vadimsh@chromium.org
BUG=116251
BUG= 342913 

Review URL: https://codereview.chromium.org/136973003
------------------------------------------------------------------------
Project Member Comment 14 by bugdroid1@chromium.org, Feb 13 2014
------------------------------------------------------------------------
r251074 | maruel@chromium.org | 2014-02-13T17:39:40.468103Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser_tests.isolate?r1=251074&r2=251073&pathrev=251074

Make all the files mapped in when running browser_tests read only.

This means no file can be opened for write during the test run.

R=vadimsh@chromium.org
NOTRY=true
BUG=116251

Review URL: https://codereview.chromium.org/137133002
------------------------------------------------------------------------
Project Member Comment 15 by bugdroid1@chromium.org, Feb 14 2014
------------------------------------------------------------------------
r251205 | maruel@chromium.org | 2014-02-14T00:30:49.875578Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/interactive_ui_tests.isolate?r1=251205&r2=251204&pathrev=251205

Make all the files mapped in when running interactive_ui_tests read only.

This means no file can be opened for write during the test run.

R=csharp@chromium.org
NOTRY=true
BUG=116251

Review URL: https://codereview.chromium.org/163553007
------------------------------------------------------------------------
Project Member Comment 16 by bugdroid1@chromium.org, Feb 14 2014
------------------------------------------------------------------------
r251411 | asvitkine@chromium.org | 2014-02-14T20:31:45.695858Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/unit_tests.isolate?r1=251411&r2=251410&pathrev=251411

Revert 251054 "Make all the files mapped in when running unit_te..."

Speculative revert, to see if this fixes CrOS MergeSession tests.

> Make all the files mapped in when running unit_tests read only on Linux and OSX.
> 
> This means no file can be opened for write during the test run.
> 
> Windows can't be done at the moment due to bug 342913.
> 
> R=vadimsh@chromium.org
> BUG=116251
> BUG= 342913 
> 
> Review URL: https://codereview.chromium.org/136973003

TBR=maruel@chromium.org

Review URL: https://codereview.chromium.org/166483006
------------------------------------------------------------------------
Project Member Comment 17 by bugdroid1@chromium.org, Feb 14 2014
------------------------------------------------------------------------
r251426 | asvitkine@chromium.org | 2014-02-14T21:25:03.028881Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/unit_tests.isolate?r1=251426&r2=251425&pathrev=251426

Revert 251411 "Revert 251054 "Make all the files mapped in when ..."

Apparently, couldn't be the cause. Reverting my revert.

> Revert 251054 "Make all the files mapped in when running unit_te..."
> 
> Speculative revert, to see if this fixes CrOS MergeSession tests.
> 
> > Make all the files mapped in when running unit_tests read only on Linux and OSX.
> > 
> > This means no file can be opened for write during the test run.
> > 
> > Windows can't be done at the moment due to bug 342913.
> > 
> > R=vadimsh@chromium.org
> > BUG=116251
> > BUG= 342913 
> > 
> > Review URL: https://codereview.chromium.org/136973003
> 
> TBR=maruel@chromium.org
> 
> Review URL: https://codereview.chromium.org/166483006

TBR=asvitkine@chromium.org

Review URL: https://codereview.chromium.org/164183010
------------------------------------------------------------------------
Most tests can now be run from read only files. The only exception is unit_tests on Windows.
Project Member Comment 19 by bugdroid1@chromium.org, Jul 20 2015
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/94bc4db5e341fbda8ce8f2210c9fc6b056a82868

commit 94bc4db5e341fbda8ce8f2210c9fc6b056a82868
Author: maruel <maruel@chromium.org>
Date: Mon Jul 20 19:45:45 2015

Remove 'read_only': 1 from most .isolate files.

These were mostly due to blind copy-paste. read_only:1 is the default so there's
no point in specifying it.

As per crbug.com/342913, only unit_tests has issue with running from read only
files.

R=jam@chromium.org
BUG=116251

Review URL: https://codereview.chromium.org/1240303002

Cr-Commit-Position: refs/heads/master@{#339490}

[modify] http://crrev.com/94bc4db5e341fbda8ce8f2210c9fc6b056a82868/base/base_unittests.isolate
[modify] http://crrev.com/94bc4db5e341fbda8ce8f2210c9fc6b056a82868/chrome/browser_tests.isolate
[modify] http://crrev.com/94bc4db5e341fbda8ce8f2210c9fc6b056a82868/chrome/interactive_ui_tests.isolate
[modify] http://crrev.com/94bc4db5e341fbda8ce8f2210c9fc6b056a82868/components/nacl_helper_nonsfi_unittests.isolate
[modify] http://crrev.com/94bc4db5e341fbda8ce8f2210c9fc6b056a82868/components/nacl_loader_unittests.isolate
[modify] http://crrev.com/94bc4db5e341fbda8ce8f2210c9fc6b056a82868/crypto/crypto_unittests.isolate
[modify] http://crrev.com/94bc4db5e341fbda8ce8f2210c9fc6b056a82868/net/net_unittests.isolate
[modify] http://crrev.com/94bc4db5e341fbda8ce8f2210c9fc6b056a82868/skia/skia_unittests.isolate
[modify] http://crrev.com/94bc4db5e341fbda8ce8f2210c9fc6b056a82868/testing/chromoting/chromoting_integration_tests.isolate
[modify] http://crrev.com/94bc4db5e341fbda8ce8f2210c9fc6b056a82868/tools/mb/mb.py

Project Member Comment 20 by sheriffbot@chromium.org, Jun 18
Labels: Hotlist-Google
Components: Infra>Client>Chrome Build
Components: -Build
Labels: -Build
Sign in to add a comment