Issue 1056733002: Run test tearDowns and clean up temporary directories when a signal is caught.

Issue 1056733002: Run test tearDowns and clean up temporary directories when a signal is caught. (Closed)

Created:
5 years, 8 months ago by nweiz

Modified:
5 years, 8 months ago

Reviewers:
kevmoo

CC:
reviews_dartlang.org

Base URL:
git@github.com:dart-lang/test@master

Target Ref:
refs/heads/master

Visibility:
Public.

More Reviews

Description

Run test tearDowns and clean up temporary directories when a signal is caught. Closes #26 R=kevmoo@google.com Committed: https://github.com/dart-lang/test/commit/7f54ecc494fc1b351004b4491b6a0a02e089cfd3

Patch Set 3 : Code review changes #

Total comments: 1

Patch Set 4 : Code review changes #

Created: 5 years, 8 months ago

Download [raw] [tar.bz2]

	Unified diffs	Side-by-side diffs	Delta from patch set	Stats (+618 lines, -56 lines)			Patch
A	lib/src/backend/closed_exception.dart	View		1 chunk	+13 lines, -0 lines	0 comments	Download
M	lib/src/backend/invoker.dart	View		5 chunks	+15 lines, -5 lines	0 comments	Download
M	lib/src/backend/live_test.dart	View		1 chunk	+7 lines, -1 line	0 comments	Download
M	lib/src/backend/live_test_controller.dart	View		4 chunks	+20 lines, -10 lines	0 comments	Download
M	lib/src/executable.dart	View		4 chunks	+42 lines, -2 lines	0 comments	Download
M	lib/src/frontend/expect.dart	View		2 chunks	+5 lines, -0 lines	0 comments	Download
M	lib/src/runner/browser/compiler_pool.dart	View	1	4 chunks	+36 lines, -4 lines	0 comments	Download
M	lib/src/runner/browser/iframe_listener.dart	View		1 chunk	+5 lines, -0 lines	0 comments	Download
M	lib/src/runner/browser/iframe_test.dart	View	1 2 3	2 chunks	+8 lines, -1 line	0 comments	Download
M	lib/src/runner/browser/server.dart	View		5 chunks	+24 lines, -4 lines	0 comments	Download
M	lib/src/runner/engine.dart	View		3 chunks	+12 lines, -2 lines	0 comments	Download
M	lib/src/runner/reporter/compact.dart	View		7 chunks	+21 lines, -8 lines	0 comments	Download
M	lib/src/runner/vm/isolate_listener.dart	View		1 chunk	+9 lines, -0 lines	0 comments	Download
M	lib/src/runner/vm/isolate_test.dart	View	1 2	4 chunks	+27 lines, -4 lines	0 comments	Download
M	lib/src/util/dart.dart	View		1 chunk	+1 line, -1 line	0 comments	Download
M	lib/src/util/io.dart	View	1	3 chunks	+16 lines, -5 lines	0 comments	Download
M	lib/src/utils.dart	View		2 chunks	+38 lines, -0 lines	0 comments	Download
M	pubspec.yaml	View	1	1 chunk	+1 line, -1 line	0 comments	Download
M	test/io.dart	View		2 chunks	+26 lines, -0 lines	0 comments	Download
M	test/runner/browser/compact_reporter_test.dart	View		2 chunks	+2 lines, -1 line	0 comments	Download
M	test/runner/browser/compiler_pool_test.dart	View		2 chunks	+2 lines, -1 line	0 comments	Download
M	test/runner/browser/loader_test.dart	View		2 chunks	+2 lines, -1 line	0 comments	Download
M	test/runner/browser/runner_test.dart	View		2 chunks	+2 lines, -1 line	0 comments	Download
M	test/runner/loader_test.dart	View		2 chunks	+2 lines, -1 line	0 comments	Download
M	test/runner/parse_metadata_test.dart	View		1 chunk	+2 lines, -1 line	0 comments	Download
M	test/runner/runner_test.dart	View		2 chunks	+2 lines, -1 line	0 comments	Download
A	test/runner/signal_test.dart	View	1 2	1 chunk	+277 lines, -0 lines	0 comments	Download
M	test/runner/test_on_test.dart	View		1 chunk	+1 line, -1 line	0 comments	Download

Messages

Total messages: 7 (0 generated)

Expand Messages | Collapse Messages

kevmoo

https://codereview.chromium.org/1056733002/diff/20001/lib/src/runner/browser/iframe_test.dart File lib/src/runner/browser/iframe_test.dart (right): https://codereview.chromium.org/1056733002/diff/20001/lib/src/runner/browser/iframe_test.dart#newcode31 lib/src/runner/browser/iframe_test.dart:31: var closeCompleter = new Completer(); This seems to not ...

5 years, 8 months ago (2015-04-03 00:49:45 UTC) #2

nweiz

5 years, 8 months ago (2015-04-03 20:56:22 UTC) #4

https://codereview.chromium.org/1056733002/diff/20001/lib/src/runner/browser/...
File lib/src/runner/browser/iframe_test.dart (right):

https://codereview.chromium.org/1056733002/diff/20001/lib/src/runner/browser/...
lib/src/runner/browser/iframe_test.dart:31: var closeCompleter = new
Completer();
On 2015/04/03 00:49:45, kevmoo wrote:
> This seems to not be used?

Done.

https://codereview.chromium.org/1056733002/diff/20001/lib/src/runner/vm/isola...
File lib/src/runner/vm/isolate_test.dart (right):

https://codereview.chromium.org/1056733002/diff/20001/lib/src/runner/vm/isola...
lib/src/runner/vm/isolate_test.dart:38: var closeCompleter = new Completer();
On 2015/04/03 00:49:45, kevmoo wrote:
> Is this used?

Done.

https://codereview.chromium.org/1056733002/diff/20001/test/runner/signal_test...
File test/runner/signal_test.dart (right):

https://codereview.chromium.org/1056733002/diff/20001/test/runner/signal_test...
test/runner/signal_test.dart:37: test("cleans up if killed while loading a VM
test", () {
On 2015/04/03 00:49:45, kevmoo wrote:
> Got a failure on my mac, built w/ latest
> 
> 06:17 +334 -1: test/runner/signal_test.dart: ... cleans up if killed while
> loading a VM test
>   Expected: empty
>     Actual: [
>     ?:Directory:
>
'/var/folders/j8/3kl8_xdn14x7mlbsq6_g8lfc0071_k/T/dart_test_YnbjgD/tmp/dart_test_O1h2TM'
>   ]
> 
>   package:test                        expect
>   test/runner/signal_test.dart 52:11  main.<fn>.<fn>.<fn>.<fn>
>   ===== asynchronous gap ===========================
>   dart:async                          _Future.then
>   test/runner/signal_test.dart 51:12  main.<fn>.<fn>.<fn>
>   ===== asynchronous gap ===========================
>   dart:async                          _Future.then
>   test/runner/signal_test.dart 46:44  main.<fn>.<fn>

I added a couple zeros to the for loop. This should ensure that the signal
always comes in while loading the test (this wasn't happening previously), and
thus avoid the flake. I'm still not sure what causes the error in the first
place, but I suspect it may just be some sort of race condition in dart:io
itself. Without a consistent, debuggable reproduction, there's not much to do
but avoid it.

kevmoo

LGTM https://codereview.chromium.org/1056733002/diff/40001/lib/src/runner/browser/iframe_test.dart File lib/src/runner/browser/iframe_test.dart (right): https://codereview.chromium.org/1056733002/diff/40001/lib/src/runner/browser/iframe_test.dart#newcode7 lib/src/runner/browser/iframe_test.dart:7: import 'dart:async'; unused import

5 years, 8 months ago (2015-04-03 22:48:32 UTC) #5

nweiz

5 years, 8 months ago (2015-04-03 23:05:41 UTC) #7

Message was sent while issue was closed.

Committed patchset #4 (id:60001) manually as
7f54ecc494fc1b351004b4491b6a0a02e089cfd3 (presubmit successful).

Expand Messages | Collapse Messages