From cb5a657650222bc670b861fea08df91ea57b52b7 Mon Sep 17 00:00:00 2001
From: Guy Sartorelli <guy.sartorelli@silverstripe.com>
Date: Wed, 1 Nov 2023 12:10:55 +1300
Subject: [PATCH] MNT Add unit tests

---
 .github/workflows/ci.yml                      |  11 ++
 .github/workflows/dispatch-ci.yml             |  16 ++
 .github/workflows/merge-up.yml                |   4 +-
 .gitignore                                    |   1 +
 composer.json                                 |   8 +-
 phpunit.xml.dist                              |  12 ++
 src/Sniffer.php                               |   4 +
 tests/CodeBlockTest.php                       |  50 +++++++
 tests/SnifferFixTest.php                      |  66 ++++++++
 tests/SnifferTest.php                         | 141 ++++++++++++++++++
 tests/bootstrap.php                           |  28 ++++
 .../lint-with-problems.md                     |  45 ++++++
 tests/fixtures/lint-but-no-problems.md        |  12 ++
 tests/fixtures/lint-with-problems.md          |  43 ++++++
 tests/fixtures/nothing-to-lint.md             |  28 ++++
 15 files changed, 465 insertions(+), 4 deletions(-)
 create mode 100644 .github/workflows/ci.yml
 create mode 100644 .github/workflows/dispatch-ci.yml
 create mode 100644 phpunit.xml.dist
 create mode 100644 tests/CodeBlockTest.php
 create mode 100644 tests/SnifferFixTest.php
 create mode 100644 tests/SnifferTest.php
 create mode 100644 tests/bootstrap.php
 create mode 100644 tests/expected-after-fixing/lint-with-problems.md
 create mode 100644 tests/fixtures/lint-but-no-problems.md
 create mode 100644 tests/fixtures/lint-with-problems.md
 create mode 100644 tests/fixtures/nothing-to-lint.md

diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
new file mode 100644
index 0000000..bf02210
--- /dev/null
+++ b/.github/workflows/ci.yml
@@ -0,0 +1,11 @@
+name: CI
+
+on:
+  push:
+  pull_request:
+  workflow_dispatch:
+
+jobs:
+  ci:
+    name: CI
+    uses: silverstripe/gha-ci/.github/workflows/ci.yml@v1
diff --git a/.github/workflows/dispatch-ci.yml b/.github/workflows/dispatch-ci.yml
new file mode 100644
index 0000000..5632551
--- /dev/null
+++ b/.github/workflows/dispatch-ci.yml
@@ -0,0 +1,16 @@
+name: Dispatch CI
+
+on:
+  # At 6:30 PM UTC, only on Sunday and Monday
+  schedule:
+  - cron: '30 18 * * 0,1'
+
+jobs:
+  dispatch-ci:
+  name: Dispatch CI
+  # Only run cron on the silverstripe account
+  if: (github.event_name == 'schedule' && github.repository_owner == 'silverstripe') || (github.event_name != 'schedule')
+  runs-on: ubuntu-latest
+  steps:
+    - name: Dispatch CI
+      uses: silverstripe/gha-dispatch-ci@v1
diff --git a/.github/workflows/merge-up.yml b/.github/workflows/merge-up.yml
index 41f3410..d343ab7 100644
--- a/.github/workflows/merge-up.yml
+++ b/.github/workflows/merge-up.yml
@@ -1,9 +1,9 @@
 name: Merge-up
 
 on:
-  # At 6:30 PM UTC, only on Sunday
+  # At 6:30 PM UTC, only on Thursday
   schedule:
-    - cron: '30 18 * * 0'
+    - cron: '30 18 * * 4'
   workflow_dispatch:
 
 jobs:
diff --git a/.gitignore b/.gitignore
index 4fbb073..525642d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -1,2 +1,3 @@
 /vendor/
 /composer.lock
+.phpunit.result.cache
diff --git a/composer.json b/composer.json
index f449e52..3da10e0 100644
--- a/composer.json
+++ b/composer.json
@@ -16,10 +16,14 @@
     },
     "autoload": {
         "psr-4": {
-            "SilverStripe\\MD_PHP_CodeSniffer\\": "src/"
+            "SilverStripe\\MD_PHP_CodeSniffer\\": "src/",
+            "SilverStripe\\MD_PHP_CodeSniffer\\Tests\\": "tests/"
         }
     },
     "bin": [
         "bin/mdphpcs"
-    ]
+    ],
+    "require-dev": {
+        "phpunit/phpunit": "^9.6"
+    }
 }
diff --git a/phpunit.xml.dist b/phpunit.xml.dist
new file mode 100644
index 0000000..d50172b
--- /dev/null
+++ b/phpunit.xml.dist
@@ -0,0 +1,12 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<phpunit bootstrap="tests/bootstrap.php" colors="true">
+    <testsuites>
+        <testsuite name="all-except-fixing">
+            <directory>tests</directory>
+            <exclude>tests/SnifferFixTest.php</exclude>
+        </testsuite>
+        <testsuite name="fixing-only">
+            <file>tests/SnifferFixTest.php</file>
+        </testsuite>
+    </testsuites>
+</phpunit>
diff --git a/src/Sniffer.php b/src/Sniffer.php
index 12ea34c..2d569cd 100644
--- a/src/Sniffer.php
+++ b/src/Sniffer.php
@@ -119,6 +119,10 @@ private function prepareConfig(bool $usingExplicitStandard, string $lintLanguage
         // Creating the Config object populates it with all required settings based on the phpcs/phpcbf CLI arguments provided.
         $config = new Config();
 
+        if (defined('PHP_CODESNIFFER_IN_TESTS') && PHP_CODESNIFFER_IN_TESTS) {
+            $config->files = [str_replace('/src', '/tests/fixtures', __DIR__ )];
+        }
+
         // We don't support STDIN for passing markdown in
         if ($config->stdin === true) {
             // 3 is the exit code phpcs uses for errors like this
diff --git a/tests/CodeBlockTest.php b/tests/CodeBlockTest.php
new file mode 100644
index 0000000..edb18ca
--- /dev/null
+++ b/tests/CodeBlockTest.php
@@ -0,0 +1,50 @@
+<?php
+
+namespace SilverStripe\MD_PHP_CodeSniffer\Test;
+
+use ReflectionProperty;
+use PHP_CodeSniffer\Config;
+use PHP_CodeSniffer\Ruleset;
+use PHPUnit\Framework\TestCase;
+use SilverStripe\MD_PHP_CodeSniffer\CodeBlock;
+
+class CodeBlockTest extends TestCase
+{
+    public static function setUpBeforeClass(): void
+    {
+        parent::setUpBeforeClass();
+
+        if (!defined('PHP_CODESNIFFER_CBF')) {
+            define('PHP_CODESNIFFER_CBF', false);
+        }
+    }
+
+    public function testGetContent()
+    {
+        $config = new Config();
+        $block = new CodeBlock('This is the content', new Ruleset($config), $config);
+
+        $this->assertSame('This is the content', $block->getContent());
+
+        $block->setContent('New content now');
+
+        $this->assertSame('New content now', $block->getContent());
+    }
+
+    public function testCleanup()
+    {
+        $config = new Config();
+        $block = new CodeBlock('This is the content', new Ruleset($config), $config);
+        $block->cleanUp();
+
+        $this->assertSame('This is the content', $block->getContent());
+
+        $reflectionContent = new ReflectionProperty($block, 'content');
+        $reflectionContent->setAccessible(true);
+        $reflectionFinalContent = new ReflectionProperty($block, 'finalContent');
+        $reflectionFinalContent->setAccessible(true);
+
+        $this->assertNull($reflectionContent->getValue($block));
+        $this->assertSame('This is the content', $reflectionFinalContent->getValue($block));
+    }
+}
diff --git a/tests/SnifferFixTest.php b/tests/SnifferFixTest.php
new file mode 100644
index 0000000..211ec4a
--- /dev/null
+++ b/tests/SnifferFixTest.php
@@ -0,0 +1,66 @@
+<?php
+
+namespace SilverStripe\MD_PHP_CodeSniffer\Test;
+
+use PHP_CodeSniffer\Files\FileList;
+use PHP_CodeSniffer\Ruleset;
+use PHPUnit\Framework\TestCase;
+use ReflectionMethod;
+use SilverStripe\MD_PHP_CodeSniffer\Sniffer;
+
+/**
+ * Because of PHP CodeSniffer's reliance on constants, this has to be done separately from the other tests.
+ */
+class SnifferFixTest extends TestCase
+{
+    public function testFix()
+    {
+        if (!defined('PHP_CODESNIFFER_CBF')) {
+            define('PHP_CODESNIFFER_CBF', true);
+        }
+
+        $sniffer = new Sniffer();
+
+        // backup original content
+        $orig = [];
+        $paths = self::getFilesList($sniffer);
+        /** @var string $path */
+        foreach ($paths as $path => $v) {
+            $orig[$path] = file_get_contents($path);
+        }
+
+        try {
+            ob_start();
+            $exitCode = $sniffer->run('PHP', true, true);
+            $output = ob_get_clean();
+
+            // Validate that the files which should change did, and which shouldn't change didn't
+            foreach ($orig as $path => $content) {
+                $this->assertFileExists($path);
+
+                if (str_contains($path, 'lint-with-problems')) {
+                    $this->assertFileEquals(str_replace('/fixtures/', '/expected-after-fixing/', $path), $path);
+                } else {
+                    $this->assertSame($content, file_get_contents($path));
+                }
+            }
+
+            // There are no remaining auto-fixable problems
+            $this->assertSame(1, $exitCode);
+        } finally {
+            // Put the original content back
+            foreach ($orig as $path => $content) {
+                file_put_contents($path, $content);
+            }
+        }
+    }
+
+    private static function getFilesList(Sniffer $sniffer): FileList
+    {
+        $prepareConfig = new ReflectionMethod($sniffer, 'prepareConfig');
+        $prepareConfig->setAccessible(true);
+        $config = $prepareConfig->invoke($sniffer, false, 'PHP', true);
+
+        return new FileList($config, new Ruleset($config));
+    }
+}
diff --git a/tests/SnifferTest.php b/tests/SnifferTest.php
new file mode 100644
index 0000000..addfc2b
--- /dev/null
+++ b/tests/SnifferTest.php
@@ -0,0 +1,141 @@
+<?php
+
+namespace SilverStripe\MD_PHP_CodeSniffer\Test;
+
+use PHP_CodeSniffer\Files\FileList;
+use PHP_CodeSniffer\Ruleset;
+use PHPUnit\Framework\TestCase;
+use ReflectionMethod;
+use SilverStripe\MD_PHP_CodeSniffer\Sniffer;
+
+class SnifferTest extends TestCase
+{
+    /**
+     * Validates that fenced code blocks are correctly identified and have the expected data
+     *
+     * @dataProvider provideFindFencedBlocks
+     */
+    public function testFindFencedCodeBlocks(string $path, bool $exists, string $realPath = '', int $num = 0, string $content = '')
+    {
+        if (!defined('PHP_CODESNIFFER_CBF')) {
+            define('PHP_CODESNIFFER_CBF', false);
+        }
+
+        $sniffer = new Sniffer();
+        $files = self::getFilesList($sniffer);
+
+        $findFencedCodeblocks = new ReflectionMethod($sniffer, 'findFencedCodeblocks');
+        $findFencedCodeblocks->setAccessible(true);
+        $blocks = $findFencedCodeblocks->invoke($sniffer, $files, 'PHP');
+
+        $blockKey = __DIR__ . $path;
+
+        if ($exists) {
+            $this->assertArrayHasKey($blockKey, $blocks, 'block must be found');
+
+            $block = $blocks[$blockKey];
+            $this->assertSame($blockKey, $block['path'], 'block path must be correct');
+            $this->assertSame(__DIR__ . $realPath, $block['realpath'], 'block realpath must be correct');
+            $this->assertSame($num, $block['num'], 'block must be numbered correctly');
+            $this->assertSame($content, ltrim($block['content']), 'block content must be correct');
+        } else {
+            $this->assertArrayNotHasKey($blockKey, $blocks, 'block must not be found');
+        }
+    }
+
+    public function provideFindFencedBlocks()
+    {
+        return [
+            'nothing to lint 1' => [
+                'path' => '/fixtures/nothing-to-lint.md',
+                'exists' => false,
+            ],
+            'nothing to lint 2' => [
+                'path' => '/fixtures/nothing-to-lint_1.md',
+                'exists' => false,
+            ],
+            'file paths all include block numbers' => [
+                'path' => '/fixtures/lint-but-no-problems.md',
+                'exists' => false,
+            ],
+            [
+                'path' => '/fixtures/lint-but-no-problems_1.md',
+                'exists' => true,
+                'realpath' => '/fixtures/lint-but-no-problems.md',
+                'num' => 1,
+                'content' => <<<'MD'
+                <?php
+                namespace App;
+
+                class MyClass
+                {
+                    private string $myProperty = 'this is the value';
+                }
+
+                MD
+            ],
+            'no hallucinated block' => [
+                'path' => '/fixtures/lint-but-no-problems_2.md',
+                'exists' => false,
+            ],
+            // No need to check lint-with-problems_1 and lint-with-problems_2 - they're functionality
+            // identical to lint-but-no-problems_1 for the purposes of this test.
+            'language identifier not case sensitive' => [
+                'path' => '/fixtures/lint-with-problems_3.md',
+                'exists' => true,
+                'realpath' => '/fixtures/lint-with-problems.md',
+                'num' => 3,
+                'content' => <<<'MD'
+                <?php
+                class AnotherClass {
+                    private string $anotherProperty='this is the value';
+                }
+
+                MD
+            ],
+            'indentation not in content' => [
+                'path' => '/fixtures/lint-with-problems_4.md',
+                'exists' => true,
+                'realpath' => '/fixtures/lint-with-problems.md',
+                'num' => 4,
+                'content' => <<<'MD'
+                <?php
+                class FinalClass {
+                    private string $lastProperty='this is the value';
+                }
+
+                MD
+            ],
+        ];
+    }
+
+    public function testSniff()
+    {
+        $sniffer = new Sniffer();
+        ob_start();
+        $exitCode = $sniffer->run('PHP', false, true);
+        $output = ob_get_clean();
+
+        // There are auto-fixable problems
+        $this->assertSame(2, $exitCode);
+
+        // Check we didn't find problems where there aren't any
+        $this->assertStringNotContainsString('nothing-to-lint', $output, 'nothing to lint, so nothing found');
+        $this->assertStringNotContainsString('lint-but-no-problems', $output, 'linted but no problems found');
+        $this->assertStringNotContainsString('lint-with-problems_2', $output, 'that code block has no linting problems');
+
+        // Check we did find problems where there are plenty
+        $this->assertStringContainsString('lint-with-problems_1', $output);
+        $this->assertStringContainsString('lint-with-problems_3', $output);
+        $this->assertStringContainsString('lint-with-problems_4', $output);
+    }
+
+    private static function getFilesList(Sniffer $sniffer): FileList
+    {
+        $prepareConfig = new ReflectionMethod($sniffer, 'prepareConfig');
+        $prepareConfig->setAccessible(true);
+        $config = $prepareConfig->invoke($sniffer, false, 'PHP', true);
+
+        return new FileList($config, new Ruleset($config));
+    }
+}
diff --git a/tests/bootstrap.php b/tests/bootstrap.php
new file mode 100644
index 0000000..7713bfa
--- /dev/null
+++ b/tests/bootstrap.php
@@ -0,0 +1,28 @@
+<?php
+
+// php_codesniffer autoloader
+$autoloadCandidates = [
+    // running from package itself as root
+    __DIR__ . '/../vendor/squizlabs/php_codesniffer/autoload.php',
+    // running from package in vendor/silverstripe/markdown-php-codesniffer
+    __DIR__ . '/../../../squizlabs/php_codesniffer/autoload.php',
+];
+
+$autoloaded = false;
+foreach ($autoloadCandidates as $candidate) {
+    if (is_file($candidate)) {
+        require_once $candidate;
+        $autoloaded = true;
+        break;
+    }
+}
+
+if (!defined('PHP_CODESNIFFER_VERBOSITY')) {
+    define('PHP_CODESNIFFER_VERBOSITY', 0);
+}
+
+if (!defined('PHP_CODESNIFFER_IN_TESTS')) {
+    define('PHP_CODESNIFFER_IN_TESTS', true);
+}
+
+$_SERVER['argv'][] = '--standard=' . str_replace('/tests', '/phpcs.default.xml', __DIR__);
diff --git a/tests/expected-after-fixing/lint-with-problems.md b/tests/expected-after-fixing/lint-with-problems.md
new file mode 100644
index 0000000..bcea627
--- /dev/null
+++ b/tests/expected-after-fixing/lint-with-problems.md
@@ -0,0 +1,45 @@
+# This file has something to lint
+
+```php
+class MyClass
+{
+    private string $myProperty = 'this is the value';
+}
+```
+
+There's plenty of problems with that one.
+
+```php
+namespace App;
+
+class MyClass
+{
+    private string $myProperty = 'this is the value';
+}
+```
+
+There should be no linting issues with that block.
+
+```js
+// This one's just here so that we can check the num counter works correctly.
+// This block should be skipped entirely.
+```
+
+```PHP
+class AnotherClass
+{
+    private string $anotherProperty = 'this is the value';
+}
+```
+
+And then that one should have linting failures again - note the capital PHP, which should still be picked up.
+
+For some reason this one doesn't get auto-fixed.
+
+- this one will be indented
+  ```php
+  class FinalClass {
+      private string $lastProperty='this is the value';
+  }
+  ```
+- And it'll also have linting failures
diff --git a/tests/fixtures/lint-but-no-problems.md b/tests/fixtures/lint-but-no-problems.md
new file mode 100644
index 0000000..5632da5
--- /dev/null
+++ b/tests/fixtures/lint-but-no-problems.md
@@ -0,0 +1,12 @@
+# This file has something to lint
+
+```php
+namespace App;
+
+class MyClass
+{
+    private string $myProperty = 'this is the value';
+}
+```
+
+There should be no linting issues with that block.
diff --git a/tests/fixtures/lint-with-problems.md b/tests/fixtures/lint-with-problems.md
new file mode 100644
index 0000000..9f65a03
--- /dev/null
+++ b/tests/fixtures/lint-with-problems.md
@@ -0,0 +1,43 @@
+# This file has something to lint
+
+```php
+class MyClass {
+    private string $myProperty='this is the value';
+}
+```
+
+There's plenty of problems with that one.
+
+```php
+namespace App;
+
+class MyClass
+{
+    private string $myProperty = 'this is the value';
+}
+```
+
+There should be no linting issues with that block.
+
+```js
+// This one's just here so that we can check the num counter works correctly.
+// This block should be skipped entirely.
+```
+
+```PHP
+class AnotherClass {
+    private string $anotherProperty='this is the value';
+}
+```
+
+And then that one should have linting failures again - note the capital PHP, which should still be picked up.
+
+For some reason this one doesn't get auto-fixed.
+
+- this one will be indented
+  ```php
+  class FinalClass {
+      private string $lastProperty='this is the value';
+  }
+  ```
+- And it'll also have linting failures
diff --git a/tests/fixtures/nothing-to-lint.md b/tests/fixtures/nothing-to-lint.md
new file mode 100644
index 0000000..bb9f7a4
--- /dev/null
+++ b/tests/fixtures/nothing-to-lint.md
@@ -0,0 +1,28 @@
+# Nothing to see here
+
+This file has no relevant code blocks.
+
+    class NothingHere
+    {
+        function($bad){exit(1);}
+    }
+
+^ That's a code block with PHP - but it won't be linted.
+
+And this one has PHP too, but no code hint so no linting
+
+```
+class NothingHere
+{
+    function($bad){exit(1);}
+}
+```
+
+This one has javascript and IT won't be linted either.
+
+```js
+class NothingHere
+{
+    function($bad){exit(1);}
+}
+```