Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Полная информация об исключении при загрузке файлов с дымовыми тестами в случае конфигурации с поддержкой синхронности #1096

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

BarinovIN
Copy link

частичная реализация #1095

сделанные изменения

  • Доработал обработку исключения при загрузке файла в загрузчике каталога. Метод "Сообщить" в итоге ничего не сообщал. Теперь причина пропуска файла кратко будет видна в логе выполнения и подробно в отладочном логе и ЖР.
  • Вызываю исключение при ошибке в загрузке файла, т.к. уже нельзя будет считать, что все тесты выполнены успешно, даже если остальные файлы загрузятся и других ошибок не будет. Но не нашёл возможности отсюда взвести флаг ошибки для тестов в целом. Пропускать незагрузившийся файл без изменения статуса - искажение результатов тестирования.

@vanessa-opensource/Collaborators - просьба прокомментировать и проверить

Copy link
Collaborator

@artbear artbear left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Важные вопросы

ПолныйТекстОшибки = ПодробноеПредставлениеОшибки(ИнформацияОбОшибке);
КраткийТекстОшибки = КраткоеПредставлениеОшибки(ИнформацияОбОшибке);

ПолныйТекстОшибки = КонтекстЯдра.СтрШаблон_(ШаблонСообщения, ФайлОбработки.ПолноеИмя, ПолныйТекстОшибки);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

уже можно\нужно юзать простое СтрШаблон вместо КонтекстЯдра.СтрШаблон_

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Заменил. Может в методе СтрШаблон_ тогда заменить тело на вызов простого СтрШаблон и пометить метод как устаревший, чтобы было понятнее, что его уже использовать не нужно? Или я не до конца понимаю его назначение?

КраткийТекстОшибки = КраткоеПредставлениеОшибки(ИнформацияОбОшибке);

ПолныйТекстОшибки = КонтекстЯдра.СтрШаблон_(ШаблонСообщения, ФайлОбработки.ПолноеИмя, ПолныйТекстОшибки);
КраткийТекстОшибки = КонтекстЯдра.СтрШаблон_(ШаблонСообщения, ФайлОбработки.Имя, КраткийТекстОшибки);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

аналогично заменить на СтрШаблон

// Тут достаточно взвести флаг ошибки для всех тестов целиком и продолжить, но непонятно как взвести флаг ошибки отсюда.
// Поэтому пока вызываем исключение.
// Лучше ведь починить и перезапустить, чем пропустить ошибку из-за незагруженного файла с тестами.
ВызватьИсключение;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

проверял в пакетном режиме?

имхо при вызове исключения здесь основной фреймворк тестирования может упасть,
а тогда сеанс 1С просто зависнет и останется в памяти, не завершившись.

а что происходит в интерактивном режиме?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

В пакетном режиме.

Выводится сообщение:

Ошибка загрузки и выполнения тестов в пакетном режиме
И дальше подробная информация об ошибке

Менеджер тестирования завершается, а клиент действительно остаётся открыт. Но так он может оставаться и из-за других исключений.
Срабатывает вот этот перехват исключения, возможно в нём стоит добавить закрытие клиентов тестирования так же, как вот здесь. Тогда ничего зависшего оставаться не будет.

В интерактивном режиме.

Фреймворк не закрывается, появляется два сообщения.

  • Короткое:
Не удалось загрузить файл ИмяФайла.epf
Причина исключения
  • И подробное описание ошибки со стеком вызовов.

Итого

По-моему поведение приемлемое. Но как я уже писал, исключение в этой строке хотелось бы убрать, заменив на установку статуса ошибки для тестов в целом.

@@ -189,7 +189,25 @@
КонецЕсли;

Исключение
Сообщить("Не удалось загрузить файл " + ФайлОбработки.ПолноеИмя + Символы.ПС + ПодробноеПредставлениеОшибки(ИнформацияОбОшибке()));
ШаблонСообщения = НСтр("ru = 'Не удалось загрузить файл %1
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

остается главный вопрос - почему исправление сделано в Загрузчике каталога, а не в загрузчике файла?

1 ведь основную работу выполняет именно загрузчик файла

2 и в коде обработчика из ПР используются те же данные, что передаются в загрузчик файла - полное имя файла

Предлагаю перенести код Попытки-Исключение в метод Загрузить загрузчика файлов

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@BarinovIN а в целом я с исправлением согласен, только нужно код перенести в загрузчик файла и тогда я смержу.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

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

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Перенёс перехват исключения в загрузчик файла.

@artbear artbear added this to the 6.9.0 milestone Jul 16, 2023
@artbear artbear changed the title Полная информация об исключении при загрузке файлов с дымовыми тестами Полная информация об исключении при загрузке файлов с дымовыми тестами в случае конфигурации с поддержкой синхронности Jul 16, 2023
@artbear
Copy link
Collaborator

artbear commented Jul 16, 2023

Уточнил заголовок, т.к. ПР решает проблему только для конфигураций с поддержкой синхронности

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

а в них обработка сложнее, т.к. простого исключения недостаточно, остановится только текущая асинхронная функция, а не весь фреймворк ((

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

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

см: vanessa-opensource#1095
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants