r74780 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r74779‎ | r74780 | r74781 >
Date:16:38, 14 October 2010
Author:pdhanda
Status:ok (Comments)
Tags:
Comment:
* WebStart.php and SeleniumWebSettings.php allow include files and global config variables to be set based on the testsuite being run. See discussion in http://www.mediawiki.org/wiki/SeleniumFramework#Test_Wiki_configuration
* Let test suites run without logging in.
Modified paths:
  • /trunk/phase3/includes/SeleniumWebSettings.php (added) (history)
  • /trunk/phase3/includes/WebStart.php (modified) (history)
  • /trunk/phase3/maintenance/tests/RunSeleniumTests.php (modified) (history)
  • /trunk/phase3/maintenance/tests/selenium/SeleniumTestSuite.php (modified) (history)
  • /trunk/phase3/maintenance/tests/selenium/SimpleSeleniumTestCase.php (modified) (history)
  • /trunk/phase3/maintenance/tests/selenium/SimpleSeleniumTestSuite.php (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/tests/RunSeleniumTests.php
——@@ -120,6 +120,7 @@
121121 foreach ( $seleniumTestSuites as $testSuiteName => $testSuiteFile ) {
122122 require( $testSuiteFile );
123123 $suite = new $testSuiteName();
 124+ $suite->setName( $testSuiteName );
124125 $suite->addTests();
125126
126127 try {
Index: trunk/phase3/maintenance/tests/selenium/SeleniumTestSuite.php
——@@ -3,6 +3,7 @@
44 abstract class SeleniumTestSuite extends PHPUnit_Framework_TestSuite {
55 private $selenium;
66 private $isSetUp = false;
 7+ private $loginBeforeTests = true;
78
89 // Do not add line break after test output
910 const CONTINUE_LINE = 1;
——@@ -20,12 +21,14 @@
2122 $this->isSetUp = true;
2223 $this->selenium = Selenium::getInstance();
2324 $this->selenium->start();
24 - //$this->selenium->open( $this->selenium->getUrl() . '/index.php?setupTestSuite=' . $this->getName() );
25 - $this->login();
 25+ $this->selenium->open( $this->selenium->getUrl() . '/index.php?setupTestSuite=' . $this->getName() );
 26+ if ( $this->loginBeforeTests ) {
 27+ $this->login();
 28+ }
2629 }
2730
2831 public function tearDown() {
29 - //$this->selenium->open( $this->selenium->getUrl() . '/index.php?clearTestSuite=' . $this->getName() );
 32+ $this->selenium->open( $this->selenium->getUrl() . '/index.php?clearTestSuite=' . $this->getName() );
3033 $this->selenium->stop();
3134 }
3235
——@@ -36,4 +39,8 @@
3740 public function loadPage( $title, $action ) {
3841 $this->selenium->loadPage( $title, $action );
3942 }
 43+
 44+ protected function setLoginBeforeTests( $loginBeforeTests = true ) {
 45+ $this->loginBeforeTests = $loginBeforeTests;
 46+ }
4047 }
Index: trunk/phase3/maintenance/tests/selenium/SimpleSeleniumTestSuite.php
——@@ -1,7 +1,21 @@
22 <?php
3 -
 3+/*
 4+ * Sample test suite.
 5+ * Two ways to configure MW for these tests
 6+ * 1) If you are running multiple test suites, add the following in LocalSettings.php
 7+ * require_once("maintenance/tests/selenium/SimpleSeleniumConfig.php");
 8+ * $wgSeleniumTestConfigs['SimpleSeleniumTestSuite'] = 'SimpleSeleniumConfig::getSettings';
 9+ * OR
 10+ * 2) Add the following to your Localsettings.php
 11+ * require_once( "$IP/skins/Chick.php" );
 12+ * $wgDefaultSkin = 'chick';
 13+ */
414 class SimpleSeleniumTestSuite extends SeleniumTestSuite
515 {
 16+ public function setUp() {
 17+ $this->setLoginBeforeTests( false );
 18+ parent::setUp();
 19+ }
620 public function addTests() {
721 $testFiles = array(
822 'maintenance/tests/selenium/SimpleSeleniumTestCase.php'
Index: trunk/phase3/maintenance/tests/selenium/SimpleSeleniumTestCase.php
——@@ -1,9 +1,10 @@
22 <?php
3 -
4 -class SimpleSeleniumTestCase extends SeleniumTestCase
5 -{
6 - public function testBasic()
7 - {
 3+/*
 4+ * This test case is part of the SimpleSeleniumTestSuite.
 5+ * Configuration for these tests are dosumented as part of SimpleSeleniumTestSuite.php
 6+ */
 7+class SimpleSeleniumTestCase extends SeleniumTestCase {
 8+ public function testBasic() {
89 $this->open( $this->getUrl() .
910 '/index.php?title=Selenium&action=edit' );
1011 $this->type( "wpTextbox1", "This is a basic test" );
——@@ -14,19 +15,12 @@
1516 $source = $this->getText( "//div[@id='wikiPreview']/p" );
1617 $correct = strstr( $source, "This is a basic test" );
1718 $this->assertEquals( $correct, true );
18 -
1919 }
2020
21 - /*
22 - * Needs the following in your LocalConfig or an alternative method of configuration (coming soon)
23 - * require_once( "$IP/extensions/UsabilityInitiative/Vector/Vector.php" );
24 - * $wgDefaultSkin='vector';
25 - */
26 - public function testGlobalVariable1()
27 - {
 21+ public function testGlobalVariableForDefaultSkin() {
2822 $this->open( $this->getUrl() . '/index.php?&action=purge' );
2923 $bodyClass = $this->getAttribute( "//body/@class" );
30 - $this-> assertContains('skin-vector', $bodyClass, 'Vector skin not set');
 24+ $this-> assertContains('skin-chick', $bodyClass, 'Chick skin not set');
3125 }
3226
3327 }
Index: trunk/phase3/includes/SeleniumWebSettings.php
——@@ -0,0 +1,69 @@
 2+<?php
 3+/*
 4+ * Dynamically change configuration variables based on the test suite name and a cookie value.
 5+ * For details on how to configure a wiki for a Selenium test, see:
 6+ * http://www.mediawiki.org/wiki/SeleniumFramework#Test_Wiki_configuration
 7+ */
 8+if ( !$wgEnableSelenium ) {
 9+ return;
 10+}
 11+$cookiePrefix = $wgSitename . "-";
 12+$name = $cookiePrefix . "Selenium";
 13+
 14+//if we find a request parameter containing the test name, set a cookie with the test name
 15+if ( array_key_exists( 'setupTestSuite', $_GET) ) {
 16+ //TODO: do a check for valid testsuite names
 17+ $setupTestSuiteName = $_GET['setupTestSuite'];
 18+ if ( strlen( $setupTestSuiteName) > 0 ) {
 19+ $expire = time() + 600;
 20+ setcookie( $name,
 21+ $setupTestSuiteName,
 22+ $expire,
 23+ $wgCookiePath,
 24+ $wgCookieDomain,
 25+ $wgCookieSecure,
 26+ true );
 27+ }
 28+}
 29+//clear the cookie based on a request param
 30+if ( array_key_exists( 'clearTestSuite', $_GET) ) {
 31+ $expire = time() - 600;
 32+ setcookie( $name,
 33+ '',
 34+ $expire,
 35+ $wgCookiePath,
 36+ $wgCookieDomain,
 37+ $wgCookieSecure,
 38+ true );
 39+}
 40+
 41+//if a cookie is found, run the appropriate callback to get the config params.
 42+if ( array_key_exists( $name, $_COOKIE) ) {
 43+ $testSuiteName = $_COOKIE[$name];
 44+ $testIncludes = array(); //array containing all the includes needed for this test
 45+ $testGlobalConfigs = array(); //an array containg all the global configs needed for this test
 46+ if ( isset( $wgSeleniumTestConfigs ) && array_key_exists($testSuiteName, $wgSeleniumTestConfigs) ) {
 47+ $callback = $wgSeleniumTestConfigs[$testSuiteName];
 48+ call_user_func_array( $callback, array( &$testIncludes, &$testGlobalConfigs));
 49+ }
 50+
 51+ foreach ( $testIncludes as $includeFile ) {
 52+ $file = $IP . '/' . $includeFile;
 53+ require_once( $file );
 54+ }
 55+ foreach ( $testGlobalConfigs as $key => $value ) {
 56+ if ( is_array( $value ) ) {
 57+
 58+ $configArray = array();
 59+ if ( isset( $GLOBALS[$key] ) ) {
 60+ $configArray = $GLOBALS[$key];
 61+ }
 62+ foreach ( $value as $configKey => $configValue ) {
 63+ $configArray[$configKey] = $configValue;
 64+ }
 65+ $GLOBALS[$key] = $configArray;
 66+ } else {
 67+ $GLOBALS[$key] = $value;
 68+ }
 69+ }
 70+}
\ No newline at end of file
Index: trunk/phase3/includes/WebStart.php
——@@ -114,6 +114,11 @@
115115 # Include site settings. $IP may be changed (hopefully before the AutoLoader is invoked)
116116 require_once( "$IP/LocalSettings.php" );
117117 }
 118+
 119+if ( $wgEnableSelenium ) {
 120+ require_once( "$IP/includes/SeleniumWebSettings.php" );
 121+}
 122+
118123 wfProfileOut( 'WebStart.php-conf' );
119124
120125 wfProfileIn( 'WebStart.php-ob_start' );
——@@ -135,3 +140,4 @@
136141 if ( !defined( 'MW_NO_SETUP' ) ) {
137142 require_once( "$IP/includes/Setup.php" );
138143 }
 144+

Follow-up revisions

RevisionCommit summaryAuthorDate
r74784Added missing file for r74780pdhanda17:06, 14 October 2010
r74954Follow up to r74780. Set default for .pdhanda18:14, 18 October 2010
r74955Follow up to r74780. Set default for . did some sanity check around testSuite...pdhanda18:15, 18 October 2010

Comments

#Comment by Platonides (talk | contribs)   16:48, 14 October 2010
  • There's no maintenance/tests/selenium/SimpleSeleniumConfig.php
  • You don't need to require the skin file from local settings in order to enable a skin.
  • How does setting the default skin to chick configure the test??
  • SeleniumWebSettings.php is vulnerable to register_globals
#Comment by 😂 (talk | contribs)   16:50, 14 October 2010

I know this is included before WebRequest is setup, but using $_GET directly always makes me feel a little uneasy :(

#Comment by Pdhanda (talk | contribs)   18:02, 14 October 2010

Chatted briefly with ^demon who is dealing with similar issues for phpunit. There may be a simpler cleaner way to do this. Let's discuss tomorrow, October 15, 9:30am PDT on #mediawiki

#Comment by Pdhanda (talk | contribs)   17:15, 15 October 2010

I also ran this by Roan and mglasser and these are some of the things that need fixing:

  • Fix the "isset( $wgSeleniumTestConfigs )" register_globals vulnerability
  • Test name could be validated simply by checking if it's a key in $wgSeleniumTestConfigs. This should be done /before/ it's setcookie()'d (and again after being pulled out of $_COOKIE).
  • Restrict test suite names to [a-zA-Z0-9].
  • Replace the $configArray deal with $GLOBALS[$key] += $value;
#Comment by Pdhanda (talk | contribs)   18:31, 15 October 2010
  • The first few lines of the if ( array_key_exists( $name, $_COOKIE) ) block use spaces for indentation instead of tabs
  • Don't use array_key_exists( 'key', $arr ); , use isset( $arr['key'] )
  • Use if ( !defined( 'MEDIAWIKI' ) ) die();

Status & tagging log