Keep only one directory for tests (#17257)

* Keep only one directory for tests * Fix namespaces * Make the CronTask::testCronTemp() test more resilient * Remove remaining usages of the tests/unit dir * Remove unit from coverage, add codecov token --------- Co-authored-by: Cédric Anne <[email protected]>
glpi-project · Jun 6, 2024 · a63aa66 · a63aa66
1 parent f906d0c
commit a63aa66
Show file tree

Hide file tree

Showing 76 changed files with 775 additions and 1,131 deletions.
diff --git a/.github/actions/test_tests-cache.sh b/.github/actions/test_tests-cache.sh
@@ -7,17 +7,6 @@ for CONFIG in {"--use-default","--dsn=memcached://memcached","--dsn=redis://redi
   php bin/console cache:configure \
     --config-dir=./tests/config --ansi --no-interaction \
     $CONFIG
-  vendor/bin/atoum \
-    -p 'php -d memory_limit=512M' \
-    --debug \
-    --force-terminal \
-    --use-dot-report \
-    --bootstrap-file tests/bootstrap.php \
-    --fail-if-void-methods \
-    --fail-if-skipped-methods \
-    --no-code-coverage \
-    -d tests/units \
-    -t cache
   vendor/bin/atoum \
     -p 'php -d memory_limit=512M' \
     --debug \

diff --git a/.github/actions/test_tests-units.sh b/.github/actions/test_tests-units.sh
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -170,10 +170,6 @@ jobs:
         run: |
           .github/actions/init_initialize-9.5-db.sh
           docker compose exec -T app .github/actions/test_update-from-9.5.sh
-      - name: "Unit tests"
-        if: env.skip != 'true'
-        run: |
-          docker compose exec -T app .github/actions/test_tests-units.sh
       - name: "Functional tests"
         if: env.skip != 'true'
         run: |

diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml
@@ -59,9 +59,6 @@ jobs:
       - name: "Install database"
         run: |
           docker compose exec -T app .github/actions/test_install.sh
-      - name: "Unit tests"
-        run: |
-          docker compose exec -T app .github/actions/test_tests-units.sh
       - name: "Functional tests"
         run: |
           docker compose exec -T app .github/actions/test_tests-functional.sh
@@ -75,6 +72,8 @@ jobs:
           docker compose exec -T app .github/actions/test_tests-imap.sh
       - name: "Codecov"
         uses: "codecov/codecov-action@v4"
+        env:
+          CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
         with:
-          files: ./tests/coverage-unit/clover.xml,./tests/coverage-functional/clover.xml,./tests/coverage-ldap/clover.xml,./tests/coverage-imap/clover.xml
+          files: ./tests/coverage-functional/clover.xml,./tests/coverage-ldap/clover.xml,./tests/coverage-imap/clover.xml
           override_branch: ${{ matrix.branch }}
diff --git a/composer.json b/composer.json
@@ -132,7 +132,6 @@
     "scripts": {
         "testdb": "atoum -p 'php -d memory_limit=512M' --debug --force-terminal --use-dot-report --bootstrap-file tests/bootstrap.php --no-code-coverage --max-children-number 1 -d tests/database",
         "testfunc": "atoum -p 'php -d memory_limit=512M' --debug --force-terminal --use-dot-report --bootstrap-file tests/bootstrap.php --no-code-coverage --max-children-number 1 -d tests/functional/",
-        "testunits": "atoum -p 'php -d memory_limit=512M' --debug --force-terminal --use-dot-report --bootstrap-file tests/bootstrap.php --no-code-coverage -d tests/units",
         "testweb": "atoum -p 'php -d memory_limit=512M' --debug --force-terminal --use-dot-report --bootstrap-file tests/bootstrap.php --no-code-coverage --max-children-number 1 -d tests/web",
         "testldap": "atoum -p 'php -d memory_limit=512M' --debug --force-terminal --use-dot-report --bootstrap-file tests/bootstrap.php --no-code-coverage --max-children-number 1 -d tests/LDAP",
         "testimap": "atoum -p 'php -d memory_limit=512M' --debug --force-terminal --use-dot-report --bootstrap-file tests/bootstrap.php --no-code-coverage --max-children-number 1 -d tests/imap",

diff --git a/tests/README.md b/tests/README.md
@@ -49,20 +49,19 @@ Running the test suite on developpement env
 -------------------------------------------
 
 There are multiple directories for tests:
-- `tests/units` for unit tests;
-- `tests/functional` for functional tests;
+- `tests/functional` for unit and functional tests;
 - `tests/imap` for Mail collector tests;
 - `tests/LDAP` for LDAP connection tests;
 - `tests/web` for API tests.
 
 You can choose to run tests on a whole directory, on any file, or on any \<class::method>. You have to specify a bootstrap file each time:
 
 ```bash
-$ atoum -bf tests/bootstrap.php -mcn 1 -d tests/units/
+$ atoum -bf tests/bootstrap.php -mcn 1 -d tests/functional/
 [...]
-$ atoum -bf tests/bootstrap.php -f tests/units/Html.php
+$ atoum -bf tests/bootstrap.php -f tests/functional/Html.php
 [...]
-$ atoum -bf tests/bootstrap.php -f tests/units/Html.php -m tests\units\Html::testConvDateTime
+$ atoum -bf tests/bootstrap.php -f tests/functional/Html.php -m tests\units\Html::testConvDateTime
 ```
 In `tests\units\Html::testConvDateTime`, you may need to double the backslashes (depending on the shell you use);
 

diff --git a/tests/bootstrap.php b/tests/bootstrap.php
@@ -108,7 +108,7 @@
 include_once __DIR__ . '/InventoryTestCase.php';
 include_once __DIR__ . '/functional/CommonITILRecurrent.php';
 include_once __DIR__ . '/functional/Glpi/ContentTemplates/Parameters/AbstractParameters.php';
-include_once __DIR__ . '/units/AbstractRightsDropdown.php';
+include_once __DIR__ . '/functional/AbstractRightsDropdown.php';
 
 // check folder exists instead of class_exists('\GuzzleHttp\Client'), to prevent global includes
 if (file_exists(__DIR__ . '/../vendor/autoload.php') && !file_exists(__DIR__ . '/../vendor/guzzlehttp/guzzle')) {

diff --git a/tests/units/AbstractRightsDropdown.php → tests/functional/AbstractRightsDropdown.php b/tests/units/AbstractRightsDropdown.php → tests/functional/AbstractRightsDropdown.php
diff --git a/tests/units/Autoload.php → tests/functional/Autoload.php b/tests/units/Autoload.php → tests/functional/Autoload.php
diff --git a/tests/functional/CronTask.php b/tests/functional/CronTask.php
@@ -36,11 +36,63 @@
 namespace tests\units;
 
 use DbTestCase;
+use RecursiveDirectoryIterator;
+use RecursiveIteratorIterator;
 
 /* Test for inc/crontask.class.php */
 
 class CronTask extends DbTestCase
 {
+    public function testCronTemp()
+    {
+        //create some files
+        $filenames = [
+            'recent_file.txt',
+            'file1.txt',
+            'file2.txt',
+            'file3.txt',
+            'file4.txt',
+        ];
+        foreach ($filenames as $filename) {
+            $this->variable(file_put_contents(GLPI_TMP_DIR . '/' . $filename, bin2hex(random_bytes(20))))->isNotFalse();
+        }
+
+        //create auto_orient directory
+        if (!file_exists(GLPI_TMP_DIR . '/auto_orient/')) {
+            $this->boolean(mkdir(GLPI_TMP_DIR . '/auto_orient/', 0755, true))->isTrue();
+        }
+
+        $tmp_dir_iterator = new RecursiveIteratorIterator(
+            new RecursiveDirectoryIterator(GLPI_TMP_DIR, RecursiveDirectoryIterator::SKIP_DOTS),
+            RecursiveIteratorIterator::CHILD_FIRST
+        );
+        foreach ($tmp_dir_iterator as $path) {
+            if (basename($path) !== 'recent_file.txt') {
+                // change the modification date of the file to make it considered as "not recent"
+                $this->boolean(touch($path, time() - (HOUR_TIMESTAMP * 2)))->isTrue();
+            }
+        }
+
+        // launch Cron for cleaning _tmp directory
+        $mode = - \CronTask::MODE_EXTERNAL; // force
+        \CronTask::launch($mode, 5, 'temp');
+
+        $nb_file = $this->getFileCountRecursively(GLPI_TMP_DIR);
+        $this->variable($nb_file)->isEqualTo(1); // only recent_file.txt should be preserved
+    }
+
+
+    public function getFileCountRecursively($path)
+    {
+
+        $dir = new \RecursiveDirectoryIterator($path, \RecursiveDirectoryIterator::SKIP_DOTS);
+        $files = new \RecursiveIteratorIterator(
+            $dir,
+            \RecursiveIteratorIterator::CHILD_FIRST
+        );
+        return iterator_count($files);
+    }
+
     protected function registerProvider()
     {
         return [

diff --git a/tests/units/DB.php → tests/functional/DB.php b/tests/units/DB.php → tests/functional/DB.php
diff --git a/tests/units/DBConnection.php → tests/functional/DBConnection.php b/tests/units/DBConnection.php → tests/functional/DBConnection.php
diff --git a/tests/units/DBmysqlIterator.php → tests/functional/DBmysqlIterator.php b/tests/units/DBmysqlIterator.php → tests/functional/DBmysqlIterator.php
diff --git a/tests/units/GLPI.php → tests/functional/GLPI.php b/tests/units/GLPI.php → tests/functional/GLPI.php
diff --git a/tests/units/GLPIMailer.php → tests/functional/GLPIMailer.php b/tests/units/GLPIMailer.php → tests/functional/GLPIMailer.php
diff --git a/...s/units/Glpi/Application/ErrorHandler.php → ...ctional/Glpi/Application/ErrorHandler.php b/...s/units/Glpi/Application/ErrorHandler.php → ...ctional/Glpi/Application/ErrorHandler.php
diff --git a/...iew/Extension/FrontEndAssetsExtension.php → ...iew/Extension/FrontEndAssetsExtension.php b/...iew/Extension/FrontEndAssetsExtension.php → ...iew/Extension/FrontEndAssetsExtension.php
diff --git a/tests/units/Glpi/Cache/CacheManager.php → tests/functional/Glpi/Cache/CacheManager.php b/tests/units/Glpi/Cache/CacheManager.php → tests/functional/Glpi/Cache/CacheManager.php
diff --git a/tests/units/Glpi/Cache/SimpleCache.php → tests/functional/Glpi/Cache/SimpleCache.php b/tests/units/Glpi/Cache/SimpleCache.php → tests/functional/Glpi/Cache/SimpleCache.php
diff --git a/tests/units/Glpi/Console/CommandLoader.php → ...functional/Glpi/Console/CommandLoader.php b/tests/units/Glpi/Console/CommandLoader.php → ...functional/Glpi/Console/CommandLoader.php
diff --git a/tests/functional/Glpi/ContentTemplates/TemplateManager.php b/tests/functional/Glpi/ContentTemplates/TemplateManager.php
@@ -47,6 +47,131 @@
  */
 class TemplateManager extends DbTestCase
 {
+    protected function testTemplatesProvider(): array
+    {
+        return [
+            [
+                'content'   => "{{ test_var }}",
+                'params'    => ['test_var' => 'test_value'],
+                'expected'  => "<p>test_value</p>",
+            ],
+            [
+                'content'   => "Test var: {{ test_var }}",
+                'params'    => ['test_var' => 'test_value'],
+                'expected'  => "<p>Test var: test_value</p>",
+            ],
+            [
+                'content'   => "Test condition: {% if test_condition == true %}TRUE{% else %}FALSE{% endif %}",
+                'params'    => ['test_condition' => 'true'],
+                'expected'  => "<p>Test condition: TRUE</p>",
+            ],
+            [
+                'content'   => "Test condition: {% if test_condition == true %}TRUE{% else %}FALSE{% endif %}",
+                'params'    => ['test_condition' => 'false'],
+                'expected'  => "<p>Test condition: TRUE</p>",
+            ],
+            [
+                'content'   => "Test for: {% for item in items %}{{ item }} {% else %}no items{% endfor %}",
+                'params'    => ['items' => ['a', 'b', 'c', 'd', 'e']],
+                'expected'  => "<p>Test for: a b c d e </p>",
+            ],
+            [
+                'content'   => "Test for: {% for item in items %}{{ item }} {% else %}no items{% endfor %}",
+                'params'    => ['items' => []],
+                'expected'  => "<p>Test for: no items</p>",
+            ],
+            [
+                'content'   => "Test forbidden tag: {% set var = 'value' %}",
+                'params'    => [],
+                'expected'  => "",
+                'error'     => 'Invalid twig template (Tag "set" is not allowed in "template" at line 1.)',
+            ],
+            [
+                'content'   => "Test syntax error {{",
+                'params'    => [],
+                'expected'  => "",
+                'error'     => 'Invalid twig template syntax',
+            ],
+            [
+                'content'   => '&#60;h1&#62;Test sanitized template&#60;/h1&#62;&#60;hr /&#62;{{content|raw}}',
+                'params'    => ['content' => '<p>Item content</p>'],
+                'expected'  => '<h1>Test sanitized template</h1><hr /><p>Item content</p>',
+                'error'     => null,
+            ],
+            [
+                'content'   => '&#60;h1&#62;Test sanitized template 2&#60;/h1&#62;&#60;hr /&#62;{{content|raw}}',
+                'params'    => ['content' => 'Item content should not be unsanitized: &#60;--'],
+                'expected'  => '<h1>Test sanitized template 2</h1><hr />Item content should not be unsanitized: &#60;--',
+                'error'     => null,
+            ],
+            [
+                'content'   => "&#60;p&#62;Test sanitized template {% if count &#62; 5 %}&#60;b&#62;++&#60;/b&#62;{% endif %}&#60;/p&#62;",
+                'params'    => ['count' => 25],
+                'expected'  => "<p>Test sanitized template <b>++</b></p>",
+                'error'     => null,
+            ],
+            [
+                'content'   => '&#60;h1 onclick="alert(1);"&#62;Test safe HTML2&#60;/h1&#62;&#60;hr /&#62;{{content|raw}}',
+                'params'    => ['content' => 'Fill this form:<iframe src="phishing.php"></iframe>'],
+                'expected'  => '<h1>Test safe HTML2</h1><hr />Fill this form:',
+                'error'     => null,
+            ],
+        ];
+    }
+
+    /**
+     * @dataProvider testTemplatesProvider
+     */
+    public function testRender(
+        string $content,
+        array $params,
+        string $expected,
+        ?string $error = null
+    ): void {
+        $manager = $this->newTestedInstance();
+
+        $html = null;
+
+        if ($error !== null) {
+            $this->exception(
+                function () use ($manager, $content, $params, &$html) {
+                    $html = $manager->render($content, $params);
+                }
+            );
+            return;
+        } else {
+            $html = $manager->render($content, $params);
+        }
+
+        $this->string($html)->isEqualTo($expected);
+    }
+
+    /**
+     * @dataProvider testTemplatesProvider
+     */
+    public function testValidate(
+        string $content,
+        array $params,
+        string $expected,
+        ?string $error = null
+    ): void {
+        $manager = $this->newTestedInstance();
+        $err_msg = null;
+        $is_valid = $manager->validate($content, $err_msg);
+        $this->boolean($is_valid)->isEqualTo(empty($error));
+
+       // Handle error if neeced
+        if ($error !== null) {
+            $this->string($err_msg)->contains($error);
+        }
+    }
+
+    public function testGetSecurityPolicy(): void
+    {
+       // Not much to test here, maybe keepk this for code coverage ?
+        $manager = $this->newTestedInstance();
+        $this->object($manager->getSecurityPolicy())->isInstanceOf(\Twig\Sandbox\SecurityPolicy::class);
+    }
     /**
      * Get all possible CommonITILObject classes.
      *

diff --git a/tests/units/Glpi/Csv/LogCsvExport.php → tests/functional/Glpi/Csv/LogCsvExport.php b/tests/units/Glpi/Csv/LogCsvExport.php → tests/functional/Glpi/Csv/LogCsvExport.php