H
H
HiDiv2021-12-30 11:26:12
PHP
HiDiv, 2021-12-30 11:26:12

How to correctly cover the code given in the example with tests?

I do not need to be convinced that autotests are needed during development, I have already done this for myself.
The main question is how to write an autotest so that it is a help, and not a burden?
I have already read a lot of articles and watched a number of videos about autotests, but it was all either purely theoretical material,
or more “academic” examples were dealt with there ...

Actually, I have a specific task (actually a simplified version of a real task), with which I can not cope and ask for help.

You need to write a function / class that would receive some text parameter $paramX as input (in a real task, there are several) and check the presence of records in the database table that satisfy a condition that depends on $paramX.
If there is at least one such entry, then return True, otherwise (if there is none) False.

Here is an example of my implementation.

Implementation code

class GetSomeValue
{
    protected $paramX;
    protected $db;
    protected $timedate;

    public function __construct(string $paramX)
    {
        $this->paramX = $paramX;

        // Инстанс от фреймворка для работы с БД
        $this->db = DBManager::getInstance();
        // Инстанс от фреймворка для работы с датой и временем
        $this->timedate = TimeDate::getInstance();
    }

    public function getValue(): bool
    {
        $row = $this->getDbData($this->getCompleteSql());
        return !empty($row);
    }

    /**
     * Вынес в отдельную функцию и реализовал такую передачу параметров, чтобы можно было замокать
     * @param string $sql
     * @return array|false Если данные не найдены или ошибка выполнения запроса, то false, иначе массив с данными одной записи
     */
    protected function getDbData(string $sql)
    {
        return $this->db->fetchOne($sql);
    }

    protected function getCompleteSql(): string
    {
        return strtr($this->getSql(), $this->getSqlParams());
    }

    protected function getSql(): string
    {
        return "
            SELECT
                'x'
            FROM
                table_1
            WHERE
                field1 = 'CONST1' AND
                field2 = <paramX> AND
                date_open <= <curDate> AND
                date_close IS NULL
        ";
    }

    protected function getSqlParams(): array
    {
        $params = [
            '<paramX>' => $this->paramX,
            '<curDate>' => $this->getCurDate(),
        ];
        return array_map([$this->db, 'quoted'], $params);
    }

    /**
     * Вынес в отдельную функцию, чтобы можно было замокать
     * @return string Дата в формате БД yyyy-mm-dd
     */
    protected function getCurDate(): string
    {
        return $this->timedate->getNowDbDate();
    }
}



As far as I understand, a unit test should test exactly one function/method (or even part of it) without interacting with other entities. If so, then the only thing to check is getValue.
By locking getDbData and getCompleteSql, you can return either an array or false, and check what will be the output.
Of course, I do not have much experience in autotests, but in my opinion it is not a very "informative" test, if we consider the whole task as a whole.

Whether it is necessary to test separately protected methods, I did not understand. If you still need it, then probably in each test you will have to generate a new class in which to increase the visibility of the method to public and only then test it. True, then I don’t really understand what the test, for example, getSql will give? Just to check that the same constant is returned? After all, checking for syntactic or "logical" correctness of the sql query will not work here ...

On the other hand, you can lock only getCurDate and getDbData and then check the final sql query (though only visually) and the reaction to the pseudo-response from the database. True, this already turns out (IMHO) not a unit test, but rather an integration test, but here I see a clear profit. Moreover, almost all components of the class will be checked, with the exception of the hacked ones. However, the downside is that the test can also break down anywhere, and again, we do not check the syntactic or "logical" correctness of the sql query in any way ...

And, finally, you can fill the database table with test data for each test, or do this once and save the dump for a later sweep. Then we can check everything. True, in order to really check all the boundary options (to sort through them), you will have to perform many tests. It could be, for example, like this:

  • no data in the table at all.
  • there is data, but not matching "static conditions" (constant or paramX).
  • if one entry matches "dynamic conditions" (dates).
  • there are several entries matching "dynamic conditions" (dates).


Perhaps this list can be shortened or expanded, but all combinations will have to be sorted out, and there will be few of them ...

Until recently, when writing tests, I always followed the last option. It turned out good coverage,
but the number of tests is growing like mushrooms after rain. Minus, when making more or less significant changes to the implementation, the tests break in batches. True, they are also repaired in batches, but it seems to me that this is not entirely correct. I feel like I'm doing something wrong!

I appeal to you Guru autotests! Tell me what I'm wrong and show on this example what I'm doing wrong!

Actually the first question. Is the implementation of the task itself correct in terms of its further coverage by autotests? If No, what exactly and how should be changed (preferably with an example),

Question two. How to properly cover the given (or modified) code with autotests? I would also very much like examples and at least minimal explanations for them.

Thanks in advance for any help!

PS: I understand that all of you are busy people, but if you decide to help, please write your answers more or less in detail! No need to write one phrase "Everything is wrong" or "You are hopeless."

Answer the question

In order to leave comments, you need to log in

2 answer(s)
I
index0h, 2021-12-30
@index0h

https://github.com/index0h/php-conventions#7-testi...
Specifically for the code:
1. Don't worry about splitting everything into separate methods. You will not reduce the actual complexity, instead you will force the engineer who will watch it to run around the class to somehow connect your one-liners. Specifically, in your case, it is worth making only one getValue method, and what you scattered over the protected ones should be stuffed into getValue.
2. db and timedate instances should be passed to the constructor explicitly, and static should be abandoned as much as possible.

F
FanatPHP, 2021-12-30
@FanatPHP

https://www.youtube.com/watch?v=psci3tB9y0M

Didn't find what you were looking for?

Ask your question

Ask a Question

731 491 924 answers to any question