code coverage (#299)

* install xdebug in web container, specify source directory for phpunit

* setup code coverage requirement in github actions

* change name of action

* document testing

* move minimum coverage to a file, fix dockerfile, more docs

Author: simonLeary42

.github/workflows/functional.yml

@@ -1,9 +1,12 @@
-name: phpunit
+name: docker-compose-phpunit
 
-on: [push]
+on:
+  pull_request:
+  push:
+    branches: [main]
 
 jobs:
-  phpunit:
+  docker-compose-phpunit:
     runs-on: ubuntu-latest
     steps:
     - uses: actions/checkout@v3
@@ -13,10 +16,12 @@ jobs:
       uses: shivammathur/setup-php@v2
       with:
         php-version: "8.3"
-        # php extensions also listed in tools/docker-dev/web/Dockerfile and README.md
-        extensions: curl,mysql,ldap,pdo,redis,intl
-        tools: composer:v2
-    - name: Install dependencies
-      run: composer install --prefer-dist --no-progress
-    - name: Run PHPUnit tests
-      run: vendor/bin/phpunit --colors=always --testsuite unit
+        tools: composer
+    - name: install composer dependencies
+      run: composer update
+    - name: Run docker compose
+      uses: hoverkraft-tech/compose-action@v2.0.1
+      with:
+        compose-file: "./tools/docker-dev/docker-compose.yml"
+    - name: Execute tests in the running services
+      run: docker compose -f ./tools/docker-dev/docker-compose.yml exec -w '/var/www/unity-web-portal' web bash -c 'XDEBUG_MODE=coverage ./vendor/bin/phpunit --coverage-clover="$(mktemp --suffix=.xml)" -d --min-coverage=./coverage.php'

.github/workflows/phpunit.yml

@@ -7,6 +7,7 @@
 * The maximum line length for any PHP file is 100 characters, instead of PSR-12's 120 characters.
 * Comments should be used sparingly.
 * Empty lines should be used sparingly.
+* No code should call `die()` or `exit()`, instead `UnitySite::die()`.
 
 This repository will automatically check PRs for linting compliance.
 
@@ -56,3 +57,52 @@ Notable users:
 ### Changes to Dev Environment
 
 Should the default schema of the web portal change, the `ldap/bootstrap.ldif` and `sql/bootstrap.sql` must be updated for the LDAP server and the MySQL server, respectively.
+
+## Testing
+
+Github Actions are used to execute all following tests for all pull requests.
+This means that if you're feeling lazy, you don't have to go out of your way to run tests, you just have to wait a bit longer for Github to do it for you.
+
+### `pre-commit`
+
+We use `pre-commit` for enforcing (and sometimes automatically fixing) the PSR-12 code standard, whitespace discrepancies, syntax validity, secret leak detection, and whatever else can be done quickly.
+`pre-commit` runs automatically every time you commit, assuming you set it up correctly.
+To save time, `pre-commit` only runs on the files with staged changes.
+To run on all files, use `pre-commit run --all-files`.
+
+### `phpunit`
+
+Since this codebase was not written with testing in mind, and this codebase makes extensive use of external resources such as SQL and LDAP, most of our testing does not focus on isolated "units", but high level functionality.
+Our functional tests pretend to make HTTP requests by modifying global variables in the same way that a production webserver would.
+This is preferred over directly calling library code because it helps to test the PHP logic in the webpages themselves, rather than just the internals.
+For example, one functional test would be to set `$_SERVER["REMOTE_USER"]` to authenticate as a user, `require "resources/init.php"` to setup the `$USER` global variable, set `$_POST["key"] = "ssh-rsa ..."` and `require "webroot/panel/account.php"` to make that user enter a new SSH key in the HTML form in the `account.php` page.
+Once a user action has been taken, internal interfaces are used to verify the results.
+
+To run `phpunit`, spawn 2 shells in differnt tabs:
+
+tab 1:
+```shell
+cd ./tools/docker-dev
+./build.sh
+./run.sh
+```
+
+tab 2:
+```
+$ container="$(docker container ls | grep web | awk '{print $1}')"
+$ docker exec "$container" -it bash
+> cd /var/www/unity-web
+> ./vendor/bin/phpunit /path/to/tests
+```
+
+For `/path/to/tests/`, you usually want `./test/functional/` but you can select a specific file to save time when troubleshooting specific tests.
+
+### code coverage
+
+`phpunit` has code coverage built in.
+It recommends the use of "strict code coverage", where every single test explicitly lists what functions it covers.
+That's a lot of work, so instead we accept what phpunit refers to as "risky unintentionally covered code".
+Using [robiningelbrecht/phpunit-coverage-tools](https://github.com/robiningelbrecht/phpunit-coverage-tools), our Github Actions testing will fail if the coverage falls below a certain percentage of lines of code.
+This percentage should be increased over time to just below whatever the current coverage is.
+
+To run a code coverage test, use the same procedure for phpunit but add these arguments: `--coverage-clover="$(mktemp --suffix=.xml)" -d --min-coverage=./coverage.php`

CONTRIBUTING.md

@@ -5,6 +5,7 @@
         "phpmailer/phpmailer": "6.6.4"
     },
     "require-dev": {
-        "phpunit/phpunit": "<12.1"
+        "phpunit/phpunit": "<12.1",
+        "robiningelbrecht/phpunit-coverage-tools": "<2"
     }
 }

composer.json

@@ -0,0 +1,11 @@
+<?php
+
+use RobinIngelbrecht\PHPUnitCoverageTools\MinCoverage\MinCoverageRule;
+
+return [
+    new MinCoverageRule(
+        pattern: '*',
+        minCoverage: 65,
+        exitOnLowCoverage: true
+    ),
+];

coverage.php

@@ -13,4 +13,14 @@
       <directory>test/functional</directory>
     </testsuite>
   </testsuites>
+  <source>
+    <include>
+      <directory suffix=".php">resources/lib</directory>
+    </include>
+  </source>
+  <extensions>
+    <bootstrap class="RobinIngelbrecht\PHPUnitCoverageTools\PhpUnitExtension">
+      <parameter name="cleanUpCloverXml" value="1"/>
+    </bootstrap>
+  </extensions>
 </phpunit>

phpunit.xml

@@ -15,7 +15,8 @@ RUN apt-get update && apt-get install -y \
     php-cli \
     php-mbstring \
     php-xml \
-    php-intl
+    php-intl \
+    php-xdebug
 COPY htpasswd /etc/apache2/.htpasswd
 RUN chown www-data /etc/apache2/.htpasswd
 COPY unity-apache.conf /etc/apache2/sites-available/unity.conf
@@ -27,6 +28,7 @@ RUN sed -i '/display_errors/c\display_errors = on' /etc/php/8.3/apache2/php.ini
 RUN sed -i '/zend.assertions/c\zend.assertions = 1' /etc/php/8.3/apache2/php.ini
 RUN sed -i '/zend.assertions/c\zend.assertions = 1' /etc/php/8.3/cli/php.ini
 RUN sed -i '/memory_limit/c\memory_limit = -1' /etc/php/8.3/apache2/php.ini
+RUN echo 'xdebug.mode=coverage' >> /etc/php/8.3/cli/php.ini
 
 # Start apache2 server
 EXPOSE 80