Skip to content

Commit

Permalink
Introduce initial unit test setup
Browse files Browse the repository at this point in the history
This commit creates a basic unit test setup which runs the exact same
tests as the original test script does, but uses the de-facto testing
framework in the PHP world: [PHPUnit](https://phpunit.de/).

Using this test framework will make it more inviting and more
straight-forward for contributors to add additional tests, either by
adding new test classes or by adding additional test methods to the
existing test class.

It will also allow for insight into the code coverage via tests, which
could help inform for which code additional tests should be added.

To run the tests, run:
```bash
composer update
composer unit-test
```

Notes:
* The test setup as currently created will allow for these tests to be run
    on PHP 5.3 to current (PHP 8.2 at the time of writing).
    Testing on PHP 5.0 - 5.2 is not supported in this setup and cannot be
    run via CI anyway.
    I've chosen not to spend time on getting that running as I can barely
    imagine anyone still using PHP 5.0 - 5.2 anyway.
* PHPUnit has introduced quite some breaking changes across PHPUnit
    versions over the years.
    The basic functionality I've used in the current tests is not affected
    by these.
    If at some point tests would be added which would be affected by this,
    I'd recommend adding a dependency on the
    [PHPUnit Polyfills](https://github.com/Yoast/PHPUnit-Polyfills)
    package, which can mitigate this.
* The configuration for the tests is in the `phpunit.xml.dist` file.
    The configuration has been set up to allow for seeing code coverage
    information providing [Xdebug](https://xdebug.org/) or PCOV is
    available.
    The configuration has also been set up to ensure the tests are run
    using the same configuration across PHPUnit versions by being explicit
    about various configuration settings, for which the default value has
    changed across PHPUnit versions.
* Contributors can choose to run tests locally with an adjusted
    (stricter) configuration if they so choose by adding a `phpunit.xml`
    file, which overrules the `phpunit.xml.dist` file.
    To allow for this, I've added an entry to the `.gitignore` file to
    ignore `phpunit.xml` files as those should not be committed to
    the repo.
* As of PHPUnit 8.x, PHPUnit generates a cache file. This file should also
    not be committed to the repo and has been added to the `.gitignore`
    file.
* I've included scripts in the `composer.json` file to run the (unit)
    tests.
* I've chosen - for now - not to remove the old `test.php` file, nor to
    remove the GH Actions workflow job for running them.
    Instead, I've chosen to _add_ a new test job to the GH Actions workflow
    to run the unit tests separately.

Refs:
* https://phpunit.de/supported-versions.html
* https://docs.phpunit.de/en/9.6/
  • Loading branch information
jrfnl authored and solardiz committed Mar 23, 2023
1 parent 75db62e commit 383b8b5
Show file tree
Hide file tree
Showing 5 changed files with 247 additions and 0 deletions.
47 changes: 47 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,50 @@ jobs:

- name: Run tests
run: php ./tests/test.php

unittest:
runs-on: ubuntu-latest

strategy:
matrix:
php: ['5.3', '5.4', '5.5', '5.6', '7.0', '7.1', '7.2', '7.3', '7.4', '8.0', '8.1', '8.2', '8.3']

continue-on-error: ${{ matrix.php == '8.3' }}

name: "Unit Test: PHP ${{ matrix.php }}"

steps:
- name: Checkout code
uses: actions/checkout@v3

- name: Install PHP
uses: shivammathur/setup-php@v2
with:
php-version: ${{ matrix.php }}
ini-values: zend.assertions=1, error_reporting=-1, display_errors=On
coverage: xdebug

# Install dependencies and handle caching in one go.
# @link https://github.com/marketplace/actions/install-composer-dependencies
- name: Install Composer dependencies - normal
if: ${{ matrix.php != '8.3' }}
uses: "ramsey/composer-install@v2"
with:
# Bust the cache at least once a month - output format: YYYY-MM.
custom-cache-suffix: $(date -u "+%Y-%m")

- name: Install Composer dependencies - ignore PHP restrictions
if: ${{ matrix.php == '8.3' }}
uses: "ramsey/composer-install@v2"
with:
composer-options: --ignore-platform-req=php+
# Bust the cache at least once a month - output format: YYYY-MM.
custom-cache-suffix: $(date -u "+%Y-%m")

# PHPUnit 10 will fail a test run when the "old" configuration format is used.
# Luckily, there is a build-in migration tool since PHPUnit 9.3.
- name: Migrate PHPUnit configuration for PHPUnit 9.3+
run: vendor/bin/phpunit --migrate-configuration || echo '--migrate-configuration not available'

- name: Run the unit tests
run: vendor/bin/phpunit
2 changes: 2 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,2 +1,4 @@
composer.lock
vendor/
phpunit.xml
/.phpunit.result.cache
14 changes: 14 additions & 0 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,24 @@
"require": {
"php": ">=5.0"
},
"require-dev": {
"phpunit/phpunit": "^4.8.36 || ^5.7.21 || ^6.0 || ^7.0 || ^8.0 || ^9.0 || ^10.0"
},
"autoload": {
"psr-0": { "PasswordHash": "src/" }
},
"autoload-dev" : {
"psr-4": {
"Openwall\\PHPass\\Tests\\": "tests/unit/"
}
},
"scripts": {
"unit-test": [
"@php ./vendor/phpunit/phpunit/phpunit --no-coverage"
],
"coverage": [
"@php ./vendor/phpunit/phpunit/phpunit"
],
"test": [
"@php ./tests/test.php"
]
Expand Down
31 changes: 31 additions & 0 deletions phpunit.xml.dist
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
<?xml version="1.0" encoding="UTF-8"?>
<phpunit
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:noNamespaceSchemaLocation="https://schema.phpunit.de/8.5/phpunit.xsd"
backupGlobals="true"
bootstrap="vendor/autoload.php"
beStrictAboutTestsThatDoNotTestAnything="true"
convertErrorsToExceptions="true"
convertWarningsToExceptions="true"
convertNoticesToExceptions="true"
convertDeprecationsToExceptions="true"
verbose="true"
colors="true"
forceCoversAnnotation="true"
>
<testsuites>
<testsuite name="PHPassTests">
<directory>./tests/unit/</directory>
</testsuite>
</testsuites>

<filter>
<whitelist addUncoveredFilesFromWhitelist="true">
<directory suffix=".php">./src</directory>
</whitelist>
</filter>

<logging>
<log type="coverage-text" target="php://stdout" showUncoveredFiles="true"/>
</logging>
</phpunit>
153 changes: 153 additions & 0 deletions tests/unit/PasswordHashEndToEndTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,153 @@
<?php

namespace Openwall\PHPass\Tests;

use PasswordHash;
use PHPUnit\Framework\TestCase;

/**
* Basic end-to-end tests for the PasswordHash class.
*
* @covers \PasswordHash
*/
final class PasswordHashEndToEndTest extends TestCase {

/**
* Test using the stronger but system-specific hashes, with a possible fallback to
* the weaker portable hashes.
*
* @dataProvider dataSets
*
* @param string $input The text to hash and compare with.
*
* @return void
*/
public function testStrongerSystemSpecificHashSuccess($input) {
$t_hasher = new PasswordHash(8, FALSE);
$hash = $t_hasher->HashPassword($input);

$this->assertTrue($t_hasher->CheckPassword($input, $hash));
}

/**
* Test using the stronger but system-specific hashes, with a possible fallback to
* the weaker portable hashes.
*
* @dataProvider dataSets
*
* @param string $input The text to hash.
* @param string $compare The text to compare the hash with.
*
* @return void
*/
public function testStrongerSystemSpecificHashFail($input, $compare) {
$t_hasher = new PasswordHash(8, FALSE);
$hash = $t_hasher->HashPassword($input);

$this->assertFalse($t_hasher->CheckPassword($compare, $hash));
}

/**
* Test using the weaker portable hashes.
*
* @dataProvider dataSets
*
* @param string $input The text to hash and compare with.
*
* @return void
*/
public function testWeakerPortableHashSuccess($input) {
# Force the use of weaker portable hashes.
$t_hasher = new PasswordHash(8, TRUE);
$hash = $t_hasher->HashPassword($input);

$this->assertTrue($t_hasher->CheckPassword($input, $hash));
}

/**
* Test using the weaker portable hashes.
*
* @dataProvider dataSets
*
* @param string $input The text to hash.
* @param string $compare The text to compare the hash with.
*
* @return void
*/
public function testWeakerPortableHashFail($input, $compare) {
# Force the use of weaker portable hashes.
$t_hasher = new PasswordHash(8, TRUE);
$hash = $t_hasher->HashPassword($input);

$this->assertFalse($t_hasher->CheckPassword($compare, $hash));
}

/**
* Data provider.
*
* @return array
*/
public static function dataSets() {
return array(
'initial test case' => array(
'input' => 'test12345',
'compare' => 'test12346',
),
);
}

/**
* Test the generated hash is correctly calculated using the weaker portable hashes.
*
* @dataProvider dataGeneratedHash
*
* @param string $expected_hash The expected password hash output.
* @param string $input The text to hash and compare with.
*
* @return void
*/
public function testGeneratedHashSuccess($expected_hash, $input) {
$t_hasher = new PasswordHash(8, TRUE);

$this->assertTrue($t_hasher->CheckPassword($input, $expected_hash));
}

/**
* Test the generated hash is correctly calculated using the weaker portable hashes.
*
* @dataProvider dataGeneratedHash
*
* @param string $expected_hash The expected password hash output.
* @param string $input Unused.
* @param string $compare The text to compare the hash with.
*
* @return void
*/
public function testGeneratedHashFail($expected_hash, $input, $compare) {
$t_hasher = new PasswordHash(8, TRUE);

$this->assertFalse($t_hasher->CheckPassword($compare, $expected_hash));
}

/**
* Data provider.
*
* @return array
*/
public static function dataGeneratedHash() {
return array(
'initial test case' => array(
/*
* A correct portable hash for 'test12345'.
* Please note the use of single quotes to ensure that the dollar signs will
* be interpreted literally. Of course, a real application making use of the
* framework won't store password hashes within a PHP source file anyway.
* We only do this for testing.
*/
'expected_hash' => '$P$9IQRaTwmfeRo7ud9Fh4E2PdI0S3r.L0',
'input' => 'test12345',
'compare' => 'test12346',
),
);
}
}

0 comments on commit 383b8b5

Please sign in to comment.