questions - https://github.com/AleksanderPopov/automician_course/blob/master/questions_and_review/12_questions.txt
==================================================================================
1. "    любые операции - замедлят работу тестов
        файловые операции - не самые быстрые и не самые надежные
        да и вообще - любые операции - могут дать ошибки
        из этих соображений - если что-то можно не делать - лучше не делать - я про работу тестов

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

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

        но вообще - я считаю - не стоит оно того, ни в каком варианте)"

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

                    не согласен что не надо чистить, чистить средствами мавена - хорошее решение, мне нравится

                    если знаешь как это можно сделать, чтобы еще можно было включать-выключать, скажи пожалуйста)
/*
    ну давай представим такую машину)

    проще всего представить машину с CI Server-ом
    например Jenkins-ом
    на котором запускаются джобы

    проектов много, настроенных Job - соответственно

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

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

    а раз так - сомнительно, что нужно их чистить

    ну допустим, хочется чистить
    ну ок, чисть в рамках post build Action (если мы о дженкинсе говорим) и делай mvn clean

    или  - запускай сначала
    mvn clean test - тесты выполнили
    mvn site - отчеты нагенерили,
    ... - куда-то там их сохранили
    mvn clean - опять все зачистили

    я никогда этого не делала - не чистила папки ПОСЛЕ запуска тестов )
    и пока твоих доводов мне не хватило - чтоб я начала так делать )

    так что рассуждаю - считай о коне в вакууме сферическом

    всегда - перед запуском тестов - выполняю clean и этого с головой достаточно (мне)

    если знаешь как это можно сделать, чтобы еще можно было включать-выключать, скажи пожалуйста - вот это не поняла
    что включать-ваключать
    если про то как вызывать - то как писала выше
    зависит от того - как тесты запукаются, каким инструментом
*/
===================================================================================
2. "    т е - в списке тасок будут вот такие таски именно вот в такой последовательности?
        активная 1
        закомпличеная compl2
        активная 3

        судя по старому коду - нет)
        а должно быть именно так"

            >>>>>>>> а у меня сейчас просто сначало все активные, потом все закомпличенные)
                    нужно сделать так чтоб они как пишутся в тестах, в таком порядке и добавлялись?
/*
    ну да, как мы задали таски, чтоб точно так, в той же последовательности они добавлялись)
    а то - нагородили кода, билдер-паттерны и все такое)
    а такого простого не обеспечили)
*/
===================================================================================
3. "    про селенидовский рефреш - первое что делай - когда вопрос возникает - смотри исходники
        вот посмотри например на код open  - https://github.com/codeborne/selenide/blob/master/src/main/java/com/codeborne/selenide/impl/Navigator.java
        там куча вариантов
        в том числе - и странички, открываемые локально

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

            >>>>>>>> я как раз смотрел, и там же и увидел что нет такого метода как refresh(),
                    а есть скрытый navigate(url), и спросил, может ты знаешь почему так)

                    странное решение, учитывая что как вот показывает практика оно не особо
                    работает в рядовых кейсах
/*
    это не рядовой кейс)
    т к вот эти странички - по фильтрам которые - ведут себя ... не совсем как разные странички с разными урлами)
    тут мы еще и на особенности реализации этой версии todoMVC наступили)
    так что случай не совсем рядовой)

    и ты б всего вот этого не заметил
    если бы в рамках гивен-методов - делал переходы используя линки переходов по фильтрам)

    но мы ж сразу наворачиваем)
    за что и заплатили)
*/
===================================================================================
4. "    не буду спорить)
        когда-то и я с Яковом не соглашалась в этом вопросе)

        теперь соглашаюсь)
        просто держала в голове - что есть еще и такой подход

        как-то на живом проекте - увидела серьезнейшие выгоды в эффективности подхода
        при том - что в точности не потеряли
        и теперь соглашаюсь)

        а поначалу никак не могла согласиться)"

            >>>>>>>> может когда я увижу что действительно в точности не потеряно,
                    поменяю мнение) пока я вижу что куски функционала остаются плохо
                    проверенными, или вообще непроверенными при таком подходе)
/*
    ну, повторяться не будем
    пока держи в голове - что есть и такой подход
    может когда-то придешь к такому же выводу)
*/
===================================================================================
5. "http://joxi.ru/Dr860ybh4oqdQ2
    /*
        исходя из того - что такие классы могут беть переиспользованы не только для todoMVC - разумно вынести это по структуре выше
        в пекедж core (или api)

        GivenHelpers - в пекедже given или helpers
        что-то более близкое по смыслу
        точно не api это
    */"

        >>>>>>>> согласен, переместил гивены для todomvc в соотв. пекедж
/*
    ок
*/
===================================================================================
6. "public class AllureHelper {

        @Attachment(value = "Page-screenshot", type = "image/png")
        public static byte[] publishScreenshot() {
            String path = getPathOf(screenshot());
            ...
        }

        @Attachment(value = "Page-source", type = "text/html")
        public static byte[] publishHtml() {
            String path = getPathOf(screenshot());
            ...
    /*
        тут можно чуть оптимальнее

        если нам нужно приаттачить и скриншот, и html
        то получится - что мы дважды вызываем screenshot()

        помимо того что это не оптимально
        еще может быть вот что
        вызвали publishScreenshot() - зафиксили скриншот
        ситуация успела поменяться
        потом вызываем publishHtml() - получаем  html, который к только что приаттаченному скриншоту - никакого отношения не имеет
        можно долго разбираться в таких дебрях)"

            >>>>>>>> я переделал, можно теперь вот так вызывать

                    AllurePublisher.publishNow().screen();

                    AllurePublisher.publishNow().pageSource();

                    AllurePublisher.publishNow().screenAndSource();

                    вроде норм, можно было бы затулить без иннер класса прям в паблишер,
                    но мне почему то показалось красивее так сделать, чем вызывать
                    длинные методы типа

                    AllurePublisher.publishScreenAndSource();

                    получается мы будем использовать этот класс или в руле, или когда нужно сфоткать
                    в конкретный момент что то, и я сразу хочу отделить визуально интерфейсы для разных
                    задач, мол если оформляешь рул, то onFail, если сейчас то publishNow

                    ну вот чет просто мне показалось что так будет красиво и просто вншне)
/*
    Не)
    я предлагала тебе вариант покрасивее)

    в моем варианте - каждый класс отвечал за свое
    Твое решение - нарушает Single Responsibility Principle

    получился более сложный вариант
    советую переделать

    скажем так, настаивать не стану в категорической форме
    но и не жди от меня - что я соглашусь - что текущее решение - красивое

    все может быть стройнее гораздо
    к тому же, мной предложенный вариант - его и нарастить сможешь другими какими-то паблишерами

    в общем - делать или нет - на твое усмотрение )
    мое мнение ты понял)

    Не факт, что Яков согласился бы со мной или с тобой)
    Он тоже фанат Single Responsibility Principle )
*/
===================================================================================
7. "    и дальше это можно использовать
        @Rule
        public Publisher publisher = Publisher.always().withAttachments(allureAttachment().withScreenshot().withHtml().build()).createPublisher();"

            >>>>>>>> это трешак какой то, читается плохо и выглядит страшно) как по мне сейчас проще
/*
    как по мне - трешак сейчас), к этой версии
    когда в одном классе - понапихано всякого

    для варианта always() - на фига вообще рула)
    гораздо проще заюзать
    After-метод с вызовом красивого лаконичного паблишера

    если с рулой все же хочется
    и мной предложенная строка не нравится
    может - можно как-то упростить выражение
    объявить переменную выше и присвоить ей allureAttachment().withScreenshot().withHtml().build()
    и уже затем - использовать ее для рулы

    для меня всегда индикатор - когда я не в состоянии придумать имя переменной
    то может и не стоит это придумывать)

    ну и наверное - можно имена как-то поравнять
    станет лучше)
    про это проще думать - когда структурно и функционально классы реализованы
    все работает
    потом дать точные лаконичные имена  - проще

    дальше - или реализуй мной предложеный вариант
    и сам подумай над именами всех классов и методов
    или - давай останемся при своих мнениях и остановимся по данному вопросу

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

*/
===================================================================================
8. "private void addTasksToLocalStorage() {
            StringBuilder tasksJson = new StringBuilder("");
            tasksJson.append(doJsonfromList(true, activeTasks))
                    .append(tasksJson.toString().isEmpty() ? "" : ",")
                    .append(doJsonfromList(false, completedTasks));
    /*
        вот - видишь
        получаем - всегда будут идти вначале - активные таски
        потом - закомпличеные
        как бы мы их не добавляли сами - картинка будет только такая

        а надо - как добавляли
        чтоб в такой последовательности они и шли"

            >>>>>>>> не совсем понял что ты имела ввиду, как ты видишь имплементацию корректную,
                    но я перепедалировал, вроде все ок, ну работает так точно, и кода меньше стало,
                    я там еще пару косяков исправил у себя)
/*
    посмотрела на текущую реализацию
    теперь таски добавятся в том же порядке
    что и задан при вызове
    что ок
*/
===================================================================================
8. "   public enum Filter {
            ALL(""),
            ACTIVE("active"),
            COMPLETED("completed");

            private String subUrl;

            Filter(String subUrl) {
                this.subUrl = subUrl;
            }

            @Override
            public String toString() { return "https://todomvc4tasj.herokuapp.com/#/" + this.subUrl; }
        }
    /*
        чуть более DRY вариант предлагаю

        не настаиваю - т к твой вариант более KISS )
        а это тоже важно)

        лично для себя я держу такую грань
        то что юзаю в тест-методах - должно быть более KISS
        а вот сама реализация пейджей/виджетов/хелперов/кондишенов - тут уже просто правила разработки ПО применяю
        DRY - тут уже важно )
        т е - снаружи все эти  пейджи/виджеты/хелперы/кондишены = то что видно при использовании этого всего
        должно конечно обеспечивать KISS
        а вот внутри - там где мы алгоритмы реализуем, какую-то сравнительно серьезную логику - там уже значимость DRY - гораздо выше

        потому бы - для enum Filter
        выбрала более DRY реализацию"

            >>>>>>>> крутая идея, спасибо! согласен про DRY, звучит логично)
                    типа то что могут видеть и неопытные автотестеры, или
                    новички, должно выглядеть просто, а то что внутри уже
                    должно быть эффективно)
/*
    )
    да, мысль простая
    )
    но рабочая
*/
===================================================================================
9. "return tasks.findBy(cssClass("editing"))
                    .find(" .edit")
    /*
        тут - пробел перед  .edit не дает ровно ничего

        метод find для SelenideElement - ищет строго внутренние элементы
        согласно цсс селектору = маске

        так что от этого пробела - ни холодно, ни жарко"

            >>>>>>>> та я случайно оставил, провтыкал )))
/*
    надеюсь, разбор по линке в описании этой ошибки - тебе что-то новое открыл )
    в свое время для меня такой эффект был неожиданностью
*/
===================================================================================
10. "    */
             given().completed("compl1")
                     .atAllFilter()
                     .prepare();

             add("2");
             /*
                 в этот момент = у тебя 2 таски в списке
                 значит - операция edit("2"
                 не проверит результат предыдущей операции add("2");
                 т к проверит состояние только одной из 2-ух тасок"

                    >>>>>>>> минуточку) мы говорили что гивен не проверяем,
                            а операция add не трогает ничего кроме создающейся
                            таски, поэтому я проверяю edit("2"), и после этого
                            остаюсь со всем проверенным (гивен + таск2)
/*
    мы результат гивена - и не проверяем
    нету же проверки после вызова гивена
    и не надо
    речь только про это
    что тут, после вызова гивен-метода - не нужна проверка

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

    это уже вопрос не к гивену
    а к тестовой ситуации
    если у тебя одна таска в списке - то да
    проверка через следующее действие над единственной таской - уместна
    а так - нет

*/
===================================================================================
11."        edit("2", "edited2");
            shouldHave("compl1", "edited2");

            goActive();
            /*
                следующее действие - лишь проверит что "edited2" - есть
                а состояние всего списка - не проверено

                я б только тут добавляла вторую таску
                после проверки состояния списка
            */"

                >>>>>>>> в смысле состояние не проверено? а что проверяет shouldHave,
                        если не состояние всего списка?
/*
    нужна проверка после перехода на Active фильтр

    shouldHave("compl1", "edited2"); - она ПЕРЕД переходом на фильтр
    а тут речь о проверке ПОСЛЕ перехода
*/
===================================================================================
12. "    @Test
         public void basicTasksFlow() {
         /*
             перемудрил малость ты с тестом)
         */
             given().completed("compl1")
                     .atAllFilter()
                     .prepare();

             add("2");
             /*
                 в этот момент = у тебя 2 таски в списке
                 значит - операция edit("2"
                 не проверит результат предыдущей операции add("2");
                 т к проверит состояние только одной из 2-ух тасок

                 не торопись добавлять таску
                 ты ж можешь и compl1 отредактировать

                 кстати - не советую так таску называть)
                 вот это - активная или закомпличеная - очень легко меняется в процессе
                 и выносить это в имя таски - имхо - перебор

                 и еще - не сокращай слова
                 если сокращение не общепринятое - не используй его
             */
             edit("2", "edited2");
             shouldHave("compl1", "edited2");

             goActive();
             /*
                 следующее действие - лишь проверит что "edited2" - есть
                 а состояние всего списка - не проверено

                 я б только тут добавляла вторую таску
                 после проверки состояния списка
             */
             cancelEdit("edited2", "notedited2");
             toggle("edited2");
             /*
                 проверка?
                 идем по кругу)
             */

             goCompleted();
             /*
                 проверка нужна
             */
             destroy("edited2");
             shouldHave("compl1");
             /*
                 как-то без имени пейджа - shouldHave("compl1") - по-сиротски выглядит)
                 не понятно - кто это кому должен)
                 читай чуть ниже
             */
             toggle("compl1");
             /*
                 нужна проверка
                 эххх)
             */

             goAll();
             shouldHave("compl1");
         }"

            >>>>>>>> половину не понял наездов, но осознал что проверок
                    не хватает, добавил)
/*
    еще раз расскажешь мне про наезд - будешь ждать ревью от Якова)
    мне вот этого цирка еще от тебя не хватало)

    наездов нет, не было и не будет
    есть только разбор кода
    и усе)

    посмотрю на код, откомменчу и приведу правильный вариант - если потребуется
*/
===================================================================================
13. "кстати - Tasks.shouldHave("compl1") - уже ниче так) попонятнее)
    а если не хочешь явно указывать для методов пейджа - имя его класса Tasks
    то тогда лучше для проверок используй имена assert....
    так оно логичнее будет"

            >>>>>>>> я переименовал shouldHave в shouldBe, без названия класса
                    выглядит норм, с названием тоже, не хочу assert.... добавлять метод)
/*
    пока не вижу особой разницы - shouldBe или shouldHave

    код уже откомменчу
    пока сложно окончательно что-то сказать

    но на первый взгляд - вряд ли оно помогло)
    если только shouldBe вместо shouldHave и более ничего
*/
===================================================================================
14. "    @Step
         public static ItemsLeft itemsLeft() {
             return new ItemsLeft();
         }

         public static class ItemsLeft {
             private static final SelenideElement element = $("#todo-count>strong");

             public void shouldBe(int itemsCount) {
                 element.shouldHave(exactText(String.valueOf(itemsCount)));
             }
         }
     /*
         для подхода с пейджами-модулями
         я бы продолжила и тут тему

         Tasks.ItemsLeft.shouldBe...

         т е - от метода itemsLeft() - избавилась бы)

         раз пейджи-модули - знач пейджи-модули)
         никаких объектов)"

            >>>>>>>>> звучит логично, но не совсем согласен) во первых если писать
                    уже ItemsLeft.shouldBe() то логично приписывать еще и Tasks.ItemsLeft

                    а если писать Tasks, то и во все остальные вызовы тоже нужно тулить
                    имя класса, чего не хотелось бы.

                    плюс еще и @Step аннотацию не удобно было бы в случае с иннер-классом
                    тулить к методу shouldBe(int i), т.к. в отчете было бы не понятно, что
                    там должно быть где куда)

                    кароче удалил все, оставил метод shouldBeItemsLeft().
/*
    да, есть такое с пейджами-модулями)
    верно подметил
    что репортиться будут только методы
    и тут конечно может быть засада)

    твой вариант - имеет право на жизнь
    но про формулировки - еще посмотрим)
    тут пока не уверена
    надо читать код
*/
===================================================================================
15. По замечаниям по именованию, покрытию, и шатанию:
        - ставлю везде Tasks.
            * т.к. чтоб не было коллизий методов пейджи с методами тестов
            * чтоб можно было нормально вставлять в аллюр itemsLeftShouldBe
        - переименовал нормально все тестовые методы
        - проверил что они соответствуют содержимому)
        - удалил SmokeTest т.к. это почти то же самое что и TasksIntegrationFlowTest
        - items left проверяю как договаривались, на одном контексте только.
            т.е. все вариации покрыты, просто на разных фильтрах, а не на всех сразу
/*
    буду читать код)
    про items left - в е2е стоит покрыть единожды вообще, это да
    но в фиче-тестах - эта проверка еше и уточняет проверку тестовой ситуации
    помимо того что мы покрываем и проверку этой фичи по пути

    действительно, если из фиче-тестов убрать проверку items left
    то не во всех тестах будет достаточно проверить состояние списка тасок
    в общем - если я тебя правильно поняла конечно
    ты с фиче-тестами погорячился)

    буду читать код)
*/
===================================================================================