Hashar has uploaded a new change for review. (
https://gerrit.wikimedia.org/r/356373 )
Change subject: test: be strict regarding globals
......................................................................
test: be strict regarding globals
PHPUnit is able to detect tests mangling globals (--strict-global-state)
Set beStrictAboutChangesToGlobalState in phpunit.xml. Since that only
act on tests, we still need WgConfTestCase::setGlobals().
[tests/WgConfTestCase.php]
When a global had a null value, we failed to unset it from the global
scope. Port the logic from MediaWikiTestCase so we keep track of null
globals and properly unset() them.
Enhance exception message when globals are left behind.
[tests/dbconfigTest.php]
MWReal.php set $wmfRealm and $wmfDatacenter which we have to save and
restore. $wmfDatacenter however is not set but the method left it set
to a null value.
Stop treating $wmfDatacenter has a global variable and thus stop
saving/restoring it.
Later we can port that suite to use WgConfTestCase but I have hit a wall
trying to do it.
[tests/noc-conf/highlightTest.php]
Make sure super global $_GET is unset after use.
Change-Id: Ib7a1e7cb0dfe7c9481e850a6a6fc89c91de18faf
---
M phpunit.xml
M tests/TestServices.php
M tests/WgConfTestCase.php
M tests/dbconfigTest.php
M tests/noc-conf/highlightTest.php
5 files changed, 54 insertions(+), 23 deletions(-)
git pull ssh://gerrit.wikimedia.org:29418/operations/mediawiki-config
refs/changes/73/356373/1
diff --git a/phpunit.xml b/phpunit.xml
index fe20413..243e355 100644
--- a/phpunit.xml
+++ b/phpunit.xml
@@ -10,6 +10,7 @@
colors="true"
verbose="true"
+ beStrictAboutChangesToGlobalState="true"
beStrictAboutOutputDuringTests="true"
beStrictAboutTestSize="true"
beStrictAboutTestsThatDoNotTestAnything="true"
diff --git a/tests/TestServices.php b/tests/TestServices.php
index f4423eb..f8e9569 100644
--- a/tests/TestServices.php
+++ b/tests/TestServices.php
@@ -9,8 +9,9 @@
#
#######################################################################
-$wmfDatacenter = $wmfMasterDatacenter = 'unittest';
+# FIXME variables should be set in WgConfTestCase::loadWgConf??
+$wmfDatacenter = $wmfMasterDatacenter = 'unittest';
$wmfAllServices = array();
$wmfAllServices['unittest'] = array(
diff --git a/tests/WgConfTestCase.php b/tests/WgConfTestCase.php
index c5e6b14..810f981 100644
--- a/tests/WgConfTestCase.php
+++ b/tests/WgConfTestCase.php
@@ -13,12 +13,18 @@
class WgConfTestCase extends PHPUnit_Framework_TestCase {
protected $globals = array();
+ protected $globalsToUnset = array();
protected function restoreGlobals() {
foreach ( $this->globals as $key => $value ) {
$GLOBALS[$key] = $value;
}
$this->globals = array();
+
+ foreach ( $this->globalsToUnset as $key ) {
+ unset( $GLOBALS[$key] );
+ }
+ $this->globalsToUnset = array();
}
protected function setGlobals( $pairs, $value = null ) {
@@ -27,16 +33,21 @@
}
foreach ( $pairs as $key => $value ) {
// only set value in $this->globals on first call
- if ( !array_key_exists( $key, $this->globals ) ) {
- if ( isset( $GLOBALS[$key] ) ) {
- // break any object references
- try {
- $this->globals[$key] =
unserialize( serialize( $GLOBALS[$key] ) );
- } catch ( \Exception $e ) {
- $this->globals[$key] =
$GLOBALS[$key];
- }
- } else {
- $this->globals[$key] = null;
+ if (
+ !array_key_exists( $key, $this->globals ) &&
+ !array_key_exists( $key, $this->globalsToUnset )
+ ) {
+ if ( !array_key_exists( $key, $GLOBALS ) ) {
+ $this->globalsToUnset[$key] = $key;
+ $GLOBALS[$key] = $value;
+ continue;
+ }
+
+ // break any object references
+ try {
+ $this->globals[$key] = unserialize(
serialize( $GLOBALS[$key] ) );
+ } catch ( \Exception $e ) {
+ $this->globals[$key] = $GLOBALS[$key];
}
}
$GLOBALS[$key] = $value;
@@ -55,8 +66,12 @@
* the only way to achieve that is when leaving the data provider scope.
*/
function __destruct() {
- if ( ! empty( $this->globals ) ) {
- throw new Exception( __CLASS__ . ': setGlobals() used
without restoreGlobals()' );
+ if ( ! empty( $this->globals ) or ! empty(
$this->globalsToUnset ) ) {
+ throw new Exception(
+ __CLASS__ . ": setGlobals() used without
restoreGlobals().\n" .
+ "Mangled globals:\n" . var_export(
$this->globals, true ) .
+ "Created globals:\n" . var_export(
$this->globalsToUnset, true )
+ );
}
}
@@ -86,7 +101,14 @@
'wgConf' => $wgConf,
) );
+ // Other InitialiseSettings.php globals that we set in
TestServices.php
+ $this->setGlobals( array(
+ 'wmfAllServices' => null,
+ 'wmfLocalServices' => null,
+ 'wmfMasterServices' => null,
+ ) );
require __DIR__ . '/TestServices.php';
+
require "{$wmfConfigDir}/InitialiseSettings.php";
$ret = $wgConf;
diff --git a/tests/dbconfigTest.php b/tests/dbconfigTest.php
index 7018074..3b1c27e 100644
--- a/tests/dbconfigTest.php
+++ b/tests/dbconfigTest.php
@@ -23,12 +23,12 @@
}
function loadDbFile( $realm, $datacenter, $masterdatacenter ) {
- global $wmfRealm, $wmfDatacenter, $wmfMasterDatacenter;
+ global $wmfRealm, $wmfDatacenter;
- list( $oldRealm, $oldDatacenter, $oldMasterDatacenter ) =
- array( $wmfRealm, $wmfDatacenter, $wmfMasterDatacenter
);
- list( $wmfRealm, $wmfDatacenter, $wmfMasterDatacenter ) =
- array( $realm, $datacenter, $masterdatacenter );
+ list( $oldRealm, $oldDatacenter ) =
+ array( $wmfRealm, $wmfDatacenter );
+ list( $wmfRealm, $wmfDatacenter ) =
+ array( $realm, $datacenter );
# "properly" load db.php in local context:
$wgDBname = 'testwiki';
@@ -38,10 +38,12 @@
define( 'DBO_DEFAULT', 16 );
}
+ // intentionally not marked as global
+ $wmfMasterDatacenter = $masterdatacenter;
include( getRealmSpecificFilename( __DIR__ .
'/../wmf-config/db.php' ) );
- list( $wmfRealm, $wmfDatacenter, $wmfMasterDatacenter ) =
- array( $oldRealm, $oldDatacenter, $oldMasterDatacenter
);
+ list( $wmfRealm, $wmfDatacenter ) =
+ array( $oldRealm, $oldDatacenter );
return $wgLBFactoryConf;
}
diff --git a/tests/noc-conf/highlightTest.php b/tests/noc-conf/highlightTest.php
index 8e9bd9c..0d3fffd 100644
--- a/tests/noc-conf/highlightTest.php
+++ b/tests/noc-conf/highlightTest.php
@@ -101,9 +101,14 @@
$_GET = array(
'file' => $q
);
- ob_start();
- require $this->nocConfDir . '/highlight.php';
- $out = ob_get_clean();
+ try {
+ ob_start();
+ require $this->nocConfDir . '/highlight.php';
+ $out = ob_get_clean();
+ } finally {
+ // make sure we never pollute the global namespace
+ unset( $_GET );
+ }
return $out;
}
}
--
To view, visit https://gerrit.wikimedia.org/r/356373
To unsubscribe, visit https://gerrit.wikimedia.org/r/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: Ib7a1e7cb0dfe7c9481e850a6a6fc89c91de18faf
Gerrit-PatchSet: 1
Gerrit-Project: operations/mediawiki-config
Gerrit-Branch: master
Gerrit-Owner: Hashar <has...@free.fr>
_______________________________________________
MediaWiki-commits mailing list
MediaWiki-commits@lists.wikimedia.org
https://lists.wikimedia.org/mailman/listinfo/mediawiki-commits
