Crash dump test: Isolate the test from any global instance of crash_service.exe

Crash dump test: Isolate the test from any global instance of crash_service.exe

Use a freshly-created temporary directory and a fresh Windows pipe name.
I recently changed crash_service.exe to support these new options.

BUG=http://code.google.com/p/nativeclient/issues/detail?id=2006
TEST=chrome_browser_tests on a 32-bit Windows system

Committed: http://src.chromium.org/viewvc/native_client?view=rev&revision=6418

[Nick Bray, 2011-08-12 21:46:27]

http://codereview.chromium.org/7569002/diff/2001/tests/inbrowser_crash_test/c...
File tests/inbrowser_crash_test/crash_dump_tester.py (right):

http://codereview.chromium.org/7569002/diff/2001/tests/inbrowser_crash_test/c...
tests/inbrowser_crash_test/crash_dump_tester.py:48: # an 'Access is denied'
WindowsError exception.
... which should not happen in normal operation?

http://codereview.chromium.org/7569002/diff/2001/tests/inbrowser_crash_test/c...
tests/inbrowser_crash_test/crash_dump_tester.py:83:
os.environ['CHROME_BREAKPAD_PIPE_NAME'] = windows_pipe_name
Hmmm.  Everything else being equal, I'd rather see these environment variables
passed to BrowserLauncher rather than modifying os.environ.  This isn't a small
change, so I won't require it.

http://codereview.chromium.org/7569002/diff/2001/tests/inbrowser_crash_test/c...
tests/inbrowser_crash_test/crash_dump_tester.py:92: all_files =
[os.path.join(dumps_dir, dump_file)
Finding the dump files should be a function taking dumps_dir.

http://codereview.chromium.org/7569002/diff/2001/tests/inbrowser_crash_test/c...
tests/inbrowser_crash_test/crash_dump_tester.py:136: shutil.rmtree(dumps_dir)
This may occasionally blow up on Windows. You should share the directory removal
code with browserlauncher.py.

[Mark Seaborn, 2011-08-12 22:13:45]

On 12 August 2011 14:46, <ncbray@google.com> wrote:

>
> http://codereview.chromium.**org/7569002/diff/2001/tests/**
>
inbrowser_crash_test/crash_**dump_tester.py<http://codereview.chromium.org/7569002/diff/2001/tests/inbrowser_crash_test/crash_dump_tester.py>
> File tests/inbrowser_crash_test/**crash_dump_tester.py (right):
>
> http://codereview.chromium.**org/7569002/diff/2001/tests/**
>
inbrowser_crash_test/crash_**dump_tester.py#newcode48<http://codereview.chromium.org/7569002/diff/2001/tests/inbrowser_crash_test/crash_dump_tester.py#newcode48>
> tests/inbrowser_crash_test/**crash_dump_tester.py:48: # an 'Access is
> denied' WindowsError exception.
> ... which should not happen in normal operation?
>

Commented more.


> http://codereview.chromium.**org/7569002/diff/2001/tests/**
>
inbrowser_crash_test/crash_**dump_tester.py#newcode83<http://codereview.chromium.org/7569002/diff/2001/tests/inbrowser_crash_test/crash_dump_tester.py#newcode83>
> tests/inbrowser_crash_test/**crash_dump_tester.py:83:
> os.environ['CHROME_BREAKPAD_**PIPE_NAME'] = windows_pipe_name
> Hmmm.  Everything else being equal, I'd rather see these environment
> variables passed to BrowserLauncher rather than modifying os.environ.
> This isn't a small change, so I won't require it.
>

Maybe.  It set os.environ before though -- one change at a time.


> http://codereview.chromium.**org/7569002/diff/2001/tests/**
>
inbrowser_crash_test/crash_**dump_tester.py#newcode92<http://codereview.chromium.org/7569002/diff/2001/tests/inbrowser_crash_test/crash_dump_tester.py#newcode92>
> tests/inbrowser_crash_test/**crash_dump_tester.py:92: all_files =
> [os.path.join(dumps_dir, dump_file)
> Finding the dump files should be a function taking dumps_dir.
>

OK, done.


> http://codereview.chromium.**org/7569002/diff/2001/tests/**
>
inbrowser_crash_test/crash_**dump_tester.py#newcode136<http://codereview.chromium.org/7569002/diff/2001/tests/inbrowser_crash_test/crash_dump_tester.py#newcode136>
> tests/inbrowser_crash_test/**crash_dump_tester.py:136:
> shutil.rmtree(dumps_dir)
> This may occasionally blow up on Windows. You should share the directory
> removal code with browserlauncher.py.
>

Done.

Mark

-- 
You received this message because you are subscribed to the Google Groups
"Native-Client-Reviews" group.
To post to this group, send email to native-client-reviews@googlegroups.com.
To unsubscribe from this group, send email to
native-client-reviews+unsubscribe@googlegroups.com.
For more options, visit this group at
http://groups.google.com/group/native-client-reviews?hl=en.

[Nick Bray, 2011-08-12 22:17:45]

LGTM