Как правильно выполнять unit-тестирования методов с параметрами по умолчанию на ваш взгляд? - Курилка

**Андре** · 14.03.2017, 22:33

Цитата:

можешь рассказать об этом применительно к методам с параметрами по умолчанию?

А я вообще не понял, что тебе сдались эти параметры по умолчанию. Что нового они приносят в плане тестирования? Этот вопрос был и выше, но я не понял твой ответ на него.

На мой взгляд ничего. Методы с параметрами по умолчанию надо тестировать точно также, как и методы с обычными параметрами. Все вот эти вопросы:

Цитата:

например, сколько тестирующих методов должно быть для метода с дефолтными параметрами?
= один тестирующий с несколькими ассертами?
= столько тестирующих, сколько различных комбинаций параметров для того, чтобы покрыть все значимые комбинации параметров? причем в каждом тестирующем методе должен быть только один ассерт?
= какое-то "достаточное" число test-методов? каков критерий достаточности?

должны возникать и в том случае, если метод и без дефолтных параметров. Более того, и ответы на них вроде будут те же самые.

Цитата:

каковы критерии необходимости и достаточности?

Здравый смысл.

У меня нет полного кода метода, а на твоей картинке я вижу только локальные методы (собственно даже начало метода класса на картинку не попало), поэтому никаких примеров привести не могу. Я обычно проверяю типичные сценарии использования и какие-то граничные условия.

Многие фреймворки для тестирования позволяют все возможные case-ы для проверки записывать в лаконичном табличном виде в виде комментариев и по ним уже генерить вызовы методов тестирования.Как то так:

PHP код:


			
import spock.lang.Specification
 
class MathSpec extends Specification {
 
    def "Get the max value of two numbers"() {
 
        expect: 'Should return the bigger number'
        Math.max(a, b) == c
 
        where:
        a | b | c
        1 | 0 | 1
        2 | 3 | 3
    }
}

Есть фреймворки, которые позволяют случайно генерить данные для тестирования (https://github.com/fscheck/FsCheck). Как то так:

PHP код:


			
let revRevIsOrig (xs:list<int>) = List.rev(List.rev xs) = xs
Check.Quick revRevIsOrig

Но я не сторонник таких подходов.

Попробую объяснить. Этой мой unit test и это мой код, который я тестирую. Я его знаю. Я могу быть не уверен, работает ли он корректно, но подразумевается, что я достаточно хорошо его понимаю, чтобы выделить какие-то сценарии его использования (как стандартные, так и наоборот - неожиданные). Их и надо тестировать.

**mazzy** · 14.03.2017, 23:08

Цитата:

Сообщение от Андре

А я вообще не понял, что тебе сдались эти параметры по умолчанию. Что нового они приносят в плане тестирования?

ничего.
кроме того, что это фишка аксапты, если я правильно понимаю.
поэтому ответ на вопрос не гуглится на стековерфлоу )))

Цитата:

Сообщение от Андре

должны возникать и в том случае, если метод и без дефолтных параметров. Более того, и ответы на них вроде будут те же самые.

угу.

Цитата:

Сообщение от Андре

Здравый смысл.

понятно.

Цитата:

Сообщение от Андре

Многие фреймворки для тестирования позволяют все возможные case-ы для проверки записывать в лаконичном табличном виде в виде комментариев и по ним уже генерить вызовы методов тестирования. Есть фреймворки, которые позволяют случайно генерить данные для тестирования (https://github.com/fscheck/FsCheck). Но я не сторонник таких подходов.

А почему не сторонник?

есть такие фреймоврки.
но в аксапте их нет.
стоит ли добавлять в аксапту?
стоит ли писать свой велосипед для подобных случаев?

Цитата:

Сообщение от Андре

Попробую объяснить. Этой мой unit test и это мой код, который я тестирую. Я его знаю. Я могу быть не уверен, работает ли он корректно, но подразумевается, что я достаточно хорошо его понимаю

А вот тут похоже и есть корень.
В аксапте чаще наоборот. И об этом писал fed выше.

Мой код использует что-то из стандартного функционала аксапты.
как правило, я не до конца понимаю как работает стандарт на всех граничных значениях.
как правило, я не понимаю нафига сделано именно так.

мало того, как правило, мой код всего лишь расширяет поведение какого-то стандартного метода.

мало того, с Аксаптой работают разные команды людей. причем могут работать одновременно. у разных команд может быть свое представление о происходящем.
причем это не только в мс. на проектах абсолютно то же самое.

и в этих условиях я должен изобразить что-то "сторожевое". )))
ну, т.е. понятно, что завсегда можно использовать "тяп-ляп и продакшн".
понятно, что каждую конкретную задачу решить можно.

вопрос то: как правильно? )))
пока есть ответ "Здравый смысл"

=================
не уверен, что понадобиться, но вот полный код метода.
повторюсь, что это очень и очень давний метод, который можно проследить до самых первых версий аксапты, в которых он был простым как пареная репа.

X++:

private boolean  findDisc(PriceType             _relation,
                              InventDimId           _inventDimId,
                              TableGroupAll         _itemCode       =  0,
                              ItemId                _itemRel        = '',
                              TableGroupAll         _accountCode    =  0,
                              CustVendAC            _accountRel     = '',
                              UnitOfMeasureSymbol   _unitID         = '',
                              Qty                   _quantityAmount =  0,
                              CurrencyCode          _currency       = CompanyInfoHelper::standardCurrency(),
                              AgreementHeaderExtRecId_RU _agreementHeaderExtRecId = 0,
                              CustVendAC                 _agreementPartnerCode = '')
    {
        PriceDiscTable      priceDiscTable;
        boolean             discExist;
        container           key;
        container           cacheValue;
        int                 i;
        FromDate            localFromDate;
        ToDate              localToDate;
        AmountQty           localQuantityAmountFrom;
        AmountQuantityTo    localQuantityAmountTo;
        RecId               localRecid;
        boolean             cacheMode;
        // <GEERU>
        CustVendAC      accountRelation = _accountRel;
        // </GEERU>
        void reselectBuffer()
        {
            if (cacheMode)
            {
                priceDiscTable = PriceDiscTable::findRecId(localRecid);
            }
        }

        void findDisc()
        {
            if ((discDate >= localFromDate  || ! localFromDate)
                && (discDate <= localToDate || ! localToDate))
            {
                if (_relation == PriceType::EndDiscPurch ||
                    _relation == PriceType::EndDiscSales )
                {
                    // for end discounts, the QuantiyAmountField field contains order total amounts, not quantities
                    if (this.calcCur2CurPriceAmount(localQuantityAmountFrom, priceDiscTable) <= qty &&
                        ((qty < this.calcCur2CurPriceAmount(localQuantityAmountTo, priceDiscTable)) || !localQuantityAmountTo))
                    {
                        reselectBuffer();

                        discExist               = true;
                        discAmount             += this.calcCur2CurPriceAmount(priceDiscTable.Amount, priceDiscTable)/ this.priceUnit();
                        percent1               += priceDiscTable.Percent1;
                        percent2               += priceDiscTable.Percent2;
                        actualDiscTable        = priceDiscTable.data();
                        quantityAmount         += priceDiscTable.QuantityAmountFrom;
                    }
                }
                else
                {
                    if (localQuantityAmountFrom <= qty
                        && (qty < localQuantityAmountTo || !localQuantityAmountTo))
                    {
                        reselectBuffer();

                        discExist               = true;
                        discAmount             += this.calcCur2CurPriceAmount(priceDiscTable.Amount, priceDiscTable)/ this.priceUnit();
                        percent1               += priceDiscTable.Percent1;
                        percent2               += priceDiscTable.Percent2;
                        actualDiscTable        =  priceDiscTable.data();
                        quantityAmount         += priceDiscTable.QuantityAmountFrom;

                        this.mcrPriceDiscTableFound(priceDiscTable);
                    }
                    else
                    {
                        // If quantity does not qualify, but calculation potential then add as found
                        if (this.parmMCRPriceHistoryPotentialCalc())
                        {
                            reselectBuffer();

                            this.mcrPriceDiscTableFound(priceDiscTable);
                        }
                    }
                }
            }
        }

        // <GEERU>
        if (countryRegion_RU)
        {
            if (_accountCode == TableGroupAll::Table &&
                _agreementHeaderExtRecId &&
                _agreementPartnerCode)
            {
                accountRelation = _agreementPartnerCode;
            }
        }
        // </GEERU>

        if (!_inventDimId)
        {
            return false;
        }

        // To avoid flooding the cache the most granualated setup isn't cached.
        cacheMode = (_itemCode       != TableGroupAll::Table
                     || _accountCode != TableGroupAll::Table)
                    && !this.parmMCRPriceHistoryPotentialCalc();

        cacheValue = conNull();

        if (cacheMode)
        {
            key = this.makeKey(_relation,
                               _itemCode,
                               _itemRel,
                               _accountCode,
                               // <GEERU>
                               accountRelation,
                               // </GEERU>
                               _unitID,
                               _currency,
                               _inventDimId
                               // <GEERU>
                               ,_agreementHeaderExtRecId
                               // </GEERU>
                               );

            cacheValue = PriceDisc::getPriceDiscCacheValue(#cacheScope_FindDisc, key);
        }
        qty        = abs(_quantityAmount);

        if (cacheValue == conNull())
        {
            if (_itemCode != TableGroupAll::Table)
            {
                _unitID     = '';
            }

            while select priceDiscTable
                order by QuantityAmountFrom, FromDate
                where priceDiscTable.Relation            == _relation
                    && priceDiscTable.ItemCode           == _itemCode
                    && priceDiscTable.ItemRelation       == _itemRel
                    && priceDiscTable.AccountCode        == _accountCode
                    // <GEERU>
                    && priceDiscTable.AccountRelation    == accountRelation
                    // </GEERU>
                    && priceDiscTable.UnitId             == _unitID
                    && (priceDiscTable.Currency          == _currency
                    || (priceDiscTable.GenericCurrency
                    &&  priceDiscTable.Currency          == genericCurrency))
                    // <GEERU>
                    && (!countryRegion_RU || priceDiscTable.AgreementHeaderExt_RU == _agreementHeaderExtRecId)
                    // </GEERU>
                    && (priceDiscTable.InventDimId       == _inventDimId || this.parmMCRPriceHistoryPotentialCalc())
            {
                if (cacheMode)
                {
                    cacheValue += [[priceDiscTable.FromDate, priceDiscTable.ToDate, priceDiscTable.QuantityAmountFrom, priceDiscTable.QuantityAmountTo, priceDiscTable.RecId]];
                }
                else
                {
                    localFromDate            = priceDiscTable.FromDate;
                    localToDate              = priceDiscTable.ToDate;
                    localQuantityAmountFrom  = priceDiscTable.QuantityAmountFrom;
                    localQuantityAmountTo    = priceDiscTable.QuantityAmountTo;
                    localRecid               = priceDiscTable.RecId;

                    findDisc();

                    if (discExist
                        && !priceDiscTable.SearchAgain)
                    {
                        searchAgain = false;
                        break;
                    }
                }
            }
            if (cacheMode)
            {
                //We also want to cache the absence of discounts.
                if (cacheValue == conNull())
                {
                    cacheValue = [[0]];
                }

                PriceDisc::insertPriceDiscCache(#cacheScope_FindDisc, key, cacheValue);
            }
        }

        if (cacheMode
            && cacheValue)
        {
            discExist = false;
            for (i=1;i<=conLen(cacheValue);i++)
            {
                [localFromDate, localToDate, localQuantityAmountFrom, localQuantityAmountTo, localRecid] = conPeek(cacheValue, i);

                if (localRecid)
                {
                    findDisc();

                    if (discExist
                        && !priceDiscTable.SearchAgain
                        && !this.parmMCRPriceHistoryPotentialCalc())
                    {
                        searchAgain = false;
                        break;
                    }
                }
            }
        }
        return discExist;
    }

skuull · 14.03.2017, 23:43

Цитата:

Сообщение от mazzy

как правило, я не понимаю нафига сделано именно так.

мало того, как правило, мой код всего лишь расширяет поведение какого-то стандартного метода.

А как мы можем протестировать то что не знаем как работат ? Методом черного ящика, а как тестируют этот ящик ? По спецификации. А где спецификация ? Нигде.
Ну значит надо покрыть тестами свои изменения, потому что мы знаем что мы делаем и забить на все остальное. Глядя на существующие юнит тесты в АХ многие из них написаны просто чтобы нагнать покрытие и получить волшебную зеленую лампочку, некоторые из них даже не асертят ничего и тут либо в МС сидят одни дураки либо это самый правильный подход с точки зрения отношения затрат к результату. Да и по моему скромному опыту и чекинам которые я видел, если АХ тесты падают, то проблема чаще в тесте чем в тестируемом методе...

Опции темы	Поиск в этой теме
Версия для печати Отправить по электронной почте	Поиск в этой теме: Расширенный поиск
Опции просмотра
Линейный вид Комбинированный вид Древовидный вид