Buildbot: Only archive core dumps from unexpected crashes.

Buildbot: Only archive core dumps from unexpected crashes.

Change UnexpectedCrashDumpArchiver to generate coredumps file in the current
directory. Each line of this file specifies a core and a binary to be
uploaded in the following format:

              test-name,core-file,binary-file

CoreDumpArchiver reads this file and uploads the first 10 crash dumps.

R=kustermann@google.com
BUG=

Committed: https://github.com/dart-lang/sdk/commit/e8f456edd53a85e1aaa0681b1f73a76cd4452ba1

[kustermann, 2017-01-20 14:56:55]

LGTM

https://codereview.chromium.org/2645963004/diff/1/tools/testing/dart/test_pro...
File tools/testing/dart/test_progress.dart (right):

https://codereview.chromium.org/2645963004/diff/1/tools/testing/dart/test_pro...
tools/testing/dart/test_progress.dart:352: new File('coredumps').openSync(mode:
FileMode.APPEND);
Instead of doing this for every test, how about opening it lazily and adding a
IOSink field.

You can use the 'void allDone() {}' callback to flush the stream and close the
file.

https://codereview.chromium.org/2645963004/diff/1/tools/testing/dart/test_pro...
tools/testing/dart/test_progress.dart:356: if (coredumpsList != null)
You could as well move the 'new File(...') out of the "try {" and get rid of
this "if (coredumpsList != null)".

https://codereview.chromium.org/2645963004/diff/1/tools/utils.py
File tools/utils.py (right):

https://codereview.chromium.org/2645963004/diff/1/tools/utils.py#newcode755
tools/utils.py:755: #
Normally python docs are like

def _find_core_dumps(self):
  """
  Load coredumps file ....
  """

(also above for the class)

https://codereview.chromium.org/2645963004/diff/1/tools/utils.py#newcode780
tools/utils.py:780: # binaries are copied next to cores and named
'core.<binary_name>'.
The binaries are now named: "binary.${mode}_${arch}_${binBaseName}"

https://codereview.chromium.org/2645963004/diff/1/tools/utils.py#newcode781
tools/utils.py:781: parts = os.path.basename(core).split('.')
Maybe simpler:

(prefix, rest) = 'a.b.c.d'.split('.', 1)
clean_name = rest if prefix == 'binary' else os.path.basename(core)

[Vyacheslav Egorov (Google), 2017-01-20 16:44:46]

Thanks for the review. Landing

https://codereview.chromium.org/2645963004/diff/1/tools/testing/dart/test_pro...
File tools/testing/dart/test_progress.dart (right):

https://codereview.chromium.org/2645963004/diff/1/tools/testing/dart/test_pro...
tools/testing/dart/test_progress.dart:352: new File('coredumps').openSync(mode:
FileMode.APPEND);
On 2017/01/20 14:56:55, kustermann wrote:
> Instead of doing this for every test, how about opening it lazily and adding a
> IOSink field.
> 
> You can use the 'void allDone() {}' callback to flush the stream and close the
> file.

I wanted to do it for each test so that even if test.dart runner crashes we
still have at least some things logged into the file. 

It's unlikely to matter - because there should be very few crashes anyway.

https://codereview.chromium.org/2645963004/diff/1/tools/testing/dart/test_pro...
tools/testing/dart/test_progress.dart:356: if (coredumpsList != null)
On 2017/01/20 14:56:55, kustermann wrote:
> You could as well move the 'new File(...') out of the "try {" and get rid of
> this "if (coredumpsList != null)".

I wanted to catch and ignore exceptions from openSync, but forgot to add that
line.

https://codereview.chromium.org/2645963004/diff/1/tools/utils.py
File tools/utils.py (right):

https://codereview.chromium.org/2645963004/diff/1/tools/utils.py#newcode755
tools/utils.py:755: #
On 2017/01/20 14:56:55, kustermann wrote:
> Normally python docs are like
> 
> def _find_core_dumps(self):
>   """
>   Load coredumps file ....
>   """
> 
> (also above for the class)

Done.

https://codereview.chromium.org/2645963004/diff/1/tools/utils.py#newcode780
tools/utils.py:780: # binaries are copied next to cores and named
'core.<binary_name>'.
On 2017/01/20 14:56:55, kustermann wrote:
> The binaries are now named: "binary.${mode}_${arch}_${binBaseName}"

Done.

https://codereview.chromium.org/2645963004/diff/1/tools/utils.py#newcode781
tools/utils.py:781: parts = os.path.basename(core).split('.')
On 2017/01/20 14:56:55, kustermann wrote:
> Maybe simpler:
> 
> (prefix, rest) = 'a.b.c.d'.split('.', 1)
> clean_name = rest if prefix == 'binary' else os.path.basename(core)

Done.

[Vyacheslav Egorov (Google), 2017-01-20 16:44:54]

Description was changed from

==========
Buildbot: Only archive core dumps from unexpected crashes.

Change UnexpectedCrashDumpArchiver to generate coredumps file in the current
directory. Each line of this file specifies a core and a binary to be
uploaded in the following format:

              test-name,core-file,binary-file

CoreDumpArchiver reads this file and uploads the first 10 crash dumps.

R=kustermann@google.com
BUG=
==========

to

==========
Buildbot: Only archive core dumps from unexpected crashes.

Change UnexpectedCrashDumpArchiver to generate coredumps file in the current
directory. Each line of this file specifies a core and a binary to be
uploaded in the following format:

              test-name,core-file,binary-file

CoreDumpArchiver reads this file and uploads the first 10 crash dumps.

R=kustermann@google.com
BUG=

Committed:
https://github.com/dart-lang/sdk/commit/e8f456edd53a85e1aaa0681b1f73a76cd4452ba1
==========

[Vyacheslav Egorov (Google), 2017-01-20 16:44:55]

Committed patchset #2 (id:20001) manually as
e8f456edd53a85e1aaa0681b1f73a76cd4452ba1 (presubmit successful).