Skip to content
Advertisement

PHPUnit tests isolation and how to write a test where expected value depends on some value in method that changes often

So I am coding in PHP and am writing tests with PHPUnit. I have this method convertCashAmountToMainCurrency that i want to test:

public static function convertCashAmountToMainCurrency(string $currency, float $amount): float
{
    $feeInMainCurrency = $amount / Constants::CURRENCIES[$currency][Constants::RATE];

    return self::roundUp($feeInMainCurrency, Constants::CURRENCIES[Constants::MAIN_CURRENCY][Constants::PRECISION]);
}

public static function roundUp(float $value, int $precision): float
{
    $pow = pow(10, $precision);

    return (ceil($pow * $value) + ceil($pow * $value - ceil($pow * $value))) / $pow;
}

And i have the Constants that are being used here:

public const RATE = 'RATE';
public const PRECISION = 'PRECISION';

public const CURRENCY_EUR = 'EUR';
public const CURRENCY_USD = 'USD';
public const CURRENCY_JPY = 'JPY';

public const MAIN_CURRENCY = self::CURRENCY_EUR;

public const CURRENCIES = [
    self::CURRENCY_EUR => [
        self::RATE => 1,
        self::PRECISION => 2,
    ],
    self::CURRENCY_USD => [
        self::RATE => 1.1497,
        self::PRECISION => 2,
    ],
    self::CURRENCY_JPY => [
        self::RATE => 129.53,
        self::PRECISION => 0,
    ],
];

I am testing the method like this:

/**
 * @param string $currency
 * @param float $amount
 * @param float $expectation
 *
 * @runInSeparateProcess
 * @preserveGlobalState disabled
 * @dataProvider dataProviderForConvertCashAmountToMainCurrencyTesting
 */
public function testConvertCashAmountToMainCurrency(string $currency, float $amount, float $expectation)
{
    $this->assertEquals(
        $expectation,
        Math::convertCashAmountToMainCurrency($currency, $amount)
    );
}

public function dataProviderForConvertCashAmountToMainCurrencyTesting(): array
{
    return [
        'convert EUR to main currency' => [Constants::CURRENCY_EUR, 100.01, 100.01],
        'convert USD to main currency' => [Constants::CURRENCY_USD, 100.01, 86.99],
        'convert JPY to main currency' => [Constants::CURRENCY_JPY, 10001, 77.21],
    ];
}

And the tests passes just fine when the expected value and the currency rate are stated in fixed size. But the problem is that i need them to pass every time, regardless of the currency convertion rate value, so i would not need to rewrite the tests every time the rate changes. Because now, if i change the RATE (or even the PRECISION, for instance) value, the tests will not pass due to assertion failure.

Since I was told that I need to isolate my tests to solve this issue, can anyone confirm this and get me on a right path with solving the issue? Thanks in advance for any help!

Advertisement

Answer

The problem is that convertCashAmountToMainCurrency is relying on global state. There are ways to handle this in tests, but to me that’s not good practice. My suggestion is to extract the conversion into a new class that gets all required information passed in. That way it does not depend on global state and can be tested easily in isolation.

This could look something like this:

class CurrencyConverter
{
    private string $mainCurrency;

    /**
     * @var array<string, CurrencyConfig>
     */
    private array $currencies;

    /**
     * @param array<string, CurrencyConfig> $currencies
     */
    public function __construct(string $mainCurrency, array $currencies)
    {
        Assert::keyExists($currencies, $mainCurrency);

        $this->mainCurrency = $mainCurrency;
        $this->currencies   = $currencies;
    }

    public function toMainCurrency(string $currency, float $amount): float
    {
        Assert::keyExists($this->currencies, $currency);

        $feeInMainCurrency = $amount / $this->currencies[$currency]->rate();

        return self::roundUp($feeInMainCurrency, $this->currencies[$this->mainCurrency]->precision());
    }

    private static function roundUp(float $value, int $precision): float
    {
        $pow = pow(10, $precision);

        return (ceil($pow * $value) + ceil($pow * $value - ceil($pow * $value))) / $pow;
    }
}
class CurrencyConfig
{
    private float $rate;

    private int $precision;

    public function __construct(float $rate, int $precision)
    {
        Assert::greaterThan($rate, 0);
        Assert::greaterThanEq($precision, 0);

        $this->rate      = $rate;
        $this->precision = $precision;
    }

    public function rate(): float
    {
        return $this->rate;
    }

    public function precision(): int
    {
        return $this->precision;
    }
}
    /**
     * @param string $currency
     * @param float  $amount
     * @param float  $expectation
     *
     * @runInSeparateProcess
     * @preserveGlobalState disabled
     * @dataProvider        dataProviderForConvertCashAmountToMainCurrencyTesting
     */
    public function testConvertCashAmountToMainCurrency(string $currency, float $amount, float $expectation)
    {
        $converter = new CurrencyConverter(
            Constants::CURRENCY_EUR,
            [
                Constants::CURRENCY_EUR => new CurrencyConfig(1, 2),
                Constants::CURRENCY_USD => new CurrencyConfig(1.1497, 2),
                Constants::CURRENCY_JPY => new CurrencyConfig(129.53, 0),
            ]
        );

        self::assertEquals($expectation, $converter->toMainCurrency($currency, $amount));
    }

If you can’t get rid of the existing static method, just create a converter with the configuration from the global state.

class Math
{
    public static function convertCashAmountToMainCurrency(string $currency, float $amount): float
    {
        $converter = new CurrencyConverter(
            Constants::MAIN_CURRENCY,
            array_map(
                fn(array $item) => new CurrencyConfig($item[Constants::RATE], $item[Constants::PRECISION]),
                Constants::CURRENCIES
            )
        );

        return $converter->toMainCurrency($currency, $amount);
    }
}
User contributions licensed under: CC BY-SA
2 People found this is helpful
Advertisement