From 9e5b779abc74e7d4a8045a4b96fcf7802f4bbaf0 Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Wed, 16 Jun 2021 00:52:31 -0500 Subject: [PATCH 01/13] feat: add helper function for mapping a function's arguments --- src/Support/Reflection.php | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/src/Support/Reflection.php b/src/Support/Reflection.php index c25709b9..1fac05b7 100644 --- a/src/Support/Reflection.php +++ b/src/Support/Reflection.php @@ -153,4 +153,21 @@ final class Reflection return $name; } + + /** + * Receive a map of function argument names to their types. + * + * @return array + */ + public static function getFunctionArguments(Closure $function): array + { + $parameters = (new ReflectionFunction($function))->getParameters(); + $arguments = []; + + foreach ($parameters as $parameter) { + $arguments[$parameter->getName()] = ($parameter->hasType()) ? $parameter->getType() : 'mixed'; + } + + return $arguments; + } } From 99040945908c97219f03b0714ceb23e0644e395d Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Wed, 16 Jun 2021 00:53:17 -0500 Subject: [PATCH 02/13] feat: add new exception for missing datasets on tests with arguments --- src/Exceptions/DatasetMissing.php | 36 +++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) create mode 100644 src/Exceptions/DatasetMissing.php diff --git a/src/Exceptions/DatasetMissing.php b/src/Exceptions/DatasetMissing.php new file mode 100644 index 00000000..5613d02e --- /dev/null +++ b/src/Exceptions/DatasetMissing.php @@ -0,0 +1,36 @@ + $args A map of argument names to their typee + */ + public function __construct(string $file, string $name, array $args) + { + parent::__construct(sprintf( + "A test with the description '%s' has %d argument(s) ([%s]) and no dataset(s) provided in %s", + $name, + count($args), + implode(', ', array_map(static function (string $arg, string $type): string { + return sprintf('%s $%s', $type, $arg); + }, array_keys($args), $args)), + $file, + )); + } +} From 9bf141f698169839761ff32628ac2175b37ea277 Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Wed, 16 Jun 2021 00:54:22 -0500 Subject: [PATCH 03/13] style: formatting & linting --- src/Exceptions/DatasetMissing.php | 2 +- src/Support/Reflection.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Exceptions/DatasetMissing.php b/src/Exceptions/DatasetMissing.php index 5613d02e..b8f0cb2d 100644 --- a/src/Exceptions/DatasetMissing.php +++ b/src/Exceptions/DatasetMissing.php @@ -10,7 +10,7 @@ use NunoMaduro\Collision\Contracts\RenderlessTrace; use Symfony\Component\Console\Exception\ExceptionInterface; /** - * Creates a new instance of dataset is not present for test that has arguments + * Creates a new instance of dataset is not present for test that has arguments. * * @internal */ diff --git a/src/Support/Reflection.php b/src/Support/Reflection.php index 1fac05b7..8fdd08fd 100644 --- a/src/Support/Reflection.php +++ b/src/Support/Reflection.php @@ -162,7 +162,7 @@ final class Reflection public static function getFunctionArguments(Closure $function): array { $parameters = (new ReflectionFunction($function))->getParameters(); - $arguments = []; + $arguments = []; foreach ($parameters as $parameter) { $arguments[$parameter->getName()] = ($parameter->hasType()) ? $parameter->getType() : 'mixed'; From 64e780cf7278d3354610d70dbd16df7960dd3d16 Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Wed, 16 Jun 2021 00:58:46 -0500 Subject: [PATCH 04/13] fix: types --- src/Support/Reflection.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Support/Reflection.php b/src/Support/Reflection.php index 8fdd08fd..1ad05804 100644 --- a/src/Support/Reflection.php +++ b/src/Support/Reflection.php @@ -165,7 +165,7 @@ final class Reflection $arguments = []; foreach ($parameters as $parameter) { - $arguments[$parameter->getName()] = ($parameter->hasType()) ? $parameter->getType() : 'mixed'; + $arguments[$parameter->getName()] = ($parameter->hasType()) ? (string) $parameter->getType() : 'mixed'; } return $arguments; From 9d66893d5a65faec6823f932e00fab319ef56ade Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Wed, 16 Jun 2021 01:00:23 -0500 Subject: [PATCH 05/13] feat: add boolean property to signal dependency proxy calls --- src/Factories/TestCaseFactory.php | 7 +++++++ src/PendingObjects/TestCall.php | 2 ++ 2 files changed, 9 insertions(+) diff --git a/src/Factories/TestCaseFactory.php b/src/Factories/TestCaseFactory.php index 6efe63f4..63eafaff 100644 --- a/src/Factories/TestCaseFactory.php +++ b/src/Factories/TestCaseFactory.php @@ -66,6 +66,13 @@ final class TestCaseFactory */ public $datasets = []; + /** + * Has the current test been marked dependent on others? + * + * @var bool + */ + public $dependent = false; + /** * The FQN of the test case class. * diff --git a/src/PendingObjects/TestCall.php b/src/PendingObjects/TestCall.php index 39486565..6cf9f0f3 100644 --- a/src/PendingObjects/TestCall.php +++ b/src/PendingObjects/TestCall.php @@ -97,6 +97,8 @@ final class TestCall ->factoryProxies ->add(Backtrace::file(), Backtrace::line(), 'addDependencies', [$tests]); + $this->testCaseFactory->dependent = true; + return $this; } From 6a7ee90ff5d80edac9339dc5107c48f0ebf18124 Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Wed, 16 Jun 2021 01:01:34 -0500 Subject: [PATCH 06/13] feat: throw user-friendly exception for missing argument data --- src/Repositories/TestRepository.php | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/Repositories/TestRepository.php b/src/Repositories/TestRepository.php index b2eb4893..305dc127 100644 --- a/src/Repositories/TestRepository.php +++ b/src/Repositories/TestRepository.php @@ -5,11 +5,13 @@ declare(strict_types=1); namespace Pest\Repositories; use Closure; +use Pest\Exceptions\DatasetMissing; use Pest\Exceptions\ShouldNotHappen; use Pest\Exceptions\TestAlreadyExist; use Pest\Exceptions\TestCaseAlreadyInUse; use Pest\Exceptions\TestCaseClassOrTraitNotFound; use Pest\Factories\TestCaseFactory; +use Pest\Support\Reflection; use Pest\Support\Str; use Pest\TestSuite; use PHPUnit\Framework\TestCase; @@ -140,6 +142,14 @@ final class TestRepository throw new TestAlreadyExist($test->filename, $test->description); } + if ($test->dependent === false && count($test->datasets) === 0) { + $arguments = Reflection::getFunctionArguments($test->test); + + if (count($arguments) > 0) { + throw new DatasetMissing($test->filename, $test->description, $arguments); + } + } + $this->state[sprintf('%s%s%s', $test->filename, self::SEPARATOR, $test->description)] = $test; } } From e21e3080e0bc4f6ff2ebff50bbbb3796d3edaafd Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Wed, 16 Jun 2021 14:04:50 -0500 Subject: [PATCH 07/13] test: add new exception check & update snapshots --- tests/.snapshots/success.txt | 3 ++- tests/Unit/TestSuite.php | 9 +++++++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/.snapshots/success.txt b/tests/.snapshots/success.txt index 9f57d4fa..6328d9a5 100644 --- a/tests/.snapshots/success.txt +++ b/tests/.snapshots/success.txt @@ -527,6 +527,7 @@ PASS Tests\Unit\TestSuite ✓ it does not allow to add the same test description twice + ✓ it alerts users about tests with arguments but no input PASS Tests\Visual\Help ✓ visual snapshot of help command output @@ -554,5 +555,5 @@ ✓ it is a test ✓ it uses correct parent class - Tests: 4 incompleted, 7 skipped, 340 passed + Tests: 4 incompleted, 7 skipped, 341 passed \ No newline at end of file diff --git a/tests/Unit/TestSuite.php b/tests/Unit/TestSuite.php index 74d9c092..c26149c8 100644 --- a/tests/Unit/TestSuite.php +++ b/tests/Unit/TestSuite.php @@ -1,5 +1,6 @@ expectExceptionMessage(sprintf('A test with the description `%s` already exist in the filename `%s`.', 'foo', __FILE__)); $testSuite->tests->set(new \Pest\Factories\TestCaseFactory(__FILE__, 'foo', $test)); }); + +it('alerts users about tests with arguments but no input', function () { + $testSuite = new TestSuite(getcwd(), 'tests'); + $test = function (int $arg) {}; + $this->expectException(DatasetMissing::class); + $this->expectExceptionMessage(sprintf("A test with the description '%s' has %d argument(s) ([%s]) and no dataset(s) provided in %s", 'foo', 1, 'int $arg', __FILE__)); + $testSuite->tests->set(new \Pest\Factories\TestCaseFactory(__FILE__, 'foo', $test)); +}); From 2e7192ab9565598c550ff53cd4a945db90639908 Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Wed, 16 Jun 2021 14:42:51 -0500 Subject: [PATCH 08/13] chore: static analysis adjustments - deprecation RE: ReflectionType::__toString --- src/Support/Reflection.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/Support/Reflection.php b/src/Support/Reflection.php index 1ad05804..7d46ece9 100644 --- a/src/Support/Reflection.php +++ b/src/Support/Reflection.php @@ -165,7 +165,9 @@ final class Reflection $arguments = []; foreach ($parameters as $parameter) { - $arguments[$parameter->getName()] = ($parameter->hasType()) ? (string) $parameter->getType() : 'mixed'; + /** @var ReflectionNamedType|null $type */ + $type = ($parameter->hasType()) ? $parameter->getType() : null; + $arguments[$parameter->getName()] = (is_null($type)) ? 'mixed' : $type->getName(); } return $arguments; From 553b45306fe50ed3aa51db521d2b501c1e2b69e7 Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Wed, 16 Jun 2021 21:29:08 -0500 Subject: [PATCH 09/13] feat: handle unions (PHP 8) --- src/Support/Reflection.php | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/Support/Reflection.php b/src/Support/Reflection.php index 7d46ece9..1719b9f6 100644 --- a/src/Support/Reflection.php +++ b/src/Support/Reflection.php @@ -12,6 +12,7 @@ use ReflectionException; use ReflectionFunction; use ReflectionNamedType; use ReflectionParameter; +use ReflectionUnionType; /** * @internal @@ -165,9 +166,23 @@ final class Reflection $arguments = []; foreach ($parameters as $parameter) { - /** @var ReflectionNamedType|null $type */ - $type = ($parameter->hasType()) ? $parameter->getType() : null; - $arguments[$parameter->getName()] = (is_null($type)) ? 'mixed' : $type->getName(); + /** @var ReflectionNamedType|ReflectionUnionType|null $types */ + $types = ($parameter->hasType()) ? $parameter->getType() : null; + + if (is_null($types)) { + $arguments[$parameter->getName()] = 'mixed'; + + continue; + } + + $arguments[$parameter->getName()] = implode('|', array_map( + static function (ReflectionNamedType $type): string { + return $type->getName(); + }, + ($types instanceof ReflectionNamedType) + ? [$types] // NOTE: normalize as list of to handle unions + : $types->getTypes(), + )); } return $arguments; From e64856c664f9cb530a4c70eb779187ed125e3710 Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Thu, 17 Jun 2021 13:33:11 -0500 Subject: [PATCH 10/13] test: use throws method instead of assert --- tests/Unit/TestSuite.php | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/tests/Unit/TestSuite.php b/tests/Unit/TestSuite.php index c26149c8..62cc724d 100644 --- a/tests/Unit/TestSuite.php +++ b/tests/Unit/TestSuite.php @@ -8,15 +8,17 @@ it('does not allow to add the same test description twice', function () { $testSuite = new TestSuite(getcwd(), 'tests'); $test = function () {}; $testSuite->tests->set(new \Pest\Factories\TestCaseFactory(__FILE__, 'foo', $test)); - $this->expectException(TestAlreadyExist::class); - $this->expectExceptionMessage(sprintf('A test with the description `%s` already exist in the filename `%s`.', 'foo', __FILE__)); $testSuite->tests->set(new \Pest\Factories\TestCaseFactory(__FILE__, 'foo', $test)); -}); +})->throws( + TestAlreadyExist::class, + sprintf('A test with the description `%s` already exist in the filename `%s`.', 'foo', __FILE__), +); it('alerts users about tests with arguments but no input', function () { $testSuite = new TestSuite(getcwd(), 'tests'); $test = function (int $arg) {}; - $this->expectException(DatasetMissing::class); - $this->expectExceptionMessage(sprintf("A test with the description '%s' has %d argument(s) ([%s]) and no dataset(s) provided in %s", 'foo', 1, 'int $arg', __FILE__)); $testSuite->tests->set(new \Pest\Factories\TestCaseFactory(__FILE__, 'foo', $test)); -}); +})->throws( + DatasetMissing::class, + sprintf("A test with the description '%s' has %d argument(s) ([%s]) and no dataset(s) provided in %s", 'foo', 1, 'int $arg', __FILE__), +); From dc75b34deb3b969831888abc4bf450fef6106c69 Mon Sep 17 00:00:00 2001 From: Jordan Brauer Date: Wed, 30 Jun 2021 09:51:49 -0500 Subject: [PATCH 11/13] refactor: move logic into boolean method in TestCaseFactory --- src/Factories/TestCaseFactory.php | 8 ++++++++ src/Repositories/TestRepository.php | 2 +- tests/.snapshots/success.txt | 2 +- 3 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/Factories/TestCaseFactory.php b/src/Factories/TestCaseFactory.php index 63eafaff..e849f95e 100644 --- a/src/Factories/TestCaseFactory.php +++ b/src/Factories/TestCaseFactory.php @@ -235,4 +235,12 @@ final class TestCaseFactory return $classFQN; } + + /** + * Determine if the test case will receive argument input from Pest, or not. + */ + public function receivesArguments(): bool + { + return $this->dependent === true || count($this->datasets) > 0; + } } diff --git a/src/Repositories/TestRepository.php b/src/Repositories/TestRepository.php index 305dc127..47684548 100644 --- a/src/Repositories/TestRepository.php +++ b/src/Repositories/TestRepository.php @@ -142,7 +142,7 @@ final class TestRepository throw new TestAlreadyExist($test->filename, $test->description); } - if ($test->dependent === false && count($test->datasets) === 0) { + if (!$test->receivesArguments()) { $arguments = Reflection::getFunctionArguments($test->test); if (count($arguments) > 0) { diff --git a/tests/.snapshots/success.txt b/tests/.snapshots/success.txt index 0a79e4e9..aa5966c7 100644 --- a/tests/.snapshots/success.txt +++ b/tests/.snapshots/success.txt @@ -580,5 +580,5 @@ ✓ it is a test ✓ it uses correct parent class - Tests: 4 incompleted, 7 skipped, 363 passed + Tests: 4 incompleted, 7 skipped, 364 passed \ No newline at end of file From 43920f79a902148325bf6da4573fdbea3bf1097d Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Tue, 27 Jul 2021 23:36:09 -0500 Subject: [PATCH 12/13] feat: add count method for checking message type presence --- src/Support/HigherOrderMessageCollection.php | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/src/Support/HigherOrderMessageCollection.php b/src/Support/HigherOrderMessageCollection.php index b107bdba..a6634685 100644 --- a/src/Support/HigherOrderMessageCollection.php +++ b/src/Support/HigherOrderMessageCollection.php @@ -53,4 +53,20 @@ final class HigherOrderMessageCollection $message->call($target); } } + + /** + * Count the number of messages with the given name. + * + * @param string $name A higher order message name (usually a method name) + */ + public function count(string $name): int + { + return array_reduce( + $this->messages, + static function (int $total, HigherOrderMessage $message) use ($name): int { + return $total + (int) ($name === $message->name); + }, + 0, + ); + } } From 8d24b4a217f9421257c08ddff86bd639fe516697 Mon Sep 17 00:00:00 2001 From: jordanbrauer <18744334+jordanbrauer@users.noreply.github.com> Date: Tue, 27 Jul 2021 23:39:26 -0500 Subject: [PATCH 13/13] chore: replace prop set/check with method call --- src/Factories/TestCaseFactory.php | 10 ++-------- src/PendingObjects/TestCall.php | 2 -- 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/src/Factories/TestCaseFactory.php b/src/Factories/TestCaseFactory.php index e849f95e..bc75f5c1 100644 --- a/src/Factories/TestCaseFactory.php +++ b/src/Factories/TestCaseFactory.php @@ -66,13 +66,6 @@ final class TestCaseFactory */ public $datasets = []; - /** - * Has the current test been marked dependent on others? - * - * @var bool - */ - public $dependent = false; - /** * The FQN of the test case class. * @@ -241,6 +234,7 @@ final class TestCaseFactory */ public function receivesArguments(): bool { - return $this->dependent === true || count($this->datasets) > 0; + return count($this->datasets) > 0 + || $this->factoryProxies->count('addDependencies') > 0; } } diff --git a/src/PendingObjects/TestCall.php b/src/PendingObjects/TestCall.php index d97f1ac2..be839bff 100644 --- a/src/PendingObjects/TestCall.php +++ b/src/PendingObjects/TestCall.php @@ -102,8 +102,6 @@ final class TestCall ->factoryProxies ->add(Backtrace::file(), Backtrace::line(), 'addDependencies', [$tests]); - $this->testCaseFactory->dependent = true; - return $this; }