Большинство замечаний к тестовому фреймворку, предоставленному в задании, справедливы, но сам фреймворк создавался исключительно для целей ТЗ и не применяется для других задач. На продуктах используются похожие подходы и принципы к автотестам, но используемые инструменты естественно отличаются от предоставленного, активно развиваются и находятся вместе с кодом проектов.
- считаю такой подход непродуктивным. "Основной-то фреймворк у нас нормальный, а этот джуновский код - только для тестового." (если это вообще правда) Если Вы тестируете кандидатов на джуновском коде, то и получите тех, кто более приспособлен к работе с джуновским кодом.
Более уважительным к времени кандидата был бы подход, где предоставлялся бы +- приближенный к реальному код, т.е. без детских ошибок и с документаций (или "самодокуентируемый" код, где названия сущностей дают недвусмысленное понимание того, что сущности делают). Иначе выходит, что кандидат, работая в своей карьере с норм кодом, должен на время тестового, нырнуть в фреймворк с грубыми ошибками, а после тестового на работе снова вынырнуть в норм код? Зачем?
Фидбэка ждал 2.5 дня.
В ходе знакомства с фреймворком для автоматизации, у меня возникало много вопросов, ответов на которые в коде я не нашёл. Обычно знакомство с такими объемными штуками происходит на онбординге, где будущего пользователя фреймворка сразу погружают в контекст, дают примеры использования, сообщают в чём фреймворк хорош и в чём бесполезен, возможные баги, неочевидности и т.п. У меня такой опции нет, поэтому оставляю фидбэк здесь в тексте.
В идеале (особенно если его часто изучают новые люди) для фреймворка иметь документацию. Можно как просто в тексте, так и в виде примеров, или описания в ///<summary> к типам и членам класса. Можно не для всего кода, но хотя бы для неочевидных мест.
Хорошие примеры документации (прямо на главной странице, в readmi): https://github.com/Cysharp/R3, https://github.com/modesttree/Zenject, https://github.com/neuecc/UniRx, https://github.com/bazyleu/UniState
Идея фреймворка интересная. Но я в последнее время в, скажем так, синьорном комьюнити Unity вижу больше подвижек в отделении моделей (POCO) от Unity корутин и MonoBehaviour. Плюсы такого подхода:
- тесты будут более легковесными, время выполнения тестов уменьшается.
- тесты становятся "пользователями кода", чем подталкивают программистов писать более качественный код. В предоставленном мне фреймворке же всё жёстко на корутинах. Разница подходов (вкупе с другими неочевидносятми в коде) ещё больше запутывает.
Стоит также добавить, что большие системы и наследование стоит использовать аккуратно. Пару слов об этом на этом стриме K-Syndicate (см таймкод 1:13:34): UNIT TESTING ⚡️ Правильно внедряем unit-тесты в Unity проект
Не говорю, что фреймворк плохой. Мой посыл в том, что отсутствие документации и примеров использования мешает хорошо вникнуть. Два пустых теста это не очень наглядно.
1.1 Методы с Get в названии по смыслу должны что-то возвращать. Здесь же (понял только когда открыл dll) должно быть AddX. Но лучше даже уточнить куда "Add" - в верстак, инвентарь, или добавляется бонус "к дереву" за срубку дерева.
1.2 должно быть "IsXEmpty", т.к. тут ничего не "считается"("Count" в названии не нужен).
1.2.1 Метод CountIsEmpty, исходя только из названия и возвращаемого типа, скорее бы выглядел как-то так:
*просто для примера. В предоставленном проекте Inventory и Cell другие.
1.3 Под методом TreeFelled могла бы скрываться как логика IsTreeFelled, так и MarkTreeAsFelled, где return это bool markesSuccessfully
1.4 GetGmGo(); чтобы понять что значит сокращение "Gm", пришлось снова лезть в dll.
2.1 Лучше не держать такую объёмную логику в конструкторе. И в любом случает лучше разбить на методы или локальные функции по нескольку строк. Code should read like well-written prose and be clean, lean, and easy to maintain.
2.2 в используемых в классе словарях ключом было бы лучше сделать enum. Это избавит от возможных опечаток, и это даст человеку, изучающему фреймворк посмотреть сразу список всех опций.
В идеале, эти словари обернуть в свои классы, чтобы их назначении стало понятнее.
Например, в UiTestContext.Inventory { get { return _buttons["inventory"]; } } //строка "inventory"
и в UiTest_Framework\UiTest\UiTestDll\UiTest\Context\Consts\Inventory\Inventory.Id //тоже строка "inventory"
При расширении кода кто-то может забыть, что всё слово в нижнем регистре, и попытаться обратиться через buttons["INVENTORY"] или buttons["Inventory"], в результате чего произойдёт ошибка. Лучше держать строку в одном месте, а во всех других местах всегда обращаться через Inventory.Id, или организовать отдельный класс Constants. Эта ситуация описывается в принципе DRY.
в половине методов Commands.cs. Лучше переиспользовать код в приватном методе/методах, если логика частично совпадает.
В половине реализующих IUiTestChecker (например, UseButtonIsActiveChecker.cs)
public void Init()
{
throw new System.NotImplementedException();
}6.1 UiTest\Scripts\Cheats.cs
Camera worldCamera = GameObject.Find("UICanvas").GetComponent<Canvas>().worldCamera;Метод GameObject.Find ненадёжный. Геймдизайнер может случайно переименовать имя GameObject, тогда поиск не найдёт ничего. Лучше искать хотя бы по типу: FindObjectOfType.
6.2 Это ещё ненадёжнее: сломается также если кто-то передвинет объекты в иерархии
public List<GameObject> FindTree( )
{
return ( from tree in GameObject.Find( "trees" ).GetComponentsInChildren<TreeContainer>() select tree.gameObject ).ToList<GameObject>();
}Всё это замедляет, и вынуждает нырять в кишки фреймворка.
Я привык писать тесты с Unity Test Framework, в отдельной .asmdef, но в том же проекте где и код игры. Когда сразу виден тестируемый код, то в тестах можно и переиспользовать некоторые DTO и типы с логикой, а также сразу можно проверить написанный тест на:
- ложноПоложительность: тест должен стать красным, если логика сломалась в сути.
- ложноОтрицательность: тест НЕ должен стать красным, если логика по сути не ломалась, если вносились косметические изменения. Т.е. после написания теста внести изменения в рабочий код, проверить поведение теста, и откатить изменения в рабочем коде. В предоставленном мне фреймворке код тестов отделён от кода проекта, что замедляет изучение.
Приятнее когда фреймворк именно помогает писать, а не загадывает загадки. Например, даёт удобный синтаксис утверждений по типу
- exampleObject.Value.Should().Be(20);
- Assert.AreEqual( 1, someArr.Length );
В предложенном фреймворке я таких методов для assert не уведел.
Я всё ещё готов к лайвкодингу. Но если там снова будет задача по этому фреймворку, то у меня все эти вопросы останутся. Я уверен, фреймворк может выполнять свои задачи. Но для тестового задания он выглядит грубовато. У не знакомого с этим кодом человека уйдёт на вкат в технологию слишком много времени и будет много вопросов. Вместо тестового будет "борьба с фреймворком".
Будет отлично, если засчитаете это review фреймворка как выполненное в таком необычном формате тестовое.