From a14f08bf367335dc2d65bd0402b32152c13a19bc Mon Sep 17 00:00:00 2001 From: Steve Grunwell Date: Thu, 29 Oct 2020 11:39:25 -0400 Subject: [PATCH 1/3] Simplify the resetting of state properties in each trait Instead of relying on `@before` fixtures to reset the [private] property that is tracking what's changed in each trait, set the default value in the property definition. --- src/Constants.php | 32 ++++------ src/EnvironmentVariables.php | 18 ++---- src/GlobalVariables.php | 28 +++------ tests/FixtureTest.php | 118 +++++++++++++++++++++++++++++++++++ 4 files changed, 142 insertions(+), 54 deletions(-) create mode 100644 tests/FixtureTest.php diff --git a/src/Constants.php b/src/Constants.php index 4895087..2e697f3 100644 --- a/src/Constants.php +++ b/src/Constants.php @@ -14,20 +14,10 @@ trait Constants * * @var array[] */ - private $_constants; - - /** - * @before - * - * @return void - */ - protected function resetConstants() - { - $this->_constants = [ - 'created' => [], - 'updated' => [], - ]; - } + private $constants = [ + 'created' => [], + 'updated' => [], + ]; /** * @after @@ -36,7 +26,7 @@ protected function resetConstants() */ protected function restoreConstants() { - foreach ($this->_constants['updated'] as $name => $value) { + foreach ($this->constants['updated'] as $name => $value) { if (defined($name)) { Runkit::constant_redefine($name, $value); } else { @@ -44,7 +34,7 @@ protected function restoreConstants() } } - foreach ($this->_constants['created'] as $name) { + foreach ($this->constants['created'] as $name) { if (defined($name)) { Runkit::constant_remove($name); } @@ -68,8 +58,8 @@ protected function setConstant($name, $value = null) $this->requiresRunkit('setConstant() requires Runkit be available, skipping.'); if (defined($name)) { - if (! isset($this->_constants['updated'][$name])) { - $this->_constants['updated'][$name] = constant($name); + if (! isset($this->constants['updated'][$name])) { + $this->constants['updated'][$name] = constant($name); } try { @@ -82,7 +72,7 @@ protected function setConstant($name, $value = null) )); } } else { - $this->_constants['created'][] = $name; + $this->constants['created'][] = $name; define($name, $value); } @@ -104,8 +94,8 @@ protected function deleteConstant($name) $this->requiresRunkit('deleteConstant() requires Runkit be available, skipping.'); - if (! isset($this->_constants[$name])) { - $this->_constants['updated'][$name] = constant($name); + if (! isset($this->constants[$name])) { + $this->constants['updated'][$name] = constant($name); } Runkit::constant_remove($name); diff --git a/src/EnvironmentVariables.php b/src/EnvironmentVariables.php index 0102af4..fe443e5 100644 --- a/src/EnvironmentVariables.php +++ b/src/EnvironmentVariables.php @@ -9,17 +9,7 @@ trait EnvironmentVariables * * @var mixed[] */ - private $_environmentVariables; - - /** - * @before - * - * @return void - */ - protected function resetEnvironmentVariableRegistry() - { - $this->_environmentVariables = []; - } + private $environmentVariables = []; /** * @after @@ -28,7 +18,7 @@ protected function resetEnvironmentVariableRegistry() */ protected function restoreEnvironmentVariables() { - foreach ($this->_environmentVariables as $variable => $value) { + foreach ($this->environmentVariables as $variable => $value) { putenv(false === $value ? $variable : "${variable}=${value}"); } } @@ -46,8 +36,8 @@ protected function restoreEnvironmentVariables() */ protected function setEnvironmentVariable($variable, $value = null) { - if (! isset($this->_environmentVariables[$variable])) { - $this->_environmentVariables[$variable] = getenv($variable); + if (! isset($this->environmentVariables[$variable])) { + $this->environmentVariables[$variable] = getenv($variable); } putenv(null === $value ? $variable : "${variable}=${value}"); diff --git a/src/GlobalVariables.php b/src/GlobalVariables.php index 999d733..b703cc7 100644 --- a/src/GlobalVariables.php +++ b/src/GlobalVariables.php @@ -7,20 +7,10 @@ trait GlobalVariables /** * @var array[] */ - private $_globalVariables; - - /** - * @before - * - * @return void - */ - protected function resetGlobalVariables() - { - $this->_globalVariables = [ - 'created' => [], - 'updated' => [], - ]; - } + private $globalVariables = [ + 'created' => [], + 'updated' => [], + ]; /** * @after @@ -30,12 +20,12 @@ protected function resetGlobalVariables() protected function restoreGlobalVariables() { // Restore existing values. - foreach ($this->_globalVariables['updated'] as $var => $value) { + foreach ($this->globalVariables['updated'] as $var => $value) { $GLOBALS[$var] = $value; } // Remove anything that was freshly-defined. - foreach ($this->_globalVariables['created'] as $var) { + foreach ($this->globalVariables['created'] as $var) { unset($GLOBALS[$var]); } } @@ -52,9 +42,9 @@ protected function restoreGlobalVariables() protected function setGlobalVariable($variable, $value) { if (! isset($GLOBALS[$variable])) { - $this->_globalVariables['created'][] = $variable; - } elseif (! isset($this->_globalVariables['updated'][$variable])) { - $this->_globalVariables['updated'][$variable] = $GLOBALS[$variable]; + $this->globalVariables['created'][] = $variable; + } elseif (! isset($this->globalVariables['updated'][$variable])) { + $this->globalVariables['updated'][$variable] = $GLOBALS[$variable]; } if (null === $value) { diff --git a/tests/FixtureTest.php b/tests/FixtureTest.php new file mode 100644 index 0000000..7a44675 --- /dev/null +++ b/tests/FixtureTest.php @@ -0,0 +1,118 @@ +setConstant('FIXTURE_SETUP_CONSTANT', true); + $this->setEnvironmentVariable('FIXTURE_SETUP_ENV', 'abc'); + $this->setGlobalVariable('FIXTURE_SETUP_GLOBAL', true); + } + + /** + * @before + */ + protected function defineInitialValues() + { + $this->setConstant('FIXTURE_BEFORE_CONSTANT', true); + $this->setEnvironmentVariable('FIXTURE_BEFORE_ENV', 'xyz'); + $this->setGlobalVariable('FIXTURE_BEFORE_GLOBAL', true); + } + + /** + * @test + * @group Constants + */ + public function it_should_permit_constants_to_be_set_in_fixtures_method() + { + $this->assertTrue( + defined('FIXTURE_SETUP_CONSTANT'), + 'The constant should have been defined in the setUp() method.' + ); + $this->assertTrue( + defined('FIXTURE_BEFORE_CONSTANT'), + 'The constant should have been defined in the @before method.' + ); + + $this->restoreConstants(); + $this->assertFalse( + defined('FIXTURE_SETUP_CONSTANT'), + 'The constant should have been undefined by restoreConstants().' + ); + $this->assertFalse( + defined('FIXTURE_BEFORE_CONSTANT'), + 'The constant should have been undefined by restoreConstants().' + ); + } + + /** + * @test + * @group EnvironmentVariables + */ + public function it_should_permit_environment_variables_to_be_set_in_fixtures_method() + { + $this->assertSame( + 'abc', + getenv('FIXTURE_SETUP_ENV'), + 'The environment variable should have been defined in the setUp() method.' + ); + $this->assertSame( + 'xyz', + getenv('FIXTURE_BEFORE_ENV'), + 'The environment variable should have been defined in the @before method.' + ); + + $this->restoreEnvironmentVariables(); + $this->assertFalse( + getenv('FIXTURE_SETUP_ENV'), + 'The environment variable should have been undefined by restoreGlobalVariables().' + ); + $this->assertFalse( + getenv('FIXTURE_BEFORE_ENV'), + 'The environment variable should have been undefined by restoreGlobalVariables().' + ); + } + + /** + * @test + * @group GlobalVariables + */ + public function it_should_permit_global_variables_to_be_set_in_fixtures_method() + { + $this->assertTrue( + isset($GLOBALS['FIXTURE_SETUP_GLOBAL']), + 'The global variable should have been defined in the setUp() method.' + ); + $this->assertTrue( + isset($GLOBALS['FIXTURE_BEFORE_GLOBAL']), + 'The global variable should have been defined in the @before method.' + ); + + $this->restoreGlobalVariables(); + $this->assertFalse( + isset($GLOBALS['FIXTURE_SETUP_GLOBAL']), + 'The global variable should have been undefined by restoreGlobalVariables().' + ); + $this->assertFalse( + isset($GLOBALS['FIXTURE_BEFORE_GLOBAL']), + 'The global variable should have been undefined by restoreGlobalVariables().' + ); + } +} From d139bd0a825783f4c637042235458e989c57fabd Mon Sep 17 00:00:00 2001 From: Steve Grunwell Date: Thu, 29 Oct 2020 11:42:04 -0400 Subject: [PATCH 2/3] Clean up some risky test warnings on PHPUnit 5.x --- tests/Concerns/RunkitTest.php | 10 ++++++++-- tests/GlobalVariablesTest.php | 4 ++++ 2 files changed, 12 insertions(+), 2 deletions(-) diff --git a/tests/Concerns/RunkitTest.php b/tests/Concerns/RunkitTest.php index 92afc38..1620e58 100644 --- a/tests/Concerns/RunkitTest.php +++ b/tests/Concerns/RunkitTest.php @@ -56,8 +56,14 @@ public function it_should_skip_tests_that_require_runkit_if_it_is_unavailable() $method = new \ReflectionMethod($this->instance, 'requiresRunkit'); $method->setAccessible(true); - $this->expectException(SkippedTestError::class); + // Older versions of PHPUnit will actually try to mark this as skipped. + try { + $method->invoke($this->instance); + } catch (SkippedTestError $e) { + $this->assertInstanceOf(SkippedTestError::class, $e); + return; + } - $method->invoke($this->instance); + $this->fail('Did not catch the expected SkippedTestError.'); } } diff --git a/tests/GlobalVariablesTest.php b/tests/GlobalVariablesTest.php index d0779ae..3f2fb7b 100644 --- a/tests/GlobalVariablesTest.php +++ b/tests/GlobalVariablesTest.php @@ -9,6 +9,10 @@ */ class GlobalVariablesTest extends TestCase { + protected $backupGlobalsBlacklist = [ + 'setGlobalVariable', + ]; + /** * @test * @testdox setGlobalVariable() should be able to handle new global variables From d1a4e6e8f573ab095f7bf9bfea611b8c8ab01513 Mon Sep 17 00:00:00 2001 From: Steve Grunwell Date: Thu, 29 Oct 2020 11:51:13 -0400 Subject: [PATCH 3/3] Use the Symfony\Bridge\PhpUnit\SetUpTearDownTrait trait to avoid conflict around the "void" return type --- tests/FixtureTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/FixtureTest.php b/tests/FixtureTest.php index 7a44675..5e153b2 100644 --- a/tests/FixtureTest.php +++ b/tests/FixtureTest.php @@ -4,6 +4,7 @@ use AssertWell\PHPUnitGlobalState\Exceptions\RedefineException; use PHPUnit\Framework\SkippedTestError; +use Symfony\Bridge\PhpUnit\SetUpTearDownTrait; /** * Tests to ensure that state may be set in PHPUnit fixtures. @@ -12,12 +13,14 @@ */ class FixtureTest extends TestCase { + use SetUpTearDownTrait; + protected $backupGlobalsBlacklist = [ 'FIXTURE_BEFORE_GLOBAL', 'FIXTURE_SETUP_GLOBAL', ]; - public function setUp(): void + public function doSetUp() { parent::setUp();