r76569 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r76568‎ | r76569 | r76570 >
Date:12:08, 12 November 2010
Author:hashar
Status:ok (Comments)
Tags:
Comment:
Fix ranges formatting issue when testing for (g,z) in IPv6 address
Add / change assertions comments
65535 -> 0xFFFF

Follow up r76560
Modified paths:
  • /trunk/phase3/maintenance/tests/phpunit/includes/IPTest.php (modified) (history)

Diff [purge]

Index: trunk/phase3/maintenance/tests/phpunit/includes/IPTest.php
@@ -6,8 +6,9 @@
77 class IPTest extends PHPUnit_Framework_TestCase {
88 // not sure it should be tested with boolean false. hashar 20100924
99 public function testisIPAddress() {
10 - $this->assertFalse( IP::isIPAddress( false ) );
11 - $this->assertFalse( IP::isIPAddress( "" ) );
 10+ $this->assertFalse( IP::isIPAddress( false ), 'Boolean false is not an IP' );
 11+ $this->assertFalse( IP::isIPAddress( true ), 'Boolean true is not an IP' );
 12+ $this->assertFalse( IP::isIPAddress( "" ), 'Empty string is not an IP' );
1213 $this->assertFalse( IP::isIPAddress( 'abc' ) );
1314 $this->assertFalse( IP::isIPAddress( ':' ) );
1415 $this->assertFalse( IP::isIPAddress( '2001:0DB8::A:1::1'), 'IPv6 with a double :: occurence' );
@@ -19,7 +20,8 @@
2021
2122 $this->assertTrue( IP::isIPAddress( 'fc:100::' ) );
2223 $this->assertTrue( IP::isIPAddress( 'fc:100:a:d:1:e:ac::' ) );
23 - $this->assertTrue( IP::isIPAddress( '::' ), 'IPv6 zero address' );
 24+ $this->assertTrue( IP::isIPAddress( '::' ), 'RFC 4291 IPv6 Unspecified Address' );
 25+ $this->assertTrue( IP::isIPAddress( '::1' ), 'RFC 4291 IPv6 Loopback Address' );
2426 $this->assertTrue( IP::isIPAddress( '::fc' ) );
2527 $this->assertTrue( IP::isIPAddress( '::fc:100:a:d:1:e:ac' ) );
2628 $this->assertTrue( IP::isIPAddress( 'fc::100' ) );
@@ -41,8 +43,8 @@
4244 $this->assertTrue( IP::isIPv6( 'fc:100:a:d:1::' ) );
4345 $this->assertTrue( IP::isIPv6( 'fc:100:a:d:1:e::' ) );
4446 $this->assertTrue( IP::isIPv6( 'fc:100:a:d:1:e:ac::' ) );
45 - $this->assertFalse( IP::isIPv6( 'fc:100:a:d:1:e:ac:0::' ), 'IPv6 ending it "::" with 8 words' );
46 - $this->assertFalse( IP::isIPv6( 'fc:100:a:d:1:e:ac:0:1::' ), 'IPv6 with 9 words' );
 47+ $this->assertFalse( IP::isIPv6( 'fc:100:a:d:1:e:ac:0::' ), 'IPv6 with 8 words ending with "::"' );
 48+ $this->assertFalse( IP::isIPv6( 'fc:100:a:d:1:e:ac:0:1::' ), 'IPv6 with 9 words ending with "::"' );
4749
4850 $this->assertFalse( IP::isIPv6( ':::' ) );
4951 $this->assertFalse( IP::isIPv6( '::0:' ), 'IPv6 ending in a lone ":"' );
@@ -55,11 +57,11 @@
5658 $this->assertTrue( IP::isIPv6( '::fc:100:a:d:1' ) );
5759 $this->assertTrue( IP::isIPv6( '::fc:100:a:d:1:e' ) );
5860 $this->assertTrue( IP::isIPv6( '::fc:100:a:d:1:e:ac' ) );
59 - $this->assertFalse( IP::isIPv6( '::fc:100:a:d:1:e:ac:0' ), 'IPv6 with "::" and 8 octets' );
60 - $this->assertFalse( IP::isIPv6( '::fc:100:a:d:1:e:ac:0:1' ), 'IPv6 with 9 octets' );
 61+ $this->assertFalse( IP::isIPv6( '::fc:100:a:d:1:e:ac:0' ), 'IPv6 with "::" and 8 words' );
 62+ $this->assertFalse( IP::isIPv6( '::fc:100:a:d:1:e:ac:0:1' ), 'IPv6 with 9 words' );
6163
6264 $this->assertFalse( IP::isIPv6( ':fc::100' ), 'IPv6 starting with lone ":"' );
63 - $this->assertFalse( IP::isIPv6( 'fc::100:' ), 'IPv6 ending in lone ":"' );
 65+ $this->assertFalse( IP::isIPv6( 'fc::100:' ), 'IPv6 ending with lone ":"' );
6466 $this->assertFalse( IP::isIPv6( 'fc:::100' ), 'IPv6 with ":::" in the middle' );
6567 $this->assertTrue( IP::isIPv6( 'fc::100' ) );
6668 $this->assertTrue( IP::isIPv6( 'fc::100:a' ) );
@@ -67,8 +69,8 @@
6870 $this->assertTrue( IP::isIPv6( 'fc::100:a:d:1' ) );
6971 $this->assertTrue( IP::isIPv6( 'fc::100:a:d:1:e' ) );
7072 $this->assertTrue( IP::isIPv6( 'fc::100:a:d:1:e:ac' ) );
71 - $this->assertFalse( IP::isIPv6( 'fc::100:a:d:1:e:ac:0' ), 'IPv6 with "::" and 8 octets' );
72 - $this->assertFalse( IP::isIPv6( 'fc::100:a:d:1:e:ac:0:1' ), 'IPv6 with 9 octets' );
 73+ $this->assertFalse( IP::isIPv6( 'fc::100:a:d:1:e:ac:0' ), 'IPv6 with "::" and 8 words' );
 74+ $this->assertFalse( IP::isIPv6( 'fc::100:a:d:1:e:ac:0:1' ), 'IPv6 with 9 words' );
7375
7476 $this->assertTrue( IP::isIPv6( 'fc:100:a:d:1:e:ac:0' ) );
7577 }
@@ -83,7 +85,7 @@
8486 $this->assertTrue( IP::isValid( $ip ) , "$ip is a valid IPv4 address" );
8587 }
8688 }
87 - foreach ( range( 0, 65535 ) as $i ) {
 89+ foreach ( range( 0x0, 0xFFFF ) as $i ) {
8890 $a = sprintf( "%04x", $i );
8991 $b = sprintf( "%03x", $i );
9092 $c = sprintf( "%02x", $i );
@@ -105,9 +107,9 @@
106108 }
107109 }
108110 foreach ( range( 'g', 'z' ) as $i ) {
109 - $a = sprintf( "%04", $i );
110 - $b = sprintf( "%03", $i );
111 - $c = sprintf( "%02", $i );
 111+ $a = sprintf( "%04s", $i );
 112+ $b = sprintf( "%03s", $i );
 113+ $c = sprintf( "%02s", $i );
112114 foreach ( array_unique( array( $a, $b, $c ) ) as $f ) {
113115 $ip = "$f:$f:$f:$f:$f:$f:$f:$f";
114116 $this->assertFalse( IP::isValid( $ip ) , "$ip is not a valid IPv6 address" );

Follow-up revisions

RevisionCommit summaryAuthorDate
r81602Reduce range of IPv6 tested by testValidIPs...hashar20:24, 6 February 2011

Past revisions this follows-up on

RevisionCommit summaryAuthorDate
r76560* Changes in IP.php:...aaron01:33, 12 November 2010

Comments

#Comment by Aaron Schulz (talk | contribs)   17:12, 12 November 2010

Heh, I left out the "s" sprintf param, and 0xFFFF looks much more obvious than 65535 ;)

#Comment by Hashar (talk | contribs)   20:03, 12 November 2010

That's what code review is for : improving quality. Thanks for your patch Aaron.

Status & tagging log