From 106b279ed062ec1eafc55e3b9a04ab554bfab21d Mon Sep 17 00:00:00 2001 From: Nuno Maduro Date: Sat, 4 Dec 2021 21:18:55 +0000 Subject: [PATCH 1/4] feat: adds --retry option --- .temp/retry.json | 1 + composer.json | 3 +- overrides/Runner/TestSuiteLoader.php | 2 +- src/Bootstrappers/BootSubscribers.php | 2 + src/Factories/TestCaseFactory.php | 8 +- src/Factories/TestCaseMethodFactory.php | 7 +- src/Plugins/Concerns/HandleArguments.php | 37 +++++++++ src/Plugins/Retry.php | 30 +++++++ src/Plugins/Version.php | 4 +- src/Repositories/TempRepository.php | 78 +++++++++++++++++++ src/Repositories/TestRepository.php | 2 +- .../EnsureFailedTestsAreStoredForRetry.php | 23 ++++++ .../EnsureRetryRepositoryExists.php | 23 ++++++ src/TestSuite.php | 17 ++-- tests/Hooks/AfterAllTest.php | 44 ----------- tests/Hooks/BeforeAllTest.php | 54 ------------- tests/Unit/Plugins/Retry.php | 15 ++++ 17 files changed, 237 insertions(+), 113 deletions(-) create mode 100644 .temp/retry.json create mode 100644 src/Plugins/Concerns/HandleArguments.php create mode 100644 src/Plugins/Retry.php create mode 100644 src/Repositories/TempRepository.php create mode 100644 src/Subscribers/EnsureFailedTestsAreStoredForRetry.php create mode 100644 src/Subscribers/EnsureRetryRepositoryExists.php create mode 100644 tests/Unit/Plugins/Retry.php diff --git a/.temp/retry.json b/.temp/retry.json new file mode 100644 index 00000000..0637a088 --- /dev/null +++ b/.temp/retry.json @@ -0,0 +1 @@ +[] \ No newline at end of file diff --git a/composer.json b/composer.json index 4d50f7b0..893e8e41 100644 --- a/composer.json +++ b/composer.json @@ -82,7 +82,8 @@ "Pest\\Plugins\\Coverage", "Pest\\Plugins\\Init", "Pest\\Plugins\\Version", - "Pest\\Plugins\\Environment" + "Pest\\Plugins\\Environment", + "Pest\\Plugins\\Retry" ] }, "laravel": { diff --git a/overrides/Runner/TestSuiteLoader.php b/overrides/Runner/TestSuiteLoader.php index 19b37a82..92fb003b 100644 --- a/overrides/Runner/TestSuiteLoader.php +++ b/overrides/Runner/TestSuiteLoader.php @@ -85,7 +85,7 @@ final class TestSuiteLoader (static function () use ($suiteClassFile) { include_once $suiteClassFile; - TestSuite::getInstance()->tests->makeIfExists($suiteClassFile); + TestSuite::getInstance()->tests->makeIfNeeded($suiteClassFile); })(); $loadedClasses = array_values( diff --git a/src/Bootstrappers/BootSubscribers.php b/src/Bootstrappers/BootSubscribers.php index 12486f50..f78dee18 100644 --- a/src/Bootstrappers/BootSubscribers.php +++ b/src/Bootstrappers/BootSubscribers.php @@ -20,6 +20,8 @@ final class BootSubscribers private static array $subscribers = [ Subscribers\EnsureConfigurationIsValid::class, Subscribers\EnsureConfigurationDefaults::class, + Subscribers\EnsureRetryRepositoryExists::class, + Subscribers\EnsureFailedTestsAreStoredForRetry::class, ]; /** diff --git a/src/Factories/TestCaseFactory.php b/src/Factories/TestCaseFactory.php index b214631b..83f1fd9a 100644 --- a/src/Factories/TestCaseFactory.php +++ b/src/Factories/TestCaseFactory.php @@ -101,7 +101,7 @@ final class TestCaseFactory * * @param array $methods */ - public function evaluate(string $filename, array $methods): string + public function evaluate(string $filename, array $methods): void { if ('\\' === DIRECTORY_SEPARATOR) { // In case Windows, strtolower drive name, like in UsesCall. @@ -123,7 +123,7 @@ final class TestCaseFactory $classFQN = 'P\\' . $relativePath; if (class_exists($classFQN)) { - return $classFQN; + return; } $hasPrintableTestCaseClassFQN = sprintf('\%s', HasPrintableTestCaseName::class); @@ -142,7 +142,7 @@ final class TestCaseFactory } $methodsCode = implode('', array_map( - fn (TestCaseMethodFactory $methodFactory) => $methodFactory->buildForEvaluation(self::$annotations), + fn (TestCaseMethodFactory $methodFactory) => $methodFactory->buildForEvaluation($classFQN, self::$annotations), $methods )); @@ -164,8 +164,6 @@ final class TestCaseFactory } catch (ParseError $caught) { throw new RuntimeException(sprintf('Unable to create test case for test file at %s', $filename), 1, $caught); } - - return $classFQN; } /** diff --git a/src/Factories/TestCaseMethodFactory.php b/src/Factories/TestCaseMethodFactory.php index 1aaa2dae..50214370 100644 --- a/src/Factories/TestCaseMethodFactory.php +++ b/src/Factories/TestCaseMethodFactory.php @@ -8,6 +8,7 @@ use Closure; use Pest\Datasets; use Pest\Exceptions\ShouldNotHappen; use Pest\Factories\Concerns\HigherOrderable; +use Pest\Plugins\Retry; use Pest\Support\Str; use Pest\TestSuite; use PHPUnit\Framework\Assert; @@ -107,7 +108,7 @@ final class TestCaseMethodFactory * * @param array $annotationsToUse */ - public function buildForEvaluation(array $annotationsToUse): string + public function buildForEvaluation(string $classFQN, array $annotationsToUse): string { if ($this->description === null) { throw ShouldNotHappen::fromMessage('The test description may not be empty.'); @@ -115,6 +116,10 @@ final class TestCaseMethodFactory $methodName = Str::evaluable($this->description); + if (Retry::$retrying && !TestSuite::getInstance()->retryTempRepository->exists(sprintf('%s::%s', $classFQN, $methodName))) { + return ''; + } + $datasetsCode = ''; $annotations = ['@test']; diff --git a/src/Plugins/Concerns/HandleArguments.php b/src/Plugins/Concerns/HandleArguments.php new file mode 100644 index 00000000..de1de7db --- /dev/null +++ b/src/Plugins/Concerns/HandleArguments.php @@ -0,0 +1,37 @@ + $arguments + */ + public function hasArgument(string $argument, array $arguments): bool + { + return in_array($argument, $arguments, true); + } + + /** + * Pops the given argument from the arguments. + * + * @param array $arguments + * + * @return array + */ + public function popArgument(string $argument, array $arguments): array + { + $arguments = array_flip($arguments); + + unset($arguments[$argument]); + + return array_flip($arguments); + } +} diff --git a/src/Plugins/Retry.php b/src/Plugins/Retry.php new file mode 100644 index 00000000..f961018e --- /dev/null +++ b/src/Plugins/Retry.php @@ -0,0 +1,30 @@ +hasArgument('--retry', $arguments); + + return $this->popArgument('--retry', $arguments); + } +} diff --git a/src/Plugins/Version.php b/src/Plugins/Version.php index f86a6cd1..a6383ecd 100644 --- a/src/Plugins/Version.php +++ b/src/Plugins/Version.php @@ -13,6 +13,8 @@ use Symfony\Component\Console\Output\OutputInterface; */ final class Version implements HandlesArguments { + use Concerns\HandleArguments; + /** * Creates a new instance of the plugin. */ @@ -24,7 +26,7 @@ final class Version implements HandlesArguments public function handleArguments(array $arguments): array { - if (in_array('--version', $arguments, true)) { + if ($this->hasArgument('--version', $arguments)) { $this->output->writeln( sprintf('Pest %s', version()), ); diff --git a/src/Repositories/TempRepository.php b/src/Repositories/TempRepository.php new file mode 100644 index 00000000..a8e21bb4 --- /dev/null +++ b/src/Repositories/TempRepository.php @@ -0,0 +1,78 @@ +save(array_merge( + $this->all(), + [$element] + )); + } + + /** + * Clears the existing file, if any, and re-creates it. + */ + public function boot(): void + { + @unlink(self::FOLDER . '/' . $this->filename . '.json'); // @phpstan-ignore-line + + $this->save([]); + } + + /** + * Checks if the given element exists. + */ + public function exists(string $element): bool + { + return in_array($element, $this->all(), true); + } + + /** + * Gets all elements. + * + * @return array + */ + private function all(): array + { + $contents = file_get_contents(self::FOLDER . '/' . $this->filename . '.json'); + + assert(is_string($contents)); + + $all = json_decode($contents, true); + + return is_array($all) ? $all : []; + } + + /** + * Save the given elements. + * + * @param array $elements + */ + private function save(array $elements): void + { + $contents = json_encode($elements); + + file_put_contents(self::FOLDER . '/' . $this->filename . '.json', $contents); + } +} diff --git a/src/Repositories/TestRepository.php b/src/Repositories/TestRepository.php index 869d1000..94ffd17a 100644 --- a/src/Repositories/TestRepository.php +++ b/src/Repositories/TestRepository.php @@ -100,7 +100,7 @@ final class TestRepository /** * Makes a Test Case from the given filename, if exists. */ - public function makeIfExists(string $filename): void + public function makeIfNeeded(string $filename): void { if (array_key_exists($filename, $this->testCases)) { $this->make($this->testCases[$filename]); diff --git a/src/Subscribers/EnsureFailedTestsAreStoredForRetry.php b/src/Subscribers/EnsureFailedTestsAreStoredForRetry.php new file mode 100644 index 00000000..46fce87a --- /dev/null +++ b/src/Subscribers/EnsureFailedTestsAreStoredForRetry.php @@ -0,0 +1,23 @@ +retryTempRepository->add($event->test()->id()); + } +} diff --git a/src/Subscribers/EnsureRetryRepositoryExists.php b/src/Subscribers/EnsureRetryRepositoryExists.php new file mode 100644 index 00000000..dc988d1d --- /dev/null +++ b/src/Subscribers/EnsureRetryRepositoryExists.php @@ -0,0 +1,23 @@ +retryTempRepository->boot(); + } +} diff --git a/src/TestSuite.php b/src/TestSuite.php index 0df60394..2d40cedd 100644 --- a/src/TestSuite.php +++ b/src/TestSuite.php @@ -9,6 +9,7 @@ use Pest\Repositories\AfterAllRepository; use Pest\Repositories\AfterEachRepository; use Pest\Repositories\BeforeAllRepository; use Pest\Repositories\BeforeEachRepository; +use Pest\Repositories\TempRepository; use Pest\Repositories\TestRepository; use PHPUnit\Framework\TestCase; @@ -47,6 +48,11 @@ final class TestSuite */ public AfterAllRepository $afterAll; + /** + * Holds the retry temp repository. + */ + public TempRepository $retryTempRepository; + /** * Holds the root path. */ @@ -64,11 +70,12 @@ final class TestSuite string $rootPath, public string $testPath) { - $this->beforeAll = new BeforeAllRepository(); - $this->beforeEach = new BeforeEachRepository(); - $this->tests = new TestRepository(); - $this->afterEach = new AfterEachRepository(); - $this->afterAll = new AfterAllRepository(); + $this->beforeAll = new BeforeAllRepository(); + $this->beforeEach = new BeforeEachRepository(); + $this->tests = new TestRepository(); + $this->afterEach = new AfterEachRepository(); + $this->afterAll = new AfterAllRepository(); + $this->retryTempRepository = new TempRepository('retry'); $this->rootPath = (string) realpath($rootPath); } diff --git a/tests/Hooks/AfterAllTest.php b/tests/Hooks/AfterAllTest.php index fb0726a7..b3d9bbc7 100644 --- a/tests/Hooks/AfterAllTest.php +++ b/tests/Hooks/AfterAllTest.php @@ -1,45 +1 @@ afterAll(function () { - expect($_SERVER['globalHook']) - ->toHaveProperty('afterAll') - ->and($_SERVER['globalHook']->afterAll) - ->toBe(0) - ->and($_SERVER['globalHook']->calls) - ->afterAll - ->toBe(1); - - $_SERVER['globalHook']->afterAll = 1; - $_SERVER['globalHook']->calls->afterAll++; -}); - -afterAll(function () { - expect($_SERVER['globalHook']) - ->toHaveProperty('afterAll') - ->and($_SERVER['globalHook']->afterAll) - ->toBe(1) - ->and($_SERVER['globalHook']->calls) - ->afterAll - ->toBe(2); - - $_SERVER['globalHook']->afterAll = 2; - $_SERVER['globalHook']->calls->afterAll++; -}); - -test('global afterAll execution order', function () { - expect($_SERVER['globalHook']) - ->not() - ->toHaveProperty('afterAll') - ->and($_SERVER['globalHook']->calls) - ->afterAll - ->toBe(0); -}); - -it('only gets called once per file', function () { - expect($_SERVER['globalHook']) - ->not() - ->toHaveProperty('afterAll') - ->and($_SERVER['globalHook']->calls) - ->afterAll - ->toBe(0); -}); diff --git a/tests/Hooks/BeforeAllTest.php b/tests/Hooks/BeforeAllTest.php index 4cea5faa..e69de29b 100644 --- a/tests/Hooks/BeforeAllTest.php +++ b/tests/Hooks/BeforeAllTest.php @@ -1,54 +0,0 @@ -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])) || ($_SERVER['PEST_PARALLEL'] ?? false); -$offset = $single ? 0 : 2; - -uses()->beforeAll(function () use ($offset) { - expect($_SERVER['globalHook']) - ->toHaveProperty('beforeAll') - ->and($_SERVER['globalHook']->beforeAll) - ->toBe(0) - ->and($_SERVER['globalHook']->calls) - ->beforeAll - ->toBe(1 + $offset); - - $_SERVER['globalHook']->beforeAll = 1; - $_SERVER['globalHook']->calls->beforeAll++; -}); - -beforeAll(function () use ($offset) { - expect($_SERVER['globalHook']) - ->toHaveProperty('beforeAll') - ->and($_SERVER['globalHook']->beforeAll) - ->toBe(1) - ->and($_SERVER['globalHook']->calls) - ->beforeAll - ->toBe(2 + $offset); - - $_SERVER['globalHook']->beforeAll = 2; - $_SERVER['globalHook']->calls->beforeAll++; -}); - -test('global beforeAll execution order', function () use ($offset) { - expect($_SERVER['globalHook']) - ->toHaveProperty('beforeAll') - ->and($_SERVER['globalHook']->beforeAll) - ->toBe(2) - ->and($_SERVER['globalHook']->calls) - ->beforeAll - ->toBe(3 + $offset); -}); - -it('only gets called once per file', function () use ($offset) { - expect($_SERVER['globalHook']) - ->beforeAll - ->toBe(2) - ->and($_SERVER['globalHook']->calls) - ->beforeAll - ->toBe(3 + $offset); -}); diff --git a/tests/Unit/Plugins/Retry.php b/tests/Unit/Plugins/Retry.php new file mode 100644 index 00000000..e0033821 --- /dev/null +++ b/tests/Unit/Plugins/Retry.php @@ -0,0 +1,15 @@ + Retry::$retrying = false); + +afterEach(fn () => Retry::$retrying = false); + +it('retries if --retry argument is used', function () { + $retry = new Retry(); + + $retry->handleArguments(['--retry']); + + expect(Retry::$retrying)->toBeTrue(); +}); From 0146186ddb0e517c70fa10ff0d9a2ad607e83920 Mon Sep 17 00:00:00 2001 From: Nuno Maduro Date: Sat, 4 Dec 2021 21:20:28 +0000 Subject: [PATCH 2/4] fix: adds retry.json to gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 7b80f949..394fb0b3 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,7 @@ coverage.xml /.php-cs-fixer.php .php-cs-fixer.cache .temp/coverage.php +.temp/retry.json *.swp *.swo .vscode/ From 5aeda553a418d487cabc4f8eaf17491490ab740d Mon Sep 17 00:00:00 2001 From: Nuno Maduro Date: Sat, 4 Dec 2021 21:21:47 +0000 Subject: [PATCH 3/4] fix: removes json file --- .temp/retry.json | 1 - 1 file changed, 1 deletion(-) delete mode 100644 .temp/retry.json diff --git a/.temp/retry.json b/.temp/retry.json deleted file mode 100644 index 0637a088..00000000 --- a/.temp/retry.json +++ /dev/null @@ -1 +0,0 @@ -[] \ No newline at end of file From 129a73388878b6ed16cb2df05104974bcabffbf2 Mon Sep 17 00:00:00 2001 From: Nuno Maduro Date: Sat, 4 Dec 2021 21:23:25 +0000 Subject: [PATCH 4/4] chore: re-adds removed tests --- tests/Hooks/AfterAllTest.php | 44 ++++++++++++++++++++++++++++ tests/Hooks/BeforeAllTest.php | 54 +++++++++++++++++++++++++++++++++++ 2 files changed, 98 insertions(+) diff --git a/tests/Hooks/AfterAllTest.php b/tests/Hooks/AfterAllTest.php index b3d9bbc7..fb0726a7 100644 --- a/tests/Hooks/AfterAllTest.php +++ b/tests/Hooks/AfterAllTest.php @@ -1 +1,45 @@ afterAll(function () { + expect($_SERVER['globalHook']) + ->toHaveProperty('afterAll') + ->and($_SERVER['globalHook']->afterAll) + ->toBe(0) + ->and($_SERVER['globalHook']->calls) + ->afterAll + ->toBe(1); + + $_SERVER['globalHook']->afterAll = 1; + $_SERVER['globalHook']->calls->afterAll++; +}); + +afterAll(function () { + expect($_SERVER['globalHook']) + ->toHaveProperty('afterAll') + ->and($_SERVER['globalHook']->afterAll) + ->toBe(1) + ->and($_SERVER['globalHook']->calls) + ->afterAll + ->toBe(2); + + $_SERVER['globalHook']->afterAll = 2; + $_SERVER['globalHook']->calls->afterAll++; +}); + +test('global afterAll execution order', function () { + expect($_SERVER['globalHook']) + ->not() + ->toHaveProperty('afterAll') + ->and($_SERVER['globalHook']->calls) + ->afterAll + ->toBe(0); +}); + +it('only gets called once per file', function () { + expect($_SERVER['globalHook']) + ->not() + ->toHaveProperty('afterAll') + ->and($_SERVER['globalHook']->calls) + ->afterAll + ->toBe(0); +}); diff --git a/tests/Hooks/BeforeAllTest.php b/tests/Hooks/BeforeAllTest.php index e69de29b..4cea5faa 100644 --- a/tests/Hooks/BeforeAllTest.php +++ b/tests/Hooks/BeforeAllTest.php @@ -0,0 +1,54 @@ +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])) || ($_SERVER['PEST_PARALLEL'] ?? false); +$offset = $single ? 0 : 2; + +uses()->beforeAll(function () use ($offset) { + expect($_SERVER['globalHook']) + ->toHaveProperty('beforeAll') + ->and($_SERVER['globalHook']->beforeAll) + ->toBe(0) + ->and($_SERVER['globalHook']->calls) + ->beforeAll + ->toBe(1 + $offset); + + $_SERVER['globalHook']->beforeAll = 1; + $_SERVER['globalHook']->calls->beforeAll++; +}); + +beforeAll(function () use ($offset) { + expect($_SERVER['globalHook']) + ->toHaveProperty('beforeAll') + ->and($_SERVER['globalHook']->beforeAll) + ->toBe(1) + ->and($_SERVER['globalHook']->calls) + ->beforeAll + ->toBe(2 + $offset); + + $_SERVER['globalHook']->beforeAll = 2; + $_SERVER['globalHook']->calls->beforeAll++; +}); + +test('global beforeAll execution order', function () use ($offset) { + expect($_SERVER['globalHook']) + ->toHaveProperty('beforeAll') + ->and($_SERVER['globalHook']->beforeAll) + ->toBe(2) + ->and($_SERVER['globalHook']->calls) + ->beforeAll + ->toBe(3 + $offset); +}); + +it('only gets called once per file', function () use ($offset) { + expect($_SERVER['globalHook']) + ->beforeAll + ->toBe(2) + ->and($_SERVER['globalHook']->calls) + ->beforeAll + ->toBe(3 + $offset); +});