From 79ff332afeb8e9ae88d966084e6569b7271e64ea Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Wed, 21 Jul 2021 20:32:39 -0500 Subject: [PATCH 1/5] test: global "all" hooks must only run once (#351) in other words, they can only run once per file --- tests/Hooks/AfterAllTest.php | 30 +++++++++++++++++++++++--- tests/Hooks/BeforeAllTest.php | 40 +++++++++++++++++++++++++++++------ tests/Pest.php | 5 ++++- 3 files changed, 65 insertions(+), 10 deletions(-) diff --git a/tests/Hooks/AfterAllTest.php b/tests/Hooks/AfterAllTest.php index a34a5847..fb8a71d4 100644 --- a/tests/Hooks/AfterAllTest.php +++ b/tests/Hooks/AfterAllTest.php @@ -2,26 +2,50 @@ global $globalHook; +// NOTE: this test does not have a $globalHook->calls offset since it is first +// in the directory and thus will always run before the others. See also the +// BeforeAllTest.php for details. + uses()->afterAll(function () use ($globalHook) { expect($globalHook) ->toHaveProperty('afterAll') ->and($globalHook->afterAll) - ->toBe(0); + ->toBe(0) + ->and($globalHook->calls) + ->afterAll + ->toBe(1); $globalHook->afterAll = 1; + $globalHook->calls->afterAll++; }); afterAll(function () use ($globalHook) { expect($globalHook) ->toHaveProperty('afterAll') ->and($globalHook->afterAll) - ->toBe(1); + ->toBe(1) + ->and($globalHook->calls) + ->afterAll + ->toBe(2); $globalHook->afterAll = 2; + $globalHook->calls->afterAll++; }); test('global afterAll execution order', function () use ($globalHook) { expect($globalHook) ->not() - ->toHaveProperty('afterAll'); + ->toHaveProperty('afterAll') + ->and($globalHook->calls) + ->afterAll + ->toBe(0); +}); + +test('it only gets called once per file', function () use ($globalHook) { + expect($globalHook) + ->not() + ->toHaveProperty('afterAll') + ->and($globalHook->calls) + ->afterAll + ->toBe(0); }); diff --git a/tests/Hooks/BeforeAllTest.php b/tests/Hooks/BeforeAllTest.php index 11c996c5..838d7c14 100644 --- a/tests/Hooks/BeforeAllTest.php +++ b/tests/Hooks/BeforeAllTest.php @@ -1,28 +1,56 @@ beforeAll(function () use ($globalHook) { +// HACK: we have to determine our $globalHook->calls baseline. This is because +// two other tests are executed before this one due to filename ordering. +$args = $_SERVER['argv'] ?? []; +$single = isset($args[1]) && Str::endsWith(__FILE__, $args[1]); +$offset = $single ? 0 : 2; + +uses()->beforeAll(function () use ($globalHook, $offset) { expect($globalHook) ->toHaveProperty('beforeAll') ->and($globalHook->beforeAll) - ->toBe(0); + ->toBe(0) + ->and($globalHook->calls) + ->beforeAll + ->toBe(1 + $offset); $globalHook->beforeAll = 1; + $globalHook->calls->beforeAll++; }); -beforeAll(function () use ($globalHook) { +beforeAll(function () use ($globalHook, $offset) { expect($globalHook) ->toHaveProperty('beforeAll') ->and($globalHook->beforeAll) - ->toBe(1); + ->toBe(1) + ->and($globalHook->calls) + ->beforeAll + ->toBe(2 + $offset); $globalHook->beforeAll = 2; + $globalHook->calls->beforeAll++; }); -test('global beforeAll execution order', function () use ($globalHook) { +test('global beforeAll execution order', function () use ($globalHook, $offset) { expect($globalHook) ->toHaveProperty('beforeAll') ->and($globalHook->beforeAll) - ->toBe(2); + ->toBe(2) + ->and($globalHook->calls) + ->beforeAll + ->toBe(3 + $offset); +}); + +it('only gets called once per file', function () use ($globalHook, $offset) { + expect($globalHook) + ->beforeAll + ->toBe(2) + ->and($globalHook->calls) + ->beforeAll + ->toBe(3 + $offset); }); diff --git a/tests/Pest.php b/tests/Pest.php index a8cd868d..d3fe584f 100644 --- a/tests/Pest.php +++ b/tests/Pest.php @@ -2,7 +2,8 @@ uses()->group('integration')->in('Visual'); -$globalHook = (object) []; // NOTE: global test value container to be mutated and checked across files, as needed +// NOTE: global test value container to be mutated and checked across files, as needed +$globalHook = (object) ['calls' => (object) ['beforeAll' => 0, 'afterAll' => 0]]; uses() ->beforeEach(function () { @@ -10,11 +11,13 @@ uses() }) ->beforeAll(function () use ($globalHook) { $globalHook->beforeAll = 0; + $globalHook->calls->beforeAll++; }) ->afterEach(function () { $this->ith = 0; }) ->afterAll(function () use ($globalHook) { $globalHook->afterAll = 0; + $globalHook->calls->afterAll++; }) ->in('Hooks'); From 60c0ad006f866f7e28dd45b6e324736d63692321 Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Wed, 21 Jul 2021 20:38:20 -0500 Subject: [PATCH 2/5] fix: prevent the global hooks from piling up (#351) this happens due to the global `*All()` hooks (before & after) being defined as static. We should be a good citizen and we need to clean up our mess for the next person in the test instance constructor --- src/Concerns/Testable.php | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Concerns/Testable.php b/src/Concerns/Testable.php index 5c72e0f3..91b73d72 100644 --- a/src/Concerns/Testable.php +++ b/src/Concerns/Testable.php @@ -73,6 +73,7 @@ trait Testable { $this->__test = $test; $this->__description = $description; + self::$beforeAll = self::$afterAll = null; parent::__construct('__test', $data); } From b6012862c4cbb02b85f8dd0193a4fc819180d83b Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Wed, 21 Jul 2021 20:43:21 -0500 Subject: [PATCH 3/5] style: run linter --- tests/Hooks/BeforeAllTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Hooks/BeforeAllTest.php b/tests/Hooks/BeforeAllTest.php index 838d7c14..05e57252 100644 --- a/tests/Hooks/BeforeAllTest.php +++ b/tests/Hooks/BeforeAllTest.php @@ -6,7 +6,7 @@ global $globalHook; // HACK: we have to determine our $globalHook->calls baseline. This is because // two other tests are executed before this one due to filename ordering. -$args = $_SERVER['argv'] ?? []; +$args = $_SERVER['argv'] ?? []; $single = isset($args[1]) && Str::endsWith(__FILE__, $args[1]); $offset = $single ? 0 : 2; From d217503a6abf126ce6617eb1c783fea0a72bb851 Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Wed, 21 Jul 2021 20:47:48 -0500 Subject: [PATCH 4/5] test: use consistent test descriptors --- tests/Hooks/AfterAllTest.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/Hooks/AfterAllTest.php b/tests/Hooks/AfterAllTest.php index fb8a71d4..b9042815 100644 --- a/tests/Hooks/AfterAllTest.php +++ b/tests/Hooks/AfterAllTest.php @@ -41,7 +41,7 @@ test('global afterAll execution order', function () use ($globalHook) { ->toBe(0); }); -test('it only gets called once per file', function () use ($globalHook) { +it('only gets called once per file', function () use ($globalHook) { expect($globalHook) ->not() ->toHaveProperty('afterAll') From 863ddea50b5269a7b9b7e83067306169cb398b9b Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Thu, 22 Jul 2021 17:54:22 -0500 Subject: [PATCH 5/5] style: split assignment into two lines clarity --- src/Concerns/Testable.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Concerns/Testable.php b/src/Concerns/Testable.php index 91b73d72..d8137311 100644 --- a/src/Concerns/Testable.php +++ b/src/Concerns/Testable.php @@ -73,7 +73,8 @@ trait Testable { $this->__test = $test; $this->__description = $description; - self::$beforeAll = self::$afterAll = null; + self::$beforeAll = null; + self::$afterAll = null; parent::__construct('__test', $data); }