From 31372c619eb9beb848d14d0c08c567a03cc92660 Mon Sep 17 00:00:00 2001 From: Tim Starling Date: Mon, 9 Jul 2018 14:56:21 +1000 Subject: [PATCH] In PathRouterTest use @dataProvider where possible "Where possible" turns out to be every test in this class. I used named data sets (i.e. array keys in the provided array) for the main set of tests, to improve the PHPUnit output. Change-Id: Ic68edc01b3e0e174983471a36f3c5d52e28abfdd --- tests/phpunit/includes/PathRouterTest.php | 426 +++++++++++----------- 1 file changed, 215 insertions(+), 211 deletions(-) diff --git a/tests/phpunit/includes/PathRouterTest.php b/tests/phpunit/includes/PathRouterTest.php index fc6a70b1f4..15c47917bb 100644 --- a/tests/phpunit/includes/PathRouterTest.php +++ b/tests/phpunit/includes/PathRouterTest.php @@ -19,133 +19,243 @@ class PathRouterTest extends MediaWikiTestCase { $this->basicRouter = $router; } - /** - * Test basic path parsing - */ - public function testBasic() { - $matches = $this->basicRouter->parse( "/wiki/Foo" ); - $this->assertEquals( $matches, [ 'title' => "Foo" ] ); - } + public static function provideParse() { + $tests = [ + // Basic path parsing + 'Basic path parsing' => [ + "/wiki/$1", + "/wiki/Foo", + [ 'title' => "Foo" ] + ], + // + 'Loose path auto-$1: /$1' => [ + "/", + "/Foo", + [ 'title' => "Foo" ] + ], + 'Loose path auto-$1: /wiki' => [ + "/wiki", + "/wiki/Foo", + [ 'title' => "Foo" ] + ], + 'Loose path auto-$1: /wiki/' => [ + "/wiki/", + "/wiki/Foo", + [ 'title' => "Foo" ] + ], + // Ensure that path is based on specificity, not order + 'Order, /$1 added first' => [ + [ "/$1", "/a/$1", "/b/$1" ], + "/a/Foo", + [ 'title' => "Foo" ] + ], + 'Order, /$1 added last' => [ + [ "/b/$1", "/a/$1", "/$1" ], + "/a/Foo", + [ 'title' => "Foo" ] + ], + // Handling of key based arrays with a url parameter + 'Key based array' => [ + [ [ + 'path' => [ 'edit' => "/edit/$1" ], + 'params' => [ 'action' => '$key' ], + ] ], + "/edit/Foo", + [ 'title' => "Foo", 'action' => 'edit' ] + ], + // Additional parameter + 'Basic $2' => [ + [ [ + 'path' => '/$2/$1', + 'params' => [ 'test' => '$2' ] + ] ], + "/asdf/Foo", + [ 'title' => "Foo", 'test' => 'asdf' ] + ], + ]; + // Shared patterns for restricted value parameter tests + $restrictedPatterns = [ + [ + 'path' => '/$2/$1', + 'params' => [ 'test' => '$2' ], + 'options' => [ '$2' => [ 'a', 'b' ] ] + ], + [ + 'path' => '/$2/$1', + 'params' => [ 'test2' => '$2' ], + 'options' => [ '$2' => 'c' ] + ], + '/$1' + ]; + $tests += [ + // Restricted value parameter tests + 'Restricted 1' => [ + $restrictedPatterns, + "/asdf/Foo", + [ 'title' => "asdf/Foo" ] + ], + 'Restricted 2' => [ + $restrictedPatterns, + "/a/Foo", + [ 'title' => "Foo", 'test' => 'a' ] + ], + 'Restricted 3' => [ + $restrictedPatterns, + "/c/Foo", + [ 'title' => "Foo", 'test2' => 'c' ] + ], - /** - * Test loose path auto-$1 - */ - public function testLoose() { - $router = new PathRouter; - $router->add( "/" ); # Should be the same as "/$1" - $matches = $router->parse( "/Foo" ); - $this->assertEquals( $matches, [ 'title' => "Foo" ] ); + // Callback test + 'Callback' => [ + [ [ + 'path' => "/$1", + 'params' => [ 'a' => 'b', 'data:foo' => 'bar' ], + 'options' => [ 'callback' => [ __CLASS__, 'callbackForTest' ] ] + ] ], + '/Foo', + [ + 'title' => "Foo", + 'x' => 'Foo', + 'a' => 'b', + 'foo' => 'bar' + ] + ], - $router = new PathRouter; - $router->add( "/wiki" ); # Should be the same as /wiki/$1 - $matches = $router->parse( "/wiki/Foo" ); - $this->assertEquals( $matches, [ 'title' => "Foo" ] ); + // Test to ensure that matches are not made if a parameter expects nonexistent input + 'Fail' => [ + [ [ + 'path' => "/wiki/$1", + 'params' => [ 'title' => "$1$2" ], + ] ], + "/wiki/A", + [] + ], - $router = new PathRouter; - $router->add( "/wiki/" ); # Should be the same as /wiki/$1 - $matches = $router->parse( "/wiki/Foo" ); - $this->assertEquals( $matches, [ 'title' => "Foo" ] ); - } + // Make sure the router handles titles like Special:Recentchanges correctly + 'Special title' => [ + "/wiki/$1", + "/wiki/Special:Recentchanges", + [ 'title' => "Special:Recentchanges" ] + ], - /** - * Test to ensure that path is based on specifity, not order - */ - public function testOrder() { - $router = new PathRouter; - $router->add( "/$1" ); - $router->add( "/a/$1" ); - $router->add( "/b/$1" ); - $matches = $router->parse( "/a/Foo" ); - $this->assertEquals( $matches, [ 'title' => "Foo" ] ); + // Make sure the router decodes urlencoding properly + 'URL encoding' => [ + "/wiki/$1", + "/wiki/Title_With%20Space", + [ 'title' => "Title_With Space" ] + ], - $router = new PathRouter; - $router->add( "/b/$1" ); - $router->add( "/a/$1" ); - $router->add( "/$1" ); - $matches = $router->parse( "/a/Foo" ); - $this->assertEquals( $matches, [ 'title' => "Foo" ] ); - } + ]; - /** - * Test the handling of key based arrays with a url parameter - */ - public function testKeyParameter() { - $router = new PathRouter; - $router->add( [ 'edit' => "/edit/$1" ], [ 'action' => '$key' ] ); - $matches = $router->parse( "/edit/Foo" ); - $this->assertEquals( $matches, [ 'title' => "Foo", 'action' => 'edit' ] ); - } + // Make sure the router doesn't break on special characters like $ used in regexp replacements + foreach ( [ "$", "$1", "\\", "\\$1" ] as $char ) { + $tests["Regexp character $char"] = [ + "/wiki/$1", + "/wiki/$char", + [ 'title' => "$char" ] + ]; + } - /** - * Test the handling of $2 inside paths - */ - public function testAdditionalParameter() { - // Basic $2 - $router = new PathRouter; - $router->add( '/$2/$1', [ 'test' => '$2' ] ); - $matches = $router->parse( "/asdf/Foo" ); - $this->assertEquals( $matches, [ 'title' => "Foo", 'test' => 'asdf' ] ); - } + $tests += [ + // Make sure the router handles characters like +&() properly + "Special characters" => [ + "/wiki/$1", + "/wiki/Plus+And&Dollar\\Stuff();[]{}*", + [ 'title' => "Plus+And&Dollar\\Stuff();[]{}*" ], + ], - /** - * Test additional restricted value parameter - */ - public function testRestrictedValue() { - $router = new PathRouter; - $router->add( '/$2/$1', - [ 'test' => '$2' ], - [ '$2' => [ 'a', 'b' ] ] - ); - $router->add( '/$2/$1', - [ 'test2' => '$2' ], - [ '$2' => 'c' ] - ); - $router->add( '/$1' ); + // Make sure the router handles unicode characters correctly + "Unicode 1" => [ + "/wiki/$1", + "/wiki/Spécial:Modifications_récentes" , + [ 'title' => "Spécial:Modifications_récentes" ], + ], - $matches = $router->parse( "/asdf/Foo" ); - $this->assertEquals( $matches, [ 'title' => "asdf/Foo" ] ); + "Unicode 2" => [ + "/wiki/$1", + "/wiki/Sp%C3%A9cial:Modifications_r%C3%A9centes", + [ 'title' => "Spécial:Modifications_récentes" ], + ] + ]; - $matches = $router->parse( "/a/Foo" ); - $this->assertEquals( $matches, [ 'title' => "Foo", 'test' => 'a' ] ); + // Ensure the router doesn't choke on long paths. + $lorem = "Lorem_ipsum_dolor_sit_amet,_consectetur_adipisicing_elit,_sed_do_eiusmod_" . + "tempor_incididunt_ut_labore_et_dolore_magna_aliqua._Ut_enim_ad_minim_veniam,_quis_" . + "nostrud_exercitation_ullamco_laboris_nisi_ut_aliquip_ex_ea_commodo_consequat._" . + "Duis_aute_irure_dolor_in_reprehenderit_in_voluptate_velit_esse_cillum_dolore_" . + "eu_fugiat_nulla_pariatur._Excepteur_sint_occaecat_cupidatat_non_proident,_sunt_" . + "in_culpa_qui_officia_deserunt_mollit_anim_id_est_laborum."; - $matches = $router->parse( "/c/Foo" ); - $this->assertEquals( $matches, [ 'title' => "Foo", 'test2' => 'c' ] ); - } + $tests += [ + "Long path" => [ + "/wiki/$1", + "/wiki/$lorem", + [ 'title' => $lorem ] + ], - public function callbackForTest( &$matches, $data ) { - $matches['x'] = $data['$1']; - $matches['foo'] = $data['foo']; - } + // Ensure that the php passed site of parameter values are not urldecoded + "Pattern urlencoding" => [ + [ [ 'path' => "/wiki/$1", 'params' => [ 'title' => '%20:$1' ] ] ], + "/wiki/Foo", + [ 'title' => '%20:Foo' ] + ], - public function testCallback() { - $router = new PathRouter; - $router->add( "/$1", - [ 'a' => 'b', 'data:foo' => 'bar' ], - [ 'callback' => [ $this, 'callbackForTest' ] ] - ); - $matches = $router->parse( '/Foo' ); - $this->assertEquals( $matches, [ - 'title' => "Foo", - 'x' => 'Foo', - 'a' => 'b', - 'foo' => 'bar' - ] ); + // Ensure that raw parameter values do not have any variable replacements or urldecoding + "Raw param value" => [ + [ [ 'path' => "/wiki/$1", 'params' => [ 'title' => [ 'value' => 'bar%20$1' ] ] ] ], + "/wiki/Foo", + [ 'title' => 'bar%20$1' ] + ] + ]; + + return $tests; } /** - * Test to ensure that matches are not made if a parameter expects nonexistent input + * Test path parsing + * @dataProvider provideParse */ - public function testFail() { + public function testParse( $patterns, $path, $expected ) { + $patterns = (array)$patterns; + $router = new PathRouter; - $router->add( "/wiki/$1", [ 'title' => "$1$2" ] ); - $matches = $router->parse( "/wiki/A" ); - $this->assertEquals( [], $matches ); + foreach ( $patterns as $pattern ) { + if ( is_array( $pattern ) ) { + $router->add( $pattern['path'], $pattern['params'] ?? [], + $pattern['options'] ?? [] ); + } else { + $router->add( $pattern ); + } + } + $matches = $router->parse( $path ); + $this->assertEquals( $matches, $expected ); + } + + public static function callbackForTest( &$matches, $data ) { + $matches['x'] = $data['$1']; + $matches['foo'] = $data['foo']; + } + + public static function provideWeight() { + return [ + [ '/Foo', [ 'title' => 'Foo' ] ], + [ '/Bar', [ 'ping' => 'pong' ] ], + [ '/Baz', [ 'marco' => 'polo' ] ], + [ '/asdf-foo', [ 'title' => 'qwerty-foo' ] ], + [ '/qwerty-bar', [ 'title' => 'asdf-bar' ] ], + [ '/a/Foo', [ 'title' => 'Foo' ] ], + [ '/asdf/Foo', [ 'title' => 'Foo' ] ], + [ '/qwerty/Foo', [ 'title' => 'Foo', 'qwerty' => 'qwerty' ] ], + [ '/baz/Foo', [ 'title' => 'Foo', 'unrestricted' => 'baz' ] ], + [ '/y/Foo', [ 'title' => 'Foo', 'restricted-to-y' => 'y' ] ], + ]; } /** * Test to ensure weight of paths is handled correctly + * @dataProvider provideWeight */ - public function testWeight() { + public function testWeight( $path, $expected ) { $router = new PathRouter; $router->addStrict( "/Bar", [ 'ping' => 'pong' ] ); $router->add( "/asdf-$1", [ 'title' => 'qwerty-$1' ] ); @@ -158,112 +268,6 @@ class PathRouterTest extends MediaWikiTestCase { $router->add( [ 'qwerty' => "/qwerty/$1" ], [ 'qwerty' => '$key' ] ); $router->add( "/$2/$1", [ 'restricted-to-y' => '$2' ], [ '$2' => 'y' ] ); - foreach ( - [ - '/Foo' => [ 'title' => 'Foo' ], - '/Bar' => [ 'ping' => 'pong' ], - '/Baz' => [ 'marco' => 'polo' ], - '/asdf-foo' => [ 'title' => 'qwerty-foo' ], - '/qwerty-bar' => [ 'title' => 'asdf-bar' ], - '/a/Foo' => [ 'title' => 'Foo' ], - '/asdf/Foo' => [ 'title' => 'Foo' ], - '/qwerty/Foo' => [ 'title' => 'Foo', 'qwerty' => 'qwerty' ], - '/baz/Foo' => [ 'title' => 'Foo', 'unrestricted' => 'baz' ], - '/y/Foo' => [ 'title' => 'Foo', 'restricted-to-y' => 'y' ], - ] as $path => $result - ) { - $this->assertEquals( $router->parse( $path ), $result ); - } - } - - /** - * Make sure the router handles titles like Special:Recentchanges correctly - */ - public function testSpecial() { - $matches = $this->basicRouter->parse( "/wiki/Special:Recentchanges" ); - $this->assertEquals( $matches, [ 'title' => "Special:Recentchanges" ] ); - } - - /** - * Make sure the router decodes urlencoding properly - */ - public function testUrlencoding() { - $matches = $this->basicRouter->parse( "/wiki/Title_With%20Space" ); - $this->assertEquals( $matches, [ 'title' => "Title_With Space" ] ); - } - - public static function provideRegexpChars() { - return [ - [ "$" ], - [ "$1" ], - [ "\\" ], - [ "\\$1" ], - ]; - } - - /** - * Make sure the router doesn't break on special characters like $ used in regexp replacements - * @dataProvider provideRegexpChars - */ - public function testRegexpChars( $char ) { - $matches = $this->basicRouter->parse( "/wiki/$char" ); - $this->assertEquals( $matches, [ 'title' => "$char" ] ); - } - - /** - * Make sure the router handles characters like +&() properly - */ - public function testCharacters() { - $matches = $this->basicRouter->parse( "/wiki/Plus+And&Dollar\\Stuff();[]{}*" ); - $this->assertEquals( $matches, [ 'title' => "Plus+And&Dollar\\Stuff();[]{}*" ] ); - } - - /** - * Make sure the router handles unicode characters correctly - * @depends testSpecial - * @depends testUrlencoding - * @depends testCharacters - */ - public function testUnicode() { - $matches = $this->basicRouter->parse( "/wiki/Spécial:Modifications_récentes" ); - $this->assertEquals( $matches, [ 'title' => "Spécial:Modifications_récentes" ] ); - - $matches = $this->basicRouter->parse( "/wiki/Sp%C3%A9cial:Modifications_r%C3%A9centes" ); - $this->assertEquals( $matches, [ 'title' => "Spécial:Modifications_récentes" ] ); - } - - /** - * Ensure the router doesn't choke on long paths. - */ - public function testLength() { - // phpcs:disable Generic.Files.LineLength - $matches = $this->basicRouter->parse( - "/wiki/Lorem_ipsum_dolor_sit_amet,_consectetur_adipisicing_elit,_sed_do_eiusmod_tempor_incididunt_ut_labore_et_dolore_magna_aliqua._Ut_enim_ad_minim_veniam,_quis_nostrud_exercitation_ullamco_laboris_nisi_ut_aliquip_ex_ea_commodo_consequat._Duis_aute_irure_dolor_in_reprehenderit_in_voluptate_velit_esse_cillum_dolore_eu_fugiat_nulla_pariatur._Excepteur_sint_occaecat_cupidatat_non_proident,_sunt_in_culpa_qui_officia_deserunt_mollit_anim_id_est_laborum." - ); - $this->assertEquals( - $matches, - [ 'title' => "Lorem_ipsum_dolor_sit_amet,_consectetur_adipisicing_elit,_sed_do_eiusmod_tempor_incididunt_ut_labore_et_dolore_magna_aliqua._Ut_enim_ad_minim_veniam,_quis_nostrud_exercitation_ullamco_laboris_nisi_ut_aliquip_ex_ea_commodo_consequat._Duis_aute_irure_dolor_in_reprehenderit_in_voluptate_velit_esse_cillum_dolore_eu_fugiat_nulla_pariatur._Excepteur_sint_occaecat_cupidatat_non_proident,_sunt_in_culpa_qui_officia_deserunt_mollit_anim_id_est_laborum." ] - ); - // phpcs:enable - } - - /** - * Ensure that the php passed site of parameter values are not urldecoded - */ - public function testPatternUrlencoding() { - $router = new PathRouter; - $router->add( "/wiki/$1", [ 'title' => '%20:$1' ] ); - $matches = $router->parse( "/wiki/Foo" ); - $this->assertEquals( $matches, [ 'title' => '%20:Foo' ] ); - } - - /** - * Ensure that raw parameter values do not have any variable replacements or urldecoding - */ - public function testRawParamValue() { - $router = new PathRouter; - $router->add( "/wiki/$1", [ 'title' => [ 'value' => 'bar%20$1' ] ] ); - $matches = $router->parse( "/wiki/Foo" ); - $this->assertEquals( $matches, [ 'title' => 'bar%20$1' ] ); + $this->assertEquals( $router->parse( $path ), $expected ); } } -- 2.20.1