first part of tests #157
first part of tests #157
Conversation
WalkthroughВ данном запросе на изменение был добавлен новый XML-файл, определяющий объект метаданных для общего модуля "ОМ_ПредопределенныеЗначения". Также был создан файл с процедурами для тестирования предопределенных значений, который включает тесты и вспомогательные процедуры. В дополнение к этому, добавлены новые XML-файлы для других модулей и внесены изменения в файл конфигурации, что расширяет список доступных модулей. Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🔇 Additional comments (2)src/cfe/YAXUnit/CommonModules/пбп_ПредопределенныеЗначенияПереопределяемый/Ext/Module.bsl (2)
В функциях +#Область Макросы
+
+// Создает функцию-перехватчик для указанного метода
+//
+// Параметры:
+// ИмяМетода - Строка - Имя перехватываемого метода
+//
+#КонецОбласти
+
+#Макрос СоздатьПерехватчик(ИмяМетода)
+&Вместо(ИмяМетода)
+Функция Мок_#ИмяМетода()
+ ПараметрыМетода = Новый Массив;
+ Возврат ВыполнитьПерехватМетода("#ИмяМетода", ПараметрыМетода);
+КонецФункции
+#КонецМакроса
+
-&Вместо("ИсключаемыеПоляДляРасчетаХешаЭлемент")
-Функция Мок_ИсключаемыеПоляДляРасчетаХешаЭлемент()
- ПараметрыМетода = Новый Массив;
- Возврат ВыполнитьПерехватМетода("ИсключаемыеПоляДляРасчетаХешаЭлемент", ПараметрыМетода);
-КонецФункции
+СоздатьПерехватчик("ИсключаемыеПоляДляРасчетаХешаЭлемент")
-&Вместо("ИсключаемыеПоляДляРасчетаХешаГруппа")
-Функция Мок_ИсключаемыеПоляДляРасчетаХешаГруппа()
- ПараметрыМетода = Новый Массив;
- Возврат ВыполнитьПерехватМетода("ИсключаемыеПоляДляРасчетаХешаГруппа", ПараметрыМетода);
-КонецФункции
+СоздатьПерехватчик("ИсключаемыеПоляДляРасчетаХешаГруппа")
Функция имеет:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначения/Ext/Module.bsl (2)
3-22: Добавьте описание назначения тестового модуляРекомендуется добавить комментарий, описывающий назначение тестового модуля и его основные сценарии тестирования перед процедурой
ИсполняемыеСценарии.#Область СлужебныйПрограммныйИнтерфейс +// Модуль тестирования функциональности предопределенных значений +// Проверяет корректность инициализации и обработки предопределенных элементов Процедура ИсполняемыеСценарии() Экспорт
24-83: Оптимизируйте повторяющийся код инициализацииВ тестовых методах присутствует дублирование кода инициализации менеджера объекта. Рекомендуется вынести общую логику в отдельную вспомогательную процедуру.
+Процедура ПолучитьМенеджерОбъекта(ИмяОбъекта, МенеджерОбъекта) + МенеджерОбъекта = пбп_Переадресация.МенеджерОбъектаПоПолномуИмени(ИмяОбъекта); +КонецПроцедуры Процедура Тест_ИнициализацияИзПодсистемыПредопределенных(ИмяОбъекта) Экспорт - Менеджер = пбп_Переадресация.МенеджерОбъектаПоПолномуИмени(ИмяОбъекта); + ПолучитьМенеджерОбъекта(ИмяОбъекта, Менеджер);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначения.xml(1 hunks)src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначения/Ext/Module.bsl(1 hunks)src/cfe/YAXUnit/Configuration.xml(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначения.xml
🔇 Additional comments (2)
src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначения/Ext/Module.bsl (1)
89-107: LGTM! Проверки реализованы корректно
Процедура ПроверкаРезультатовОбработкиПредопределенных содержит полный набор необходимых проверок типов и структуры результата.
src/cfe/YAXUnit/Configuration.xml (1)
164-164: LGTM! Модуль добавлен корректно
Регистрация общего модуля ОМ_ПредопределенныеЗначения выполнена в соответствии с существующей структурой конфигурации.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (5)
src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначенияСлужебный/Ext/Module.bsl (2)
3-21: Структура тестового набора организована корректноТестовые сценарии хорошо структурированы и логически сгруппированы. Названия тестов информативны.
Рекомендуется добавить краткое описание назначения каждого тестового набора в виде комментария перед его определением:
ЮТТесты + // Проверка корректности обработки дополнительных параметров .ДобавитьТестовыйНабор("Доп. параметры обработки предопределенных элементов") + // Проверка принадлежности менеджера к подсистеме .ДобавитьТестовыйНабор("Менеджер входит в подсистему предопределенных")
86-109: Рекомендуется добавить раннюю валидацию параметровПроцедура проверки результатов реализована корректно, но можно улучшить обработку входных параметров.
Рекомендуется добавить проверку входных параметров в начале процедуры:
Процедура ПроверкаРезультатовОбработкиПредопределенных(МенеджерОбъекта, Таблица, МенеджерИзПодсистемы = Истина) + // Проверка входных параметров + Если Таблица = Неопределено Тогда + ВызватьИсключение НСтр("ru='Параметр Таблица не может быть Неопределено'"); + КонецЕсли; + + Если ТипЗнч(МенеджерИзПодсистемы) <> Тип("Булево") Тогда + ВызватьИсключение НСтр("ru='Параметр МенеджерИзПодсистемы должен быть булевого типа'"); + КонецЕсли; Результат = пбп_ПредопределенныеЗначенияСлужебный .ДопПараметрыОбработкиПредопределенныхЭлементов(Таблица, МенеджерОбъекта);src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначения/Ext/Module.bsl (2)
14-36: Рекомендуется добавить проверку исключения служебных полейВ тесте присутствует поле "Служебный_Поле3", но отсутствует явная проверка его исключения из результата. Рекомендуется добавить проверку, что служебные поля корректно отфильтровываются.
Ожидаемое = Новый Структура("Поле1,Поле4"); + // Проверяем что служебные поля отсутствуют в результате + ЮТест.ОжидаетЧто(Результат.Свойство("Служебный_Поле3")).Равно(Ложь); ЮТест.ОжидаетЧто(Результат).Равно(Ожидаемое);
47-51: Рекомендуется соблюдать единый стиль форматированияВ данном тесте точка с запятой вынесена на отдельную строку, в то время как в других тестах она находится в той же строке, что и вызов метода .Прогон().
Мокито.Обучение(пбп_ПредопределенныеЗначенияПереопределяемый) .Когда("ИсключаемыеПоляДляРасчетаХешаГруппа") .Вернуть(ИсключаемыеПоля) - .Прогон() - ; + .Прогон();src/cfe/YAXUnit/CommonModules/пбп_ПредопределенныеЗначенияПереопределяемый/Ext/Module.bsl (1)
2-19: Добавьте документацию к функцииРекомендуется добавить описание назначения функции, её параметров и возвращаемого значения в формате:
+// Функция-перехватчик для тестирования метода ИсключаемыеПоляДляРасчетаХешаЭлемент +// +// Возвращаемое значение: +// Произвольный - Результат оригинального метода или мока &Вместо("ИсключаемыеПоляДляРасчетаХешаЭлемент") Функция Мок_ИсключаемыеПоляДляРасчетаХешаЭлемент()
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначения/Ext/Module.bsl(1 hunks)src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначенияСлужебный.xml(1 hunks)src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначенияСлужебный/Ext/Module.bsl(1 hunks)src/cfe/YAXUnit/CommonModules/пбп_ПредопределенныеЗначенияПереопределяемый.xml(1 hunks)src/cfe/YAXUnit/CommonModules/пбп_ПредопределенныеЗначенияПереопределяемый/Ext/Module.bsl(1 hunks)src/cfe/YAXUnit/Configuration.xml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/cfe/YAXUnit/CommonModules/пбп_ПредопределенныеЗначенияПереопределяемый.xml
- src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначенияСлужебный.xml
🚧 Files skipped from review as they are similar to previous changes (1)
- src/cfe/YAXUnit/Configuration.xml
🔇 Additional comments (2)
src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначения/Ext/Module.bsl (1)
3-12: Корректная организация тестового набора!
Процедура правильно организует тестовый набор, используя понятные названия тестов и корректную группировку.
src/cfe/YAXUnit/CommonModules/пбп_ПредопределенныеЗначенияПереопределяемый/Ext/Module.bsl (1)
1-38: Корректная реализация тестовых заглушек
Реализация корректно использует фреймворк Mockito и следует стандартным паттернам создания заглушек для тестирования. Основная логика работоспособна.
| Процедура Тест_ДопПараметрыОбработкиКорректныеДанные() Экспорт | ||
|
|
||
| МенеджерОбъекта = пбп_Переадресация | ||
| .МенеджерОбъектаПоПолномуИмени("ПланВидовХарактеристик.пбп_ПредопределенныеЗначения"); | ||
|
|
||
| Таблица = пбп_ПредопределенныеЗначенияПереопределяемый.ТаблицаПредопределенныхЭлементов(МенеджерОбъекта); | ||
|
|
||
| ПроверкаРезультатовОбработкиПредопределенных(МенеджерОбъекта, Таблица); | ||
|
|
||
| КонецПроцедуры | ||
|
|
||
| Процедура Тест_ДопПараметрыОбработкиПустаяТаблица() Экспорт | ||
|
|
||
| МенеджерОбъекта = пбп_Переадресация | ||
| .МенеджерОбъектаПоПолномуИмени("ПланВидовХарактеристик.пбп_ПредопределенныеЗначения"); | ||
|
|
||
| Таблица = Новый ТаблицаЗначений; | ||
|
|
||
| ПроверкаРезультатовОбработкиПредопределенных(МенеджерОбъекта, Таблица); | ||
|
|
||
| КонецПроцедуры | ||
|
|
||
| Процедура Тест_ДопПараметрыОбработкиМенеджерНеИзПодсистемыПредопределенных(ИмяОбъекта) Экспорт | ||
|
|
||
| МенеджерОбъекта = пбп_Переадресация.МенеджерОбъектаПоПолномуИмени(ИмяОбъекта); | ||
|
|
||
| Таблица = Новый ТаблицаЗначений; | ||
|
|
||
| ПроверкаРезультатовОбработкиПредопределенных(МенеджерОбъекта, Таблица, Ложь); | ||
|
|
||
| КонецПроцедуры | ||
|
|
||
| Процедура Тест_ДопПараметрыОбработкиМенеджерРавенНеопределено() Экспорт | ||
|
|
||
| МенеджерОбъекта = Неопределено; | ||
|
|
||
| Таблица = Новый ТаблицаЗначений; | ||
|
|
||
| ПроверкаРезультатовОбработкиПредопределенных(МенеджерОбъекта, Таблица, Ложь); | ||
|
|
||
| КонецПроцедуры | ||
|
|
||
| Процедура Тест_МенеджерВходитВПодсистему(ИмяОбъекта, ОжидаемоеЗначение) Экспорт | ||
|
|
||
| МенеджерОбъекта = Неопределено; | ||
| Если ЗначениеЗаполнено(ИмяОбъекта) Тогда | ||
| МенеджерОбъекта = пбп_Переадресация.МенеджерОбъектаПоПолномуИмени(ИмяОбъекта); | ||
| КонецЕсли; | ||
|
|
||
| Результат = пбп_ПредопределенныеЗначенияСлужебный.МенеджерВходитВПодсистемуПредопределенных(МенеджерОбъекта); | ||
|
|
||
| Если ОжидаемоеЗначение Тогда | ||
| ЮТест.ОжидаетЧто(Результат).ЭтоИстина(); | ||
| Иначе | ||
| ЮТест.ОжидаетЧто(Результат).ЭтоЛожь(); | ||
| КонецЕсли; | ||
|
|
||
| КонецПроцедуры |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Необходимо улучшить документацию тестовых процедур
Тестовые процедуры реализованы корректно, но отсутствует документация, описывающая назначение каждого теста и его ожидаемое поведение.
Рекомендуется:
- Добавить комментарии перед каждой тестовой процедурой, описывающие:
- Назначение теста
- Входные данные
- Ожидаемый результат
Пример:
+// Проверяет обработку корректных данных для предопределенных элементов
+//
+// Параметры:
+// Нет
+//
+// Ожидаемый результат:
+// Структура с корректно заполненными свойствами
Процедура Тест_ДопПараметрыОбработкиКорректныеДанные() Экспорт- Вынести повторяющийся код инициализации в отдельную вспомогательную процедуру:
Процедура ИнициализироватьМенеджерОбъекта(ИмяОбъекта, МенеджерОбъекта)
Если ЗначениеЗаполнено(ИмяОбъекта) Тогда
МенеджерОбъекта = пбп_Переадресация.МенеджерОбъектаПоПолномуИмени(ИмяОбъекта);
Иначе
МенеджерОбъекта = Неопределено;
КонецЕсли;
КонецПроцедуры| Процедура Тест_КлючиХеша_ПустойМассив() Экспорт | ||
|
|
||
| Поля = Новый Массив; | ||
|
|
||
| ИсключаемыеПоля = Новый Соответствие; | ||
|
|
||
| Мокито.Обучение(пбп_ПредопределенныеЗначенияПереопределяемый) | ||
| .Когда("ИсключаемыеПоляДляРасчетаХешаЭлемент") | ||
| .Вернуть(ИсключаемыеПоля) | ||
| .Прогон(); | ||
|
|
||
| Ожидаемое = Новый Структура; | ||
|
|
||
| Результат = пбп_ПредопределенныеЗначения.КлючиХешаПредопределенногоЭлемента(Поля); | ||
|
|
||
| ЮТест.ОжидаетЧто(Результат).Равно(Ожидаемое); | ||
|
|
||
| КонецПроцедуры |
There was a problem hiding this comment.
Необходимо исправить несоответствия в реализации
- Используется тип Соответствие для ИсключаемыеПоля, в то время как в других тестах используется Структура
- В вызове КлючиХешаПредопределенногоЭлемента отсутствует второй параметр, который присутствует в других тестах
- ИсключаемыеПоля = Новый Соответствие;
+ ИсключаемыеПоля = Новый Структура;
- Результат = пбп_ПредопределенныеЗначения.КлючиХешаПредопределенногоЭлемента(Поля);
+ Результат = пбп_ПредопределенныеЗначения.КлючиХешаПредопределенногоЭлемента(Поля, Ложь);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Процедура Тест_КлючиХеша_ПустойМассив() Экспорт | |
| Поля = Новый Массив; | |
| ИсключаемыеПоля = Новый Соответствие; | |
| Мокито.Обучение(пбп_ПредопределенныеЗначенияПереопределяемый) | |
| .Когда("ИсключаемыеПоляДляРасчетаХешаЭлемент") | |
| .Вернуть(ИсключаемыеПоля) | |
| .Прогон(); | |
| Ожидаемое = Новый Структура; | |
| Результат = пбп_ПредопределенныеЗначения.КлючиХешаПредопределенногоЭлемента(Поля); | |
| ЮТест.ОжидаетЧто(Результат).Равно(Ожидаемое); | |
| КонецПроцедуры | |
| Процедура Тест_КлючиХеша_ПустойМассив() Экспорт | |
| Поля = Новый Массив; | |
| ИсключаемыеПоля = Новый Структура; | |
| Мокито.Обучение(пбп_ПредопределенныеЗначенияПереопределяемый) | |
| .Когда("ИсключаемыеПоляДляРасчетаХешаЭлемент") | |
| .Вернуть(ИсключаемыеПоля) | |
| .Прогон(); | |
| Ожидаемое = Новый Структура; | |
| Результат = пбп_ПредопределенныеЗначения.КлючиХешаПредопределенногоЭлемента(Поля, Ложь); | |
| ЮТест.ОжидаетЧто(Результат).Равно(Ожидаемое); | |
| КонецПроцедуры |
There was a problem hiding this comment.
Результат = пбп_ПредопределенныеЗначения.КлючиХешаПредопределенногоЭлемента(Поля, Ложь); так можно не писать, необязательные параметры можем опустить, при этом будет использовано значение по умолчанию
There was a problem hiding this comment.
@denisushakov, спасибо за пояснение. Принял к сведению, что в функции КлючиХешаПредопределенногоЭлемента необязательные параметры можно не указывать, и будут использованы значения по умолчанию.
✏️ Learnings added
Learnt from: denisushakov
PR: firstBitSportivnaya/PSSL#157
File: src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначения/Ext/Module.bsl:61-78
Timestamp: 2024-12-16T08:09:35.244Z
Learning: В функции `КлючиХешаПредопределенногоЭлемента` модуля `пбп_ПредопределенныеЗначения` необязательные параметры можно опускать, и тогда используются значения по умолчанию.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| &Вместо("ИсключаемыеПоляДляРасчетаХешаЭлемент") | ||
| Функция Мок_ИсключаемыеПоляДляРасчетаХешаЭлемент() | ||
|
|
||
| ПараметрыМетода = Мокито.МассивПараметров(); | ||
|
|
||
| ПрерватьВыполнение = Ложь; | ||
| Результат = МокитоПерехват.АнализВызова(пбп_ПредопределенныеЗначенияПереопределяемый, | ||
| "ИсключаемыеПоляДляРасчетаХешаЭлемент", | ||
| ПараметрыМетода, | ||
| ПрерватьВыполнение); | ||
|
|
||
| Если НЕ ПрерватьВыполнение Тогда | ||
| Возврат ПродолжитьВызов(); | ||
| Иначе | ||
| Возврат Результат; | ||
| КонецЕсли; | ||
|
|
||
| КонецФункции | ||
|
|
||
| &Вместо("ИсключаемыеПоляДляРасчетаХешаГруппа") | ||
| Функция Мок_ИсключаемыеПоляДляРасчетаХешаГруппа() | ||
|
|
||
| ПараметрыМетода = Мокито.МассивПараметров(); | ||
|
|
||
| ПрерватьВыполнение = Ложь; | ||
| Результат = МокитоПерехват.АнализВызова(пбп_ПредопределенныеЗначенияПереопределяемый, | ||
| "ИсключаемыеПоляДляРасчетаХешаГруппа", | ||
| ПараметрыМетода, | ||
| ПрерватьВыполнение); | ||
|
|
||
| Если НЕ ПрерватьВыполнение Тогда | ||
| Возврат ПродолжитьВызов(); | ||
| Иначе | ||
| Возврат Результат; | ||
| КонецЕсли; | ||
|
|
||
| КонецФункции |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Рефакторинг дублирующегося кода
В обеих функциях наблюдается идентичная структура кода. Рекомендуется выделить общую логику в отдельную вспомогательную функцию для уменьшения дублирования:
+// Выполняет перехват вызова метода
+//
+// Параметры:
+// ИмяМетода - Строка - Имя перехватываемого метода
+//
+// Возвращаемое значение:
+// Произвольный - Результат вызова
+Функция ВыполнитьПерехватМетода(ИмяМетода)
+ ПараметрыМетода = Мокито.МассивПараметров();
+
+ ПрерватьВыполнение = Ложь;
+ Результат = МокитоПерехват.АнализВызова(пбп_ПредопределенныеЗначенияПереопределяемый,
+ ИмяМетода,
+ ПараметрыМетода,
+ ПрерватьВыполнение);
+
+ Если НЕ ПрерватьВыполнение Тогда
+ Возврат ПродолжитьВызов();
+ Иначе
+ Возврат Результат;
+ КонецЕсли;
+КонецФункции
+
&Вместо("ИсключаемыеПоляДляРасчетаХешаЭлемент")
Функция Мок_ИсключаемыеПоляДляРасчетаХешаЭлемент()
- ПараметрыМетода = Мокито.МассивПараметров();
-
- ПрерватьВыполнение = Ложь;
- Результат = МокитоПерехват.АнализВызова(пбп_ПредопределенныеЗначенияПереопределяемый,
- "ИсключаемыеПоляДляРасчетаХешаЭлемент",
- ПараметрыМетода,
- ПрерватьВыполнение);
-
- Если НЕ ПрерватьВыполнение Тогда
- Возврат ПродолжитьВызов();
- Иначе
- Возврат Результат;
- КонецЕсли;
+ Возврат ВыполнитьПерехватМетода("ИсключаемыеПоляДляРасчетаХешаЭлемент");
КонецФункции
&Вместо("ИсключаемыеПоляДляРасчетаХешаГруппа")
Функция Мок_ИсключаемыеПоляДляРасчетаХешаГруппа()
- ПараметрыМетода = Мокито.МассивПараметров();
-
- ПрерватьВыполнение = Ложь;
- Результат = МокитоПерехват.АнализВызова(пбп_ПредопределенныеЗначенияПереопределяемый,
- "ИсключаемыеПоляДляРасчетаХешаГруппа",
- ПараметрыМетода,
- ПрерватьВыполнение);
-
- Если НЕ ПрерватьВыполнение Тогда
- Возврат ПродолжитьВызов();
- Иначе
- Возврат Результат;
- КонецЕсли;
+ Возврат ВыполнитьПерехватМетода("ИсключаемыеПоляДляРасчетаХешаГруппа");
КонецФункции📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| &Вместо("ИсключаемыеПоляДляРасчетаХешаЭлемент") | |
| Функция Мок_ИсключаемыеПоляДляРасчетаХешаЭлемент() | |
| ПараметрыМетода = Мокито.МассивПараметров(); | |
| ПрерватьВыполнение = Ложь; | |
| Результат = МокитоПерехват.АнализВызова(пбп_ПредопределенныеЗначенияПереопределяемый, | |
| "ИсключаемыеПоляДляРасчетаХешаЭлемент", | |
| ПараметрыМетода, | |
| ПрерватьВыполнение); | |
| Если НЕ ПрерватьВыполнение Тогда | |
| Возврат ПродолжитьВызов(); | |
| Иначе | |
| Возврат Результат; | |
| КонецЕсли; | |
| КонецФункции | |
| &Вместо("ИсключаемыеПоляДляРасчетаХешаГруппа") | |
| Функция Мок_ИсключаемыеПоляДляРасчетаХешаГруппа() | |
| ПараметрыМетода = Мокито.МассивПараметров(); | |
| ПрерватьВыполнение = Ложь; | |
| Результат = МокитоПерехват.АнализВызова(пбп_ПредопределенныеЗначенияПереопределяемый, | |
| "ИсключаемыеПоляДляРасчетаХешаГруппа", | |
| ПараметрыМетода, | |
| ПрерватьВыполнение); | |
| Если НЕ ПрерватьВыполнение Тогда | |
| Возврат ПродолжитьВызов(); | |
| Иначе | |
| Возврат Результат; | |
| КонецЕсли; | |
| КонецФункции | |
| // Выполняет перехват вызова метода | |
| // | |
| // Параметры: | |
| // ИмяМетода - Строка - Имя перехватываемого метода | |
| // | |
| // Возвращаемое значение: | |
| // Произвольный - Результат вызова | |
| Функция ВыполнитьПерехватМетода(ИмяМетода) | |
| ПараметрыМетода = Мокито.МассивПараметров(); | |
| ПрерватьВыполнение = Ложь; | |
| Результат = МокитоПерехват.АнализВызова(пбп_ПредопределенныеЗначенияПереопределяемый, | |
| ИмяМетода, | |
| ПараметрыМетода, | |
| ПрерватьВыполнение); | |
| Если НЕ ПрерватьВыполнение Тогда | |
| Возврат ПродолжитьВызов(); | |
| Иначе | |
| Возврат Результат; | |
| КонецЕсли; | |
| КонецФункции | |
| &Вместо("ИсключаемыеПоляДляРасчетаХешаЭлемент") | |
| Функция Мок_ИсключаемыеПоляДляРасчетаХешаЭлемент() | |
| Возврат ВыполнитьПерехватМетода("ИсключаемыеПоляДляРасчетаХешаЭлемент"); | |
| КонецФункции | |
| &Вместо("ИсключаемыеПоляДляРасчетаХешаГруппа") | |
| Функция Мок_ИсключаемыеПоляДляРасчетаХешаГруппа() | |
| Возврат ВыполнитьПерехватМетода("ИсключаемыеПоляДляРасчетаХешаГруппа"); | |
| КонецФункции |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/cfe/YAXUnit/CommonModules/пбп_ПредопределенныеЗначенияПереопределяемый/Ext/Module.bsl (1)
23-42:⚠️ Potential issueОбнаружен недостижимый код после оператора возврата
В функции
Мок_ИсключаемыеПоляДляРасчетаХешаГруппакод после строки 28 никогда не будет выполнен из-за оператораВозврат. Этот код дублирует логику, которая уже реализована в функцииВыполнитьПерехватМетода.Примените следующие изменения:
Функция Мок_ИсключаемыеПоляДляРасчетаХешаГруппа() ПараметрыМетода = Новый Массив; Возврат ВыполнитьПерехватМетода("ИсключаемыеПоляДляРасчетаХешаГруппа", ПараметрыМетода); - - ПрерватьВыполнение = Ложь; - Результат = МокитоПерехват.АнализВызова(пбп_ПредопределенныеЗначенияПереопределяемый, - "ИсключаемыеПоляДляРасчетаХешаГруппа", - ПараметрыМетода, - ПрерватьВыполнение); - - Если НЕ ПрерватьВыполнение Тогда - Возврат ПродолжитьВызов(); - Иначе - Возврат Результат; - КонецЕсли; КонецФункции
🧹 Nitpick comments (4)
src/cfe/YAXUnit/CommonModules/пбп_ПредопределенныеЗначения/Ext/Module.bsl (2)
5-11: Рекомендуется добавить обработку ошибокСледует добавить проверку параметра
МенеджерОбъектанаНеопределенои корректно обработать возможные исключения при вызове методовМокито.Предлагаемые изменения:
Функция Мок_ИнициализироватьПредопределенныеЗначения(МенеджерОбъекта) + Если МенеджерОбъекта = Неопределено Тогда + ВызватьИсключение НСтр("ru='Параметр МенеджерОбъекта не может быть неопределен'"); + КонецЕсли; + + Попытка ПараметрыМетода = Мокито.МассивПараметров(МенеджерОбъекта); ПрерватьВыполнение = Ложь; Результат = МокитоПерехват.АнализВызова(пбп_ПредопределенныеЗначения, "ИнициализироватьПредопределенныеЗначения", ПараметрыМетода, ПрерватьВыполнение); + Исключение + ВызватьИсключение НСтр("ru='Ошибка при инициализации мока: '") + ОписаниеОшибки(); + КонецПопытки;
1-19: Добавьте документацию к методуРекомендуется добавить описание назначения метода, параметров и возвращаемого значения в формате документации 1С.
Предлагаемые изменения:
+// Выполняет подмену метода ИнициализироватьПредопределенныеЗначения для целей тестирования +// +// Параметры: +// МенеджерОбъекта - Произвольный - Объект, для которого выполняется инициализация предопределенных значений +// +// Возвращаемое значение: +// Произвольный - Результат выполнения метода +// &Вместо("ИнициализироватьПредопределенныеЗначения") Функция Мок_ИнициализироватьПредопределенныеЗначения(МенеджерОбъекта)src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначенияВызовСервера/Ext/Module.bsl (2)
27-31: Рекомендуется вынести пустые значения в константыДля улучшения поддерживаемости кода рекомендуется объявить константы для часто используемых значений:
+ ПустойАдресТаблицы = ""; + ОжидаемаяСтруктура = Новый Структура; - ОжидаемаяСтруктура.Вставить("АдресТаблицы", ""); + ОжидаемаяСтруктура.Вставить("АдресТаблицы", ПустойАдресТаблицы); ОжидаемаяСтруктура.Вставить("ЕстьСтроки", Ложь);
55-57: Рекомендуется вынести значения в константы и рассмотреть общую инициализацию
- Строковые литералы следует вынести в константы
- Структура ожидаемого результата дублируется между тестами
+ // В начале модуля + Перем АдресВременногоХранилища; + АдресВременногоХранилища = "АдресВременногоХранилища"; + + // Общая процедура инициализации + Функция СоздатьСтруктуруРезультата(АдресТаблицы, ЕстьСтроки) + Результат = Новый Структура; + Результат.Вставить("АдресТаблицы", АдресТаблицы); + Результат.Вставить("ЕстьСтроки", ЕстьСтроки); + Возврат Новый ФиксированнаяСтруктура(Результат); + КонецФункции + + // В тесте ОжидаемаяСтруктура = Новый Структура; - ОжидаемаяСтруктура.Вставить("АдресТаблицы", "АдресВременногоХранилища"); - ОжидаемаяСтруктура.Вставить("ЕстьСтроки", Истина); - - Ожидаемое = Новый ФиксированнаяСтруктура(ОжидаемаяСтруктура); + Ожидаемое = СоздатьСтруктуруРезультата(АдресВременногоХранилища, Истина);
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначения/Ext/Module.bsl(1 hunks)src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначенияВызовСервера.xml(1 hunks)src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначенияВызовСервера/Ext/Module.bsl(1 hunks)src/cfe/YAXUnit/CommonModules/пбп_ПредопределенныеЗначения.xml(1 hunks)src/cfe/YAXUnit/CommonModules/пбп_ПредопределенныеЗначения/Ext/Module.bsl(1 hunks)src/cfe/YAXUnit/CommonModules/пбп_ПредопределенныеЗначенияПереопределяемый/Ext/Module.bsl(1 hunks)src/cfe/YAXUnit/Configuration.xml(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/cfe/YAXUnit/CommonModules/пбп_ПредопределенныеЗначения.xml
- src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначенияВызовСервера.xml
🚧 Files skipped from review as they are similar to previous changes (2)
- src/cfe/YAXUnit/Configuration.xml
- src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначения/Ext/Module.bsl
🔇 Additional comments (5)
src/cfe/YAXUnit/CommonModules/пбп_ПредопределенныеЗначения/Ext/Module.bsl (2)
2-3: Корректное использование директивы подмены метода
Правильное применение директивы &Вместо и наименование мок-метода соответствует конвенциям.
13-17: Логика обработки результата реализована корректно
Правильное использование условного оператора для определения необходимости перехвата или продолжения выполнения.
src/cfe/YAXUnit/CommonModules/пбп_ПредопределенныеЗначенияПереопределяемый/Ext/Module.bsl (1)
44-71: Корректная реализация вспомогательной функции
Функция ВыполнитьПерехватМетода имеет:
- Подробную документацию с описанием параметров и возвращаемого значения
- Правильную реализацию перехвата вызова метода
- Корректную обработку результатов выполнения
src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначенияВызовСервера/Ext/Module.bsl (2)
7-13: Несогласованность в параметризации тестов
Тест Тест_ИнициализироватьПредопределенныеЭлементы_ЕстьКонфликты параметризован только одним метаданным (Справочник.пбп_ИнтеграционныеПотоки), в то время как первый тест выполняется для трех разных типов метаданных. Рекомендуется:
- Либо добавить остальные метаданные во второй тест
- Либо документировать причину различия в параметризации тестов
1-72: Тесты написаны корректно, но есть возможности для улучшения
Общая структура тестов хорошая, покрыты основные сценарии. Рекомендуется внести предложенные улучшения для повышения качества и поддерживаемости кода.
| &Вместо("ИсключаемыеПоляДляРасчетаХешаЭлемент") | ||
| Функция Мок_ИсключаемыеПоляДляРасчетаХешаЭлемент() | ||
|
|
||
| ПараметрыМетода = Новый Массив; | ||
|
|
||
| Возврат ВыполнитьПерехватМетода("ИсключаемыеПоляДляРасчетаХешаЭлемент", ПараметрыМетода); | ||
|
|
||
| ПрерватьВыполнение = Ложь; | ||
| Результат = МокитоПерехват.АнализВызова(пбп_ПредопределенныеЗначенияПереопределяемый, | ||
| "ИсключаемыеПоляДляРасчетаХешаЭлемент", | ||
| ПараметрыМетода, | ||
| ПрерватьВыполнение); | ||
|
|
||
| Если НЕ ПрерватьВыполнение Тогда | ||
| Возврат ПродолжитьВызов(); | ||
| Иначе | ||
| Возврат Результат; | ||
| КонецЕсли; | ||
|
|
||
| КонецФункции |
There was a problem hiding this comment.
Обнаружен недостижимый код после оператора возврата
В функции Мок_ИсключаемыеПоляДляРасчетаХешаЭлемент код после строки 7 никогда не будет выполнен из-за оператора Возврат. Этот код дублирует логику, которая уже реализована в функции ВыполнитьПерехватМетода.
Примените следующие изменения:
Функция Мок_ИсключаемыеПоляДляРасчетаХешаЭлемент()
ПараметрыМетода = Новый Массив;
Возврат ВыполнитьПерехватМетода("ИсключаемыеПоляДляРасчетаХешаЭлемент", ПараметрыМетода);
-
- ПрерватьВыполнение = Ложь;
- Результат = МокитоПерехват.АнализВызова(пбп_ПредопределенныеЗначенияПереопределяемый,
- "ИсключаемыеПоляДляРасчетаХешаЭлемент",
- ПараметрыМетода,
- ПрерватьВыполнение);
-
- Если НЕ ПрерватьВыполнение Тогда
- Возврат ПродолжитьВызов();
- Иначе
- Возврат Результат;
- КонецЕсли;
КонецФункции📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| &Вместо("ИсключаемыеПоляДляРасчетаХешаЭлемент") | |
| Функция Мок_ИсключаемыеПоляДляРасчетаХешаЭлемент() | |
| ПараметрыМетода = Новый Массив; | |
| Возврат ВыполнитьПерехватМетода("ИсключаемыеПоляДляРасчетаХешаЭлемент", ПараметрыМетода); | |
| ПрерватьВыполнение = Ложь; | |
| Результат = МокитоПерехват.АнализВызова(пбп_ПредопределенныеЗначенияПереопределяемый, | |
| "ИсключаемыеПоляДляРасчетаХешаЭлемент", | |
| ПараметрыМетода, | |
| ПрерватьВыполнение); | |
| Если НЕ ПрерватьВыполнение Тогда | |
| Возврат ПродолжитьВызов(); | |
| Иначе | |
| Возврат Результат; | |
| КонецЕсли; | |
| КонецФункции | |
| &Вместо("ИсключаемыеПоляДляРасчетаХешаЭлемент") | |
| Функция Мок_ИсключаемыеПоляДляРасчетаХешаЭлемент() | |
| ПараметрыМетода = Новый Массив; | |
| Возврат ВыполнитьПерехватМетода("ИсключаемыеПоляДляРасчетаХешаЭлемент", ПараметрыМетода); | |
| КонецФункции |
| Мокито.Обучение(пбп_ПредопределенныеЗначения) | ||
| .Когда("ИнициализироватьПредопределенныеЗначения") | ||
| .Вернуть(ТаблицаКонфликтов) | ||
| .Прогон() | ||
| ; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Отсутствует проверка вызова мока
Аналогично первому тесту, необходимо добавить проверку вызова мока:
Мокито.Обучение(пбп_ПредопределенныеЗначения)
.Когда("ИнициализироватьПредопределенныеЗначения")
.Вернуть(ТаблицаКонфликтов)
.Прогон()
;
+
+ Мокито.Проверить(пбп_ПредопределенныеЗначения)
+ .Что("ИнициализироватьПредопределенныеЗначения")
+ .СПараметрами(ИмяОбъекта)
+ .БылВызван()
+ ;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Мокито.Обучение(пбп_ПредопределенныеЗначения) | |
| .Когда("ИнициализироватьПредопределенныеЗначения") | |
| .Вернуть(ТаблицаКонфликтов) | |
| .Прогон() | |
| ; | |
| Мокито.Обучение(пбп_ПредопределенныеЗначения) | |
| .Когда("ИнициализироватьПредопределенныеЗначения") | |
| .Вернуть(ТаблицаКонфликтов) | |
| .Прогон() | |
| ; | |
| Мокито.Проверить(пбп_ПредопределенныеЗначения) | |
| .Что("ИнициализироватьПредопределенныеЗначения") | |
| .СПараметрами(ИмяОбъекта) | |
| .БылВызван() | |
| ; |
There was a problem hiding this comment.
Ты ошибаешься, такой функции нет в yaxunit
There was a problem hiding this comment.
@denisushakov, благодарю за уточнение. Вы правы, функция Мокито.Проверить отсутствует в yaxunit. Приношу извинения за ошибку.
✏️ Learnings added
Learnt from: denisushakov
PR: firstBitSportivnaya/PSSL#157
File: src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначенияВызовСервера/Ext/Module.bsl:49-53
Timestamp: 2024-12-16T14:32:56.592Z
Learning: В yaxunit функция `Мокито.Проверить` отсутствует и не должна предлагаться для использования в тестах.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| Мокито.Обучение(пбп_ПредопределенныеЗначения) | ||
| .Когда("ИнициализироватьПредопределенныеЗначения") | ||
| .Вернуть(ТаблицаКонфликтов) | ||
| .Прогон() | ||
| ; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Отсутствует проверка вызова мока
После выполнения теста следует добавить проверку, что мок действительно был вызван с ожидаемыми параметрами:
Мокито.Обучение(пбп_ПредопределенныеЗначения)
.Когда("ИнициализироватьПредопределенныеЗначения")
.Вернуть(ТаблицаКонфликтов)
.Прогон()
;
+
+ Мокито.Проверить(пбп_ПредопределенныеЗначения)
+ .Что("ИнициализироватьПредопределенныеЗначения")
+ .СПараметрами(ИмяОбъекта)
+ .БылВызван()
+ ;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Мокито.Обучение(пбп_ПредопределенныеЗначения) | |
| .Когда("ИнициализироватьПредопределенныеЗначения") | |
| .Вернуть(ТаблицаКонфликтов) | |
| .Прогон() | |
| ; | |
| Мокито.Обучение(пбп_ПредопределенныеЗначения) | |
| .Когда("ИнициализироватьПредопределенныеЗначения") | |
| .Вернуть(ТаблицаКонфликтов) | |
| .Прогон() | |
| ; | |
| Мокито.Проверить(пбп_ПредопределенныеЗначения) | |
| .Что("ИнициализироватьПредопределенныеЗначения") | |
| .СПараметрами(ИмяОбъекта) | |
| .БылВызван() | |
| ; |
There was a problem hiding this comment.
Ты ошибаешься, такой функции нет в yaxunit
There was a problem hiding this comment.
@denisushakov, спасибо за уточнение. Действительно, функции Мокито.Проверить нет в yaxunit. Принял к сведению для будущих ревью.
✏️ Learnings added
Learnt from: denisushakov
PR: firstBitSportivnaya/PSSL#157
File: src/cfe/YAXUnit/CommonModules/ОМ_ПредопределенныеЗначенияВызовСервера/Ext/Module.bsl:21-25
Timestamp: 2024-12-16T14:33:04.959Z
Learning: В фреймворке yaxunit отсутствует функция `Мокито.Проверить`.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Analysis Details0 Issues
Coverage and Duplications
Project ID: PSSL |
ivanmolodec
deleted the
feature/cover-predefined-values-unit-tests-93
branch
Summary by CodeRabbit
Новые функции
Исправления ошибок