Merge pull request #17 from DavidGoodwin/master

tomverran · web-flow · commit 953b5c161a6d · 2020-01-31T10:59:54.000Z
Bump phpunit; try and avoid PHP notice ...
diff --git a/.travis.yml b/.travis.yml
@@ -1,12 +1,12 @@
 language: php
 php:
+  - 7.2
+  - 7.1
   - 7.0
   - 5.6
-  - 5.5
-  - 5.4
 
 before_script: composer install
-script: phpunit -c phpunit.xml --coverage-clover build/logs/clover.xml
+script: vendor/bin/phpunit -c phpunit.xml --coverage-clover build/logs/clover.xml
 
 after_script:
   - vendor/bin/test-reporter
diff --git a/composer.json b/composer.json
@@ -16,7 +16,7 @@
         "php": ">=5.4.0"
     },
     "require-dev": {
-        "phpunit/phpunit": "3.7.*",
+        "phpunit/phpunit": "^4.0",
         "codeclimate/php-test-reporter": "dev-master"
     },
     "autoload": {
@@ -29,4 +29,4 @@
             "tomverran\\Robot\\": ["src/Robot/", "tests/"]
         }
     }
-}
+}
diff --git a/src/Robot/AccessRules.php b/src/Robot/AccessRules.php
@@ -7,7 +7,7 @@ class AccessRules
     private $rules;
 
     /**
-     * @param Boolean[] $rules A map of path => allowed
+     * @param string[] $rules A map of path => allowed
      */
     public function __construct(array $rules)
     {
@@ -45,4 +45,4 @@ public function isAllowed($url)
         }
         return empty($matches) || array_shift($matches);
     }
-}
+}
diff --git a/src/Robot/RobotsFile.php b/src/Robot/RobotsFile.php
@@ -44,6 +44,10 @@ public function __construct($content)
     public function current()
     {
         $cur = $this->lineIterator->current();
+
+        if (!is_array($cur) || !array_key_exists(1, $cur)) {
+            return '';
+        }
         return count($cur) > 1 ? $cur[1] : '';
     }
 
diff --git a/src/Robot/RobotsTxt.php b/src/Robot/RobotsTxt.php
@@ -24,7 +24,7 @@ class RobotsTxt
      */
     public function __construct($contents)
     {
-        $this->parseFile(new RobotsFile($contents, [self::USER_AGENT, self::ALLOW, self::DISALLOW]));
+        $this->parseFile(new RobotsFile($contents));
     }
 
     /**
diff --git a/tests/RobotTest.php b/tests/RobotTest.php
@@ -178,15 +178,20 @@ public function testCaseSensitivePaths()
      */
     public function testHandlesEmptyUserAgentLine()
     {
-        $invalid_text = <<<EOF
-User-agent: foo
-User-agent:
-Disallow: /
-EOF;
 
-        $g = new RobotsTxt($invalid_text);
+        $g = self::getRobotsTxt('empty-useragent');
 
         $this->assertFalse($g->isAllowed('foo', '/something'));
         $this->assertTrue($g->isAllowed('Googlebot', '/something'));
     }
+
+    /**
+     * Sometimes we might get rubbish with a :: in it, which will confuse the parser...
+     */
+    public function testHtmlAndCSS() {
+
+        $g = self::getRobotsTxt('html-with-css');
+
+        $this->assertTrue($g->isAllowed('foo', '/something'));
+    }
 }
diff --git a/tests/RobotsFileTest.php b/tests/RobotsFileTest.php
@@ -31,4 +31,35 @@ public function givenCrOnlyLineBreaks_pickUpTwoLines($newline)
         $test->next();
         $this->assertEquals(['Disallow', '/'], [$test->key(), $test->current()]);
     }
+
+
+    /**
+     * Get a RobotsTxt class loaded up with google's robots.txt
+     * which is like, the most complex one I've seen.
+     * @param string $file The filename
+     * @return RobotsTxt
+     */
+    private static function getRobotsFile($file)
+    {
+        $d = DIRECTORY_SEPARATOR;
+        return new RobotsFile(file_get_contents(dirname(__FILE__) . $d . 'files' . $d . $file . '.txt'));
+    }
+
+    public function testInvalidDataIn()
+    {
+        $robots = self::getRobotsFile('invalid-data-in');
+        $lines = implode(',', iterator_to_array($robots));
+        $this->assertEquals('SomeUserAgent,x,', $lines);
+    }
+
+    public function testEmptyfile()
+    {
+        $robots = self::getRobotsFile('empty-file');
+        $this->assertEmpty(iterator_to_array($robots));
+    }
+
+    public function testTestContent() {
+        $robots = self::getRobotsFile('test');
+        $this->assertEquals(['Test' => ''], iterator_to_array($robots));
+    }
 }
diff --git a/tests/files/empty-file.txt b/tests/files/empty-file.txt
diff --git a/tests/files/empty-useragent.txt b/tests/files/empty-useragent.txt
@@ -0,0 +1,3 @@
+User-agent: foo
+User-agent:
+Disallow: /
diff --git a/tests/files/html-with-css.txt b/tests/files/html-with-css.txt
@@ -0,0 +1,10 @@
+<!doctype html>
+<head><title>test</title>
+    <style>
+    * {
+    }
+    *::before, <-- THIS
+    ....
+    </style>
+</head>
+</html>
diff --git a/tests/files/invalid-data-in.txt b/tests/files/invalid-data-in.txt
@@ -0,0 +1,3 @@
+User-Agent: SomeUserAgent
+Allow: x
+Disallow:
diff --git a/tests/files/invalid-data.txt b/tests/files/invalid-data.txt
@@ -0,0 +1,3 @@
+User-Agent: SomeUserAgent
+Allow: x
+Disallow:
diff --git a/tests/files/test.txt b/tests/files/test.txt
@@ -0,0 +1 @@
+Test          

Original file line number	Diff line number	Diff line change
`@@ -16,7 +16,7 @@`
`16`	`16`	`"php": ">=5.4.0"`
`17`	`17`	`},`
`18`	`18`	`"require-dev": {`
`19`		`- "phpunit/phpunit": "3.7.*",`
	`19`	`+ "phpunit/phpunit": "^4.0",`
`20`	`20`	`"codeclimate/php-test-reporter": "dev-master"`
`21`	`21`	`},`
`22`	`22`	`"autoload": {`
`@@ -29,4 +29,4 @@`
`29`	`29`	`"tomverran\\Robot\\": ["src/Robot/", "tests/"]`
`30`	`30`	`}`
`31`	`31`	`}`
`32`		`-}`
	`32`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -7,7 +7,7 @@ class AccessRules`
`7`	`7`	`private $rules;`
`8`	`8`
`9`	`9`	`/**`
`10`		`- * @param Boolean[] $rules A map of path => allowed`
	`10`	`+ * @param string[] $rules A map of path => allowed`
`11`	`11`	`*/`
`12`	`12`	`public function __construct(array $rules)`
`13`	`13`	`{`
`@@ -45,4 +45,4 @@ public function isAllowed($url)`
`45`	`45`	`}`
`46`	`46`	`return empty($matches) \|\| array_shift($matches);`
`47`	`47`	`}`
`48`		`-}`
	`48`	`+}`
Original file line number	Diff line number	Diff line change
`@@ -44,6 +44,10 @@ public function __construct($content)`
`44`	`44`	`public function current()`
`45`	`45`	`{`
`46`	`46`	`$cur = $this->lineIterator->current();`
	`47`	`+`
	`48`	`+ if (!is_array($cur) \|\| !array_key_exists(1, $cur)) {`
	`49`	`+ return '';`
	`50`	`+ }`
`47`	`51`	`return count($cur) > 1 ? $cur[1] : '';`
`48`	`52`	`}`
`49`	`53`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ class RobotsTxt`
`24`	`24`	`*/`
`25`	`25`	`public function __construct($contents)`
`26`	`26`	`{`
`27`		`- $this->parseFile(new RobotsFile($contents, [self::USER_AGENT, self::ALLOW, self::DISALLOW]));`
	`27`	`+ $this->parseFile(new RobotsFile($contents));`
`28`	`28`	`}`
`29`	`29`
`30`	`30`	`/**`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+User-agent: foo`
	`2`	`+User-agent:`
	`3`	`+Disallow: /`