r96565 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r96564‎ | r96565 | r96566 >
Date:14:04, 8 September 2011
Author:demon
Status:ok
Tags:
Comment:
Gut anything from HttpTest that involves making external requests and setting cookies - the way we do this suite needs to be completely rethought.
Chalk this up as an example of how *not* to write unit tests.
Still have 5 failures :(
Modified paths:
  • /trunk/phase3/tests/phpunit/includes/HttpTest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/tests/phpunit/includes/HttpTest.php
——@@ -1,364 +1,12 @@
22 <?php
3 -
4 -class MockCookie extends Cookie {
5 - public function canServeDomain( $arg ) { return parent::canServeDomain( $arg ); }
6 - public function canServePath( $arg ) { return parent::canServePath( $arg ); }
7 - public function isUnExpired() { return parent::isUnExpired(); }
8 -}
9 -
103 /**
114 * @group Broken
125 */
136 class HttpTest extends MediaWikiTestCase {
14 - static $content;
15 - static $headers;
16 - static $has_curl;
17 - static $has_fopen;
18 - static $has_proxy = false;
19 - static $proxy = "http://hulk:8080/";
20 - var $test_geturl = array(
21 - "http://en.wikipedia.org/robots.txt",
22 - "https://secure.wikimedia.org/",
23 - "http://pecl.php.net/feeds/pkg_apc.rss",
24 - "http://meta.wikimedia.org/w/index.php?title=Interwiki_map&action=raw",
25 - "http://www.mediawiki.org/w/api.php?action=query&list=categorymembers&cmtitle=Category:MediaWiki_hooks&format=php",
26 - );
27 - var $test_requesturl = array( "http://en.wikipedia.org/wiki/Special:Export/User:MarkAHershberger" );
28 -
29 - var $test_posturl = array( "http://www.comp.leeds.ac.uk/cgi-bin/Perl/environment-example" => "review=test" );
30 -
31 - function setUp() {
32 - putenv( "http_proxy" ); /* Remove any proxy env var, so curl doesn't get confused */
33 - if ( is_array( self::$content ) ) {
34 - return;
35 - }
36 - self::$has_curl = function_exists( 'curl_init' );
37 - self::$has_fopen = wfIniGetBool( 'allow_url_fopen' );
38 -
39 - if ( !file_exists( "/usr/bin/curl" ) ) {
40 - $this->markTestIncomplete( "This test requires the curl binary at /usr/bin/curl. If you have curl, please file a bug on this test, or, better yet, provide a patch." );
41 - }
42 -
43 - $content = tempnam( wfTempDir(), "" );
44 - $headers = tempnam( wfTempDir(), "" );
45 - if ( !$content && !$headers ) {
46 - die( "Couldn't create temp file!" );
47 - }
48 -
49 - // This probably isn't the best test for a proxy, but it works on my system!
50 - system( "curl -0 -o $content -s " . self::$proxy );
51 - $out = file_get_contents( $content );
52 - if ( $out ) {
53 - self::$has_proxy = true;
54 - }
55 -
56 - /* Maybe use wget instead of curl here ... just to use a different codebase? */
57 - foreach ( $this->test_geturl as $u ) {
58 - system( "curl -0 -s -D $headers '$u' -o $content" );
59 - self::$content["GET $u"] = file_get_contents( $content );
60 - self::$headers["GET $u"] = file_get_contents( $headers );
61 - }
62 - foreach ( $this->test_requesturl as $u ) {
63 - system( "curl -0 -s -X POST -H 'Content-Length: 0' -D $headers '$u' -o $content" );
64 - self::$content["POST $u"] = file_get_contents( $content );
65 - self::$headers["POST $u"] = file_get_contents( $headers );
66 - }
67 - foreach ( $this->test_posturl as $u => $postData ) {
68 - system( "curl -0 -s -X POST -d '$postData' -D $headers '$u' -o $content" );
69 - self::$content["POST $u => $postData"] = file_get_contents( $content );
70 - self::$headers["POST $u => $postData"] = file_get_contents( $headers );
71 - }
72 - unlink( $content );
73 - unlink( $headers );
74 - }
75 -
76 -
77 - function testInstantiation() {
78 - Http::$httpEngine = false;
79 -
80 - $r = MWHttpRequest::factory( "http://www.example.com/" );
81 - if ( self::$has_curl ) {
82 - $this->assertThat( $r, $this->isInstanceOf( 'CurlHttpRequest' ) );
83 - } else {
84 - $this->assertThat( $r, $this->isInstanceOf( 'PhpHttpRequest' ) );
85 - }
86 - $this->assertTrue( $r->status->isGood(), '$r->status is good' );
87 - unset( $r );
88 -
89 - $r = MWHttpRequest::factory( "//www.example.com/" );
90 - if ( self::$has_curl ) {
91 - $this->assertThat( $r, $this->isInstanceOf( 'CurlHttpRequest' ) );
92 - } else {
93 - $this->assertThat( $r, $this->isInstanceOf( 'PhpHttpRequest' ) );
94 - }
95 - $this->assertTrue( $r->status->isGood(), '$r->status is good' );
96 -
97 - if ( !self::$has_fopen ) {
98 - $this->setExpectedException( 'MWException' );
99 - }
100 - Http::$httpEngine = 'php';
101 - $r = MWHttpRequest::factory( "http://www.example.com/" );
102 - $this->assertThat( $r, $this->isInstanceOf( 'PhpHttpRequest' ) );
103 - $this->assertTrue( $r->status->isGood(), '$r->status is good' );
104 - unset( $r );
105 -
106 - if ( !self::$has_curl ) {
107 - $this->setExpectedException( 'MWException' );
108 - }
109 - Http::$httpEngine = 'curl';
110 - $r = MWHttpRequest::factory( "http://www.example.com/" );
111 - if ( self::$has_curl ) {
112 - $this->assertThat( $r, $this->isInstanceOf( 'CurlHttpRequest' ) );
113 - $this->assertTrue( $r->status->isGood(), '$r->status is good' );
114 - }
115 - }
116 -
117 - function runHTTPFailureChecks() {
118 - // Each of the following requests should result in a failure.
119 -
120 - $timeout = 1;
121 - $start_time = time();
122 - $r = Http::get( "http://www.example.com:1/", $timeout );
123 - $end_time = time();
124 - $this->assertLessThan( $timeout + 2, $end_time - $start_time,
125 - "Request took less than {$timeout}s via " . Http::$httpEngine );
126 - $this->assertEquals( $r, false, "false -- what we get on error from Http::get()" );
127 -
128 - $r = Http::get( "http://www.mediawiki.org/xml/made-up-url", $timeout );
129 - $this->assertFalse( $r, "False on 404s" );
130 -
131 -
132 - $r = MWHttpRequest::factory( "http://www.mediawiki.org/xml/made-up-url" );
133 - $er = $r->execute();
134 - if ( $r instanceof PhpHttpRequest && version_compare( '5.2.10', phpversion(), '>' ) ) {
135 - $this->assertRegexp( "/HTTP request failed/", $er->getWikiText() );
136 - } else {
137 - $this->assertRegexp( "/404 Not Found/", $er->getWikiText() );
138 - }
139 - }
140 -
141 - function testFailureDefault() {
142 - Http::$httpEngine = false;
143 - $this->runHTTPFailureChecks();
144 - }
145 -
146 - function testFailurePhp() {
147 - if ( !self::$has_fopen ) {
148 - $this->markTestIncomplete( "This test requires allow_url_fopen=true." );
149 - }
150 -
151 - Http::$httpEngine = "php";
152 - $this->runHTTPFailureChecks();
153 - }
154 -
155 - function testFailureCurl() {
156 - if ( !self::$has_curl ) {
157 - $this->markTestIncomplete( "This test requires curl." );
158 - }
159 -
160 - Http::$httpEngine = "curl";
161 - $this->runHTTPFailureChecks();
162 - }
163 -
164 - /* ./phase3/includes/Import.php:1108: $data = Http::request( $method, $url ); */
165 - /* ./includes/Import.php:1124: $link = Title::newFromText( "$interwiki:Special:Export/$page" ); */
166 - /* ./includes/Import.php:1134: return ImportStreamSource::newFromURL( $url, "POST" ); */
167 - function runHTTPRequests( $proxy = null ) {
168 - $opt = array();
169 -
170 - if ( $proxy ) {
171 - $opt['proxy'] = $proxy;
172 - } elseif ( $proxy === false ) {
173 - $opt['noProxy'] = true;
174 - }
175 -
176 - /* no postData here because the only request I could find in code so far didn't have any */
177 - foreach ( $this->test_requesturl as $u ) {
178 - $r = Http::request( "POST", $u, $opt );
179 - $this->assertEquals( self::$content["POST $u"], "$r", "POST $u with " . Http::$httpEngine );
180 -
181 - // If this was an HTTP URL, repeat the same test with a protocol-relative URL
182 - // We can't do this with HTTPS URLs because MWHttpRequest expands protocol-relative
183 - // URLs to HTTP.
184 - list( $prot, $url ) = explode( ':', $u, 2 );
185 - if ( $prot === 'http' ) {
186 - $r = Http::request( "POST", $url, $opt );
187 - $this->assertEquals( self::$content["POST $u"], "$r", "POST $url with " . Http::$httpEngine );
188 - }
189 - }
190 - }
191 -
192 - function testRequestDefault() {
193 - Http::$httpEngine = false;
194 - $this->runHTTPRequests();
195 - }
196 -
197 - function testRequestPhp() {
198 - if ( !self::$has_fopen ) {
199 - $this->markTestIncomplete( "This test requires allow_url_fopen=true." );
200 - }
201 -
202 - Http::$httpEngine = "php";
203 - $this->runHTTPRequests();
204 - }
205 -
206 - function testRequestCurl() {
207 - if ( !self::$has_curl ) {
208 - $this->markTestIncomplete( "This test requires curl." );
209 - }
210 -
211 - Http::$httpEngine = "curl";
212 - $this->runHTTPRequests();
213 - }
214 -
215 - function runHTTPGets( $proxy = null ) {
216 - $opt = array();
217 -
218 - if ( $proxy ) {
219 - $opt['proxy'] = $proxy;
220 - } elseif ( $proxy === false ) {
221 - $opt['noProxy'] = true;
222 - }
223 -
224 - foreach ( $this->test_geturl as $u ) {
225 - $r = Http::get( $u, 30, $opt ); /* timeout of 30s */
226 - $this->assertEquals( self::$content["GET $u"], "$r", "Get $u with " . Http::$httpEngine );
227 -
228 - // If this was an HTTP URL, repeat the same test with a protocol-relative URL
229 - // We can't do this with HTTPS URLs because MWHttpRequest expands protocol-relative
230 - // URLs to HTTP.
231 - list( $prot, $url ) = explode( ':', $u, 2 );
232 - if ( $prot === 'http' ) {
233 - $r = Http::get( $url, 30, $opt );
234 - $this->assertEquals( self::$content["GET $u"], "$r", "Get $url with " . Http::$httpEngine );
235 - }
236 - }
237 - }
238 -
239 - function testGetDefault() {
240 - Http::$httpEngine = false;
241 - $this->runHTTPGets();
242 - }
243 -
244 - function testGetPhp() {
245 - if ( !self::$has_fopen ) {
246 - $this->markTestIncomplete( "This test requires allow_url_fopen=true." );
247 - }
248 -
249 - Http::$httpEngine = "php";
250 - $this->runHTTPGets();
251 - }
252 -
253 - function testGetCurl() {
254 - if ( !self::$has_curl ) {
255 - $this->markTestIncomplete( "This test requires curl." );
256 - }
257 -
258 - Http::$httpEngine = "curl";
259 - $this->runHTTPGets();
260 - }
261 -
262 - function runHTTPPosts( $proxy = null ) {
263 - $opt = array();
264 -
265 - if ( $proxy ) {
266 - $opt['proxy'] = $proxy;
267 - } elseif ( $proxy === false ) {
268 - $opt['noProxy'] = true;
269 - }
270 -
271 - foreach ( $this->test_posturl as $u => $postData ) {
272 - $opt['postData'] = $postData;
273 - $r = Http::post( $u, $opt );
274 - $this->assertEquals( self::$content["POST $u => $postData"], "$r",
275 - "POST $u (postData=$postData) with " . Http::$httpEngine );
276 -
277 - // If this was an HTTP URL, repeat the same test with a protocol-relative URL
278 - // We can't do this with HTTPS URLs because MWHttpRequest expands protocol-relative
279 - // URLs to HTTP.
280 - list( $prot, $url ) = explode( ':', $u, 2 );
281 - if ( $prot === 'http' ) {
282 - $r = Http::post( $url, $opt );
283 - $this->assertEquals( self::$content["POST $u => $postData"], "$r",
284 - "POST $u (postData=$postData) with " . Http::$httpEngine );
285 - }
286 - }
287 - }
288 -
289 - function testPostDefault() {
290 - Http::$httpEngine = false;
291 - $this->runHTTPPosts();
292 - }
293 -
294 - function testPostPhp() {
295 - if ( !self::$has_fopen ) {
296 - $this->markTestIncomplete( "This test requires allow_url_fopen=true." );
297 - }
298 -
299 - Http::$httpEngine = "php";
300 - $this->runHTTPPosts();
301 - }
302 -
303 - function testPostCurl() {
304 - if ( !self::$has_curl ) {
305 - $this->markTestIncomplete( "This test requires curl." );
306 - }
307 -
308 - Http::$httpEngine = "curl";
309 - $this->runHTTPPosts();
310 - }
311 -
312 - function runProxyRequests() {
313 - if ( !self::$has_proxy ) {
314 - $this->markTestIncomplete( "This test requires a proxy." );
315 - }
316 - $this->runHTTPGets( self::$proxy );
317 - $this->runHTTPPosts( self::$proxy );
318 - $this->runHTTPRequests( self::$proxy );
319 -
320 - // Set false here to do noProxy
321 - $this->runHTTPGets( false );
322 - $this->runHTTPPosts( false );
323 - $this->runHTTPRequests( false );
324 - }
325 -
326 - function testProxyDefault() {
327 - Http::$httpEngine = false;
328 - $this->runProxyRequests();
329 - }
330 -
331 - function testProxyPhp() {
332 - if ( !self::$has_fopen ) {
333 - $this->markTestIncomplete( "This test requires allow_url_fopen=true." );
334 - }
335 -
336 - Http::$httpEngine = 'php';
337 - $this->runProxyRequests();
338 - }
339 -
340 - function testProxyCurl() {
341 - if ( !self::$has_curl ) {
342 - $this->markTestIncomplete( "This test requires curl." );
343 - }
344 -
345 - Http::$httpEngine = 'curl';
346 - $this->runProxyRequests();
347 - }
348 -
349 - function testIsLocalUrl() {
350 - }
351 -
352 - /* ./extensions/DonationInterface/payflowpro_gateway/payflowpro_gateway.body.php:559: $user_agent = Http::userAgent(); */
353 - function testUserAgent() {
354 - }
355 -
356 - function testIsValidUrl() {
357 - }
358 -
3597 /**
3608 * @dataProvider cookieDomains
3619 */
362 - function testValidateCookieDomain( $expected, $domain, $origin=null ) {
 10+ function testValidateCookieDomain( $expected, $domain, $origin = null ) {
36311 if ( $origin ) {
36412 $ok = Cookie::validateCookieDomain( $domain, $origin );
36513 $msg = "$domain against origin $origin";
——@@ -368,7 +16,7 @@
36917 }
37018 $this->assertEquals( $expected, $ok, $msg );
37119 }
372 -
 20+
37321 function cookieDomains() {
37422 return array(
37523 array( false, "org"),
——@@ -398,189 +46,11 @@
39947 );
40048 }
40149
402 - function testSetCooke() {
403 - $c = new MockCookie( "name", "value",
404 - array(
405 - "domain" => "ac.th",
406 - "path" => "/path/",
407 - ) );
408 - $this->assertFalse( $c->canServeDomain( "ac.th" ) );
409 -
410 - $c = new MockCookie( "name", "value",
411 - array(
412 - "domain" => "example.com",
413 - "path" => "/path/",
414 - ) );
415 -
416 - $this->assertTrue( $c->canServeDomain( "example.com" ) );
417 - $this->assertFalse( $c->canServeDomain( "www.example.com" ) );
418 -
419 - $c = new MockCookie( "name", "value",
420 - array(
421 - "domain" => ".example.com",
422 - "path" => "/path/",
423 - ) );
424 -
425 - $this->assertFalse( $c->canServeDomain( "www.example.net" ) );
426 - $this->assertFalse( $c->canServeDomain( "example.com" ) );
427 - $this->assertTrue( $c->canServeDomain( "www.example.com" ) );
428 -
429 - $this->assertFalse( $c->canServePath( "/" ) );
430 - $this->assertFalse( $c->canServePath( "/bogus/path/" ) );
431 - $this->assertFalse( $c->canServePath( "/path" ) );
432 - $this->assertTrue( $c->canServePath( "/path/" ) );
433 -
434 - $this->assertTrue( $c->isUnExpired() );
435 -
436 - $this->assertEquals( "", $c->serializeToHttpRequest( "/path/", "www.example.net" ) );
437 - $this->assertEquals( "", $c->serializeToHttpRequest( "/", "www.example.com" ) );
438 - $this->assertEquals( "name=value", $c->serializeToHttpRequest( "/path/", "www.example.com" ) );
439 -
440 - $c = new MockCookie( "name", "value",
441 - array(
442 - "domain" => "www.example.com",
443 - "path" => "/path/",
444 - ) );
445 - $this->assertFalse( $c->canServeDomain( "example.com" ) );
446 - $this->assertFalse( $c->canServeDomain( "www.example.net" ) );
447 - $this->assertTrue( $c->canServeDomain( "www.example.com" ) );
448 -
449 - $c = new MockCookie( "name", "value",
450 - array(
451 - "domain" => ".example.com",
452 - "path" => "/path/",
453 - "expires" => "-1 day",
454 - ) );
455 - $this->assertFalse( $c->isUnExpired() );
456 - $this->assertEquals( "", $c->serializeToHttpRequest( "/path/", "www.example.com" ) );
457 -
458 - $c = new MockCookie( "name", "value",
459 - array(
460 - "domain" => ".example.com",
461 - "path" => "/path/",
462 - "expires" => "+1 day",
463 - ) );
464 - $this->assertTrue( $c->isUnExpired() );
465 - $this->assertEquals( "name=value", $c->serializeToHttpRequest( "/path/", "www.example.com" ) );
466 - }
467 -
468 - function testCookieJarSetCookie() {
469 - $cj = new CookieJar;
470 - $cj->setCookie( "name", "value",
471 - array(
472 - "domain" => ".example.com",
473 - "path" => "/path/",
474 - ) );
475 - $cj->setCookie( "name2", "value",
476 - array(
477 - "domain" => ".example.com",
478 - "path" => "/path/sub",
479 - ) );
480 - $cj->setCookie( "name3", "value",
481 - array(
482 - "domain" => ".example.com",
483 - "path" => "/",
484 - ) );
485 - $cj->setCookie( "name4", "value",
486 - array(
487 - "domain" => ".example.net",
488 - "path" => "/path/",
489 - ) );
490 - $cj->setCookie( "name5", "value",
491 - array(
492 - "domain" => ".example.net",
493 - "path" => "/path/",
494 - "expires" => "-1 day",
495 - ) );
496 -
497 - $this->assertEquals( "name4=value", $cj->serializeToHttpRequest( "/path/", "www.example.net" ) );
498 - $this->assertEquals( "name3=value", $cj->serializeToHttpRequest( "/", "www.example.com" ) );
499 - $this->assertEquals( "name=value; name3=value", $cj->serializeToHttpRequest( "/path/", "www.example.com" ) );
500 -
501 - $cj->setCookie( "name5", "value",
502 - array(
503 - "domain" => ".example.net",
504 - "path" => "/path/",
505 - "expires" => "+1 day",
506 - ) );
507 - $this->assertEquals( "name4=value; name5=value", $cj->serializeToHttpRequest( "/path/", "www.example.net" ) );
508 -
509 - $cj->setCookie( "name4", "value",
510 - array(
511 - "domain" => ".example.net",
512 - "path" => "/path/",
513 - "expires" => "-1 day",
514 - ) );
515 - $this->assertEquals( "name5=value", $cj->serializeToHttpRequest( "/path/", "www.example.net" ) );
516 - }
517 -
518 - function testParseResponseHeader() {
519 - $cj = new CookieJar;
520 -
521 - $h[] = "Set-Cookie: name4=value; domain=.example.com; path=/; expires=Mon, 09-Dec-2029 13:46:00 GMT";
522 - $cj->parseCookieResponseHeader( $h[0], "www.example.com" );
523 - $this->assertEquals( "name4=value", $cj->serializeToHttpRequest( "/", "www.example.com" ) );
524 -
525 - $h[] = "name4=value2; domain=.example.com; path=/path/; expires=Mon, 09-Dec-2029 13:46:00 GMT";
526 - $cj->parseCookieResponseHeader( $h[1], "www.example.com" );
527 - $this->assertEquals( "", $cj->serializeToHttpRequest( "/", "www.example.com" ) );
528 - $this->assertEquals( "name4=value2", $cj->serializeToHttpRequest( "/path/", "www.example.com" ) );
529 -
530 - $h[] = "name5=value3; domain=.example.com; path=/path/; expires=Mon, 09-Dec-2029 13:46:00 GMT";
531 - $cj->parseCookieResponseHeader( $h[2], "www.example.com" );
532 - $this->assertEquals( "name4=value2; name5=value3", $cj->serializeToHttpRequest( "/path/", "www.example.com" ) );
533 -
534 - $h[] = "name6=value3; domain=.example.net; path=/path/; expires=Mon, 09-Dec-2029 13:46:00 GMT";
535 - $cj->parseCookieResponseHeader( $h[3], "www.example.com" );
536 - $this->assertEquals( "", $cj->serializeToHttpRequest( "/path/", "www.example.net" ) );
537 -
538 - $h[] = "name6=value0; domain=.example.net; path=/path/; expires=Mon, 09-Dec-1999 13:46:00 GMT";
539 - $cj->parseCookieResponseHeader( $h[4], "www.example.net" );
540 - $this->assertEquals( "", $cj->serializeToHttpRequest( "/path/", "www.example.net" ) );
541 -
542 - $h[] = "name6=value4; domain=.example.net; path=/path/; expires=Mon, 09-Dec-2029 13:46:00 GMT";
543 - $cj->parseCookieResponseHeader( $h[5], "www.example.net" );
544 - $this->assertEquals( "name6=value4", $cj->serializeToHttpRequest( "/path/", "www.example.net" ) );
545 - }
546 -
547 - function runCookieRequests() {
548 - $r = MWHttpRequest::factory( "http://www.php.net/manual", array( 'followRedirects' => true ) );
549 - $r->execute();
550 -
551 - $jar = $r->getCookieJar();
552 - $this->assertThat( $jar, $this->isInstanceOf( 'CookieJar' ) );
553 -
554 - $serialized = $jar->serializeToHttpRequest( "/search?q=test", "www.php.net" );
555 - $this->assertRegExp( '/\bCOUNTRY=[^=;]+/', $serialized );
556 - $this->assertRegExp( '/\bLAST_LANG=[^=;]+/', $serialized );
557 - $this->assertEquals( '', $jar->serializeToHttpRequest( "/search?q=test", "www.php.com" ) );
558 - }
559 -
560 - function testCookieRequestDefault() {
561 - Http::$httpEngine = false;
562 - $this->runCookieRequests();
563 - }
564 - function testCookieRequestPhp() {
565 - if ( !self::$has_fopen ) {
566 - $this->markTestIncomplete( "This test requires allow_url_fopen=true." );
567 - }
568 -
569 - Http::$httpEngine = 'php';
570 - $this->runCookieRequests();
571 - }
572 - function testCookieRequestCurl() {
573 - if ( !self::$has_curl ) {
574 - $this->markTestIncomplete( "This test requires curl." );
575 - }
576 -
577 - Http::$httpEngine = 'curl';
578 - $this->runCookieRequests();
579 - }
580 -
58150 /**
58251 * Test Http::isValidURI()
58352 * @bug 27854 : Http::isValidURI is to lax
584 - *@dataProvider provideURI */
 53+ * @dataProvider provideURI
 54+ */
58555 function testIsValidUri( $expect, $URI, $message = '' ) {
58656 $this->assertEquals(
58757 $expect,

Status & tagging log