Commit 6af4049
test_runner: simplify test end time tracking
This commit simplifies the logic for tracking test end time.
The end time is now only set in postRun(), which every test
runs when it ends.
PR-URL: #52182
Reviewed-By: Yagiz Nizipli <yagiz.nizipli@sentry.io>
Reviewed-By: Chemi Atlow <chemi@atlow.co.il>1 parent 878047b commit 6af4049
2 files changed
Lines changed: 15 additions & 26 deletions
File tree
- lib/internal/test_runner
- test/fixtures/test-runner/output
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -526,7 +526,7 @@ class Test extends AsyncResource { | |||
| 526 | 526 | }; | |
| 527 | 527 | ||
| 528 | 528 | #cancel(error) { | |
| 529 | - if (this.endTime !== null) { | ||
| 529 | + if (this.endTime !== null || this.error !== null) { | ||
| 530 | 530 | return; | |
| 531 | 531 | } | |
| 532 | 532 | ||
@@ -564,17 +564,15 @@ class Test extends AsyncResource { | |||
| 564 | 564 | return; | |
| 565 | 565 | } | |
| 566 | 566 | ||
| 567 | - this.endTime = hrtime(); | ||
| 568 | 567 | this.passed = false; | |
| 569 | 568 | this.error = err; | |
| 570 | 569 | } | |
| 571 | 570 | ||
| 572 | 571 | pass() { | |
| 573 | - if (this.endTime !== null) { | ||
| 572 | + if (this.error !== null) { | ||
| 574 | 573 | return; | |
| 575 | 574 | } | |
| 576 | 575 | ||
| 577 | - this.endTime = hrtime(); | ||
| 578 | 576 | this.passed = true; | |
| 579 | 577 | } | |
| 580 | 578 | ||
@@ -707,15 +705,8 @@ class Test extends AsyncResource { | |||
| 707 | 705 | } | |
| 708 | 706 | ||
| 709 | 707 | this.pass(); | |
| 710 | - try { | ||
| 711 | - await afterEach(); | ||
| 712 | - await after(); | ||
| 713 | - } catch (err) { | ||
| 714 | - // If one of the after hooks has thrown unset endTime so that the | ||
| 715 | - // catch below can do its cancel/fail logic. | ||
| 716 | - this.endTime = null; | ||
| 717 | - throw err; | ||
| 718 | - } | ||
| 708 | + await afterEach(); | ||
| 709 | + await after(); | ||
| 719 | 710 | } catch (err) { | |
| 720 | 711 | if (isTestFailureError(err)) { | |
| 721 | 712 | if (err.failureType === kTestTimeoutFailure) { | |
@@ -761,13 +752,10 @@ class Test extends AsyncResource { | |||
| 761 | 752 | } | |
| 762 | 753 | ||
| 763 | 754 | postRun(pendingSubtestsError) { | |
| 764 | - this.startTime ??= hrtime(); | ||
| 765 | - | ||
| 766 | - // If the test was failed before it even started, then the end time will | ||
| 767 | - // be earlier than the start time. Correct that here. | ||
| 768 | - if (this.endTime < this.startTime) { | ||
| 769 | - this.endTime = hrtime(); | ||
| 770 | - } | ||
| 755 | + // If the test was cancelled before it started, then the start and end | ||
| 756 | + // times need to be corrected. | ||
| 757 | + this.endTime ??= hrtime(); | ||
| 758 | + this.startTime ??= this.endTime; | ||
| 771 | 759 | ||
| 772 | 760 | // The test has run, so recursively cancel any outstanding subtests and | |
| 773 | 761 | // mark this test as failed if any subtests failed. | |
@@ -974,6 +962,7 @@ class TestHook extends Test { | |||
| 974 | 962 | error.failureType = kHookFailure; | |
| 975 | 963 | } | |
| 976 | 964 | ||
| 965 | + this.endTime ??= hrtime(); | ||
| 977 | 966 | parent.reporter.fail(0, loc, parent.subtests.length + 1, loc.file, { | |
| 978 | 967 | __proto__: null, | |
| 979 | 968 | duration_ms: this.duration(), | |
Lines changed: 6 additions & 6 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -11,10 +11,10 @@ | |||
| 11 | 11 | ||
| 12 | 12 | describe hooks - no subtests (*ms) | |
| 13 | 13 | before throws | |
| 14 | - 1 (*ms) | ||
| 14 | + 1 | ||
| 15 | 15 | 'test did not finish before its parent and was cancelled' | |
| 16 | 16 | ||
| 17 | - 2 (*ms) | ||
| 17 | + 2 | ||
| 18 | 18 | 'test did not finish before its parent and was cancelled' | |
| 19 | 19 | ||
| 20 | 20 | before throws (*ms) | |
@@ -390,7 +390,7 @@ | |||
| 390 | 390 | ||
| 391 | 391 | - after() called | |
| 392 | 392 | run after when before throws | |
| 393 | - 1 (*ms) | ||
| 393 | + 1 | ||
| 394 | 394 | 'test did not finish before its parent and was cancelled' | |
| 395 | 395 | ||
| 396 | 396 | run after when before throws (*ms) | |
@@ -422,11 +422,11 @@ | |||
| 422 | 422 | failing tests: | |
| 423 | 423 | ||
| 424 | 424 | * | |
| 425 | - 1 (*ms) | ||
| 425 | + 1 | ||
| 426 | 426 | 'test did not finish before its parent and was cancelled' | |
| 427 | 427 | ||
| 428 | 428 | * | |
| 429 | - 2 (*ms) | ||
| 429 | + 2 | ||
| 430 | 430 | 'test did not finish before its parent and was cancelled' | |
| 431 | 431 | ||
| 432 | 432 | * | |
@@ -772,7 +772,7 @@ | |||
| 772 | 772 | * | |
| 773 | 773 | ||
| 774 | 774 | * | |
| 775 | - 1 (*ms) | ||
| 775 | + 1 | ||
| 776 | 776 | 'test did not finish before its parent and was cancelled' | |
| 777 | 777 | ||
| 778 | 778 | * | |
0 commit comments