public class TaskBuilder {
/*
    да, ты развил структуру проекта)

    все эти классы - TaskBuilder, TaskStatus
    можно было бы расположить в Helpers
    разумно переименовать класс Helpers на GivenHelpers
    почитай про nested classes in java
    https://docs.oracle.com/javase/tutorial/java/javaOO/nested.html
    https://www.tutorialspoint.com/java/java_innerclasses.htm

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

    что касается класса TaskBuilder
    что он такого строит
    фактически - он является хранилищем данных о таске
    и у класса есть статические методы build - которые создают одну или несколько тасок

    т е - класс TaskBuilder объединяет в себе  2 задачи
        - хранить данные о таске
        - создавать таски
    Почитай о Single Responsibility Principle
         если кратко - у каждого класса - есть своя ответственность
         должен быть класс Task - хранилише данных о таске
         и можно реализовать класс TaskBuilder - который будет реализовывать методы
         создающие таску или таски
         но, честно говоря, если класс Task будет жить внутри GivenHelpers
         то там же, в GivenHelpers я бы поселила и эти методы
         не вижу большого смысла для таких 2-ух методов создавать отдельный класс
*/

   ....
    public String getName() {
    public TaskStatus getStatus() {
    /*
        http://stackoverflow.com/questions/5055177/example-of-getters-setters-as-a-bad-choice-in-java
        http://joxi.ru/Y2LXgYnfn6XnV2

        из вот таких соображений - можно не реализовывать геттеры в данном случае
    */
   ...
    public static List<TaskBuilder> build(TaskStatus taskStatus, String... taskNames){
    /*
        я бы реализовывала метод как возвращающий массив Task[]
        почему - ниже станет понятнее

        и в обоих методах build - в качестве первого параметра использовала TaskStatus taskStatus
        тоже - ниже станет понятнее
    */
************************************************
     public void given(TaskBuilder task) {
     public void given(List<TaskBuilder> tasks) {
/*
    реализация этих 2-ух методов - похожа

    был бы вариант given(Task... tasks)
     он бы покрыл варианты
        пустой список тасок
        одна таска
        несколько тасок

    и метод нужен был бы один - что более DRY
    и все варианты вызова этого метода - достаточно простыми были бы - не надо  собирать список
        given()
        given(build(ACTIVE, "a"))
        given(build(ACTIVE, "a"), build(COMPLETED, "b"))
        given(build(ACTIVE, "a", "b"))
*/
*****************************************************
public class BaseTest extends AllureAttachmentTest {

    protected TaskManagerPage page = new TaskManagerPage();
    /*
        не очень это хороший вариант - прятать в предка тест-класса - создание пейджа
        был у нас тут спор уже про это)

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

    @Before
    public void setUp() {
        open("https://todomvc4tasj.herokuapp.com/");
    }

    @After
    public void cleanUp() {
        executeJavaScript("localStorage.clear()");
    }

    /*
        мы наследуем тест-класс от этого класса

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

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

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

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

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

            вот и не нужен нам код - по открытию урла в бифор-методе
            сразу вопрос - какой предок нужен тест-классу)

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

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

public class AtTodoMVCTest extends BaseTest{
    protected TodoMVCPage todoMVCPage = new TodoMVCPage();
}

***********
    student
    Всім привіт.
    стосовно зауваження по ініціалізації пейджи в базовому тест класі https://www.refheap.com/120326
    "Соображения -
       даже в рамках тестов одного приложения не факт, что в разных тест-классах понадобидся одна и та же пейджа
       не хочется портить универсального предка для тест-класса
       хочется быть более KISS - чтобы не приходилось из тест-класса искать - что за пейджа у нас "

    Я залишусь при свої думці. В захист своєї позиції, що досить зручно коли ініціалізація всіх пейджів знаходиться в одному класі. А те що з тестового класу не видно що саме за пейджа, то у випадку комплексного веб додатку це вирішується лаконічними назвами змінних в парент тест-класі і звикається до цього досить швидко.

    yashaka
    ти порушуєш деякі принципи

    наприклад:
    Single Responsibility Principle

    твій бейс клас починає відповідати і за пейджі…
    хоча перед цим він відповідав за інше…
    А тепер він вже став “контейнером пейджів" (edited)

    більше того, ім’я твого класу нічого не каже про те що він є контейнером пейджів

    такий код - є магічним - тому що приховує важливу інформацію

    те що до цього можна звикнути - бо запам’ятати - це погана практика…
    бо колись твоя пам’ять зламається, якщо ти будеш забагато магії тримати в голові

    тести мають бути очевидними - це основне правило…

    і це значить що все що стосується тест логіки - має бути в тесті, навіть якщо воно дублюється

    тут KISS важливіше за DRY

    якщо тобі так сильно хочеться тримати всі пейджі в одному місці - то для цього варто створити окремо клас - контейнер пейджів
    назвати наприклад його App
    і вже через нього мати доступ до цих пейджів…

    можливо у вигляді статичних просто змінних
    щоб писати App.todoMVCPage
    ну чи просто
    App.page
    сокільки у нас інших пейджів немає…

    так що ти звісно можеш робити як завгодно, але при такому рішенні ти отримаєш не done, а accepted )
    бо ми таки не згодні з таким варіантом )

    якщо хочеш “зручності”
    то або створи окремо контейнер пейджів
    або забери з бейс теста пейджу і запхни в базовий тест клас з відповідним іменем
    типу AtTodoMVCPageTest
    щоб було зрозуміло за що цей клас відповідає

    але можеш ще нас переконати ) якщо знайдеш аргументи )

    >  А те що з тестового класу не видно що саме за пейджа,
    то у випадку комплексного веб додатку це вирішується лаконічними назвами змінних в парент тест-класі

    не зрозумів чому “лаконічні назви” допомагають “видимості” )

    проблема все одно є, і “я все запам’ятаю бо назви лаконічні і у мене хороша пам’ять” - це якийсь заслабкий аргумент

    не у всіх хороша пам’ять… плюс тести сапортяться часто тими хто їх не писав, наприклад програмістами -
    їх теж ти змусиш запам’ятати все що живе в базовому класі?

    > що досить зручно коли ініціалізація всіх пейджів знаходиться в одному класі

    “досить зручно” це не аргумент

    аргументом було б щось типу “це зручно тому що… “

    ну і знову ж таки… вище я писав - що "одне місце” можна реалізувати і іншим способом,
    більш “безпечним” і простим, не вимагаючим великих затра пам’яті всієї команди

    з приводу зручності - він хоче сказати що є “одна точка доступу” до всіх вже наперед ініціалізованих пейджів…
    але знову ж таки… сумнівна зручність тримати їх в базовому классі…

    мати до них доступ через окремий обьект контейнер типу App
    чи Pages
    імхо набагато зручніше тому що це явно
    і через крапочку можна отримати доступ тільки до пейджів, а не до ще чогось, що може жити в тому ж базовому класі

    при цьому хоча такий варіант з контейнером - я б прийняв як для done
    я все одно не вважаю це добрим рішенням
    бо імхо явно в тесті показати з якими пейджами іде робота  - набагато більш очевидніше і
    інформативніше ніж в кожен тест “передавати всю пачку пейджів"

    julia.v.iluhina
    т е мы заранее считаем - что нам в любых тест-классах нужен одинаковый набор пейджей?

    yashaka
    ну… ми не вважаємо - але це так, є мінусом цього рішення )

    що ми трохи “запудрюємо мозги” надлишковою інформацією )

    це як таскати з собою повний мішок інструментів “на всі випадки життя” :slightly_smiling_face:

    хоча можна було б передбачити що сьогодні тобі знадобиться тільки те то і те то

    і доречі… поки у нас всього одна пейджа…
    що означає - що нам App - якби трохи зайвий )
    і можна вважати що сам TodoMVC - і є єдиною точкою входу)

    таке… враховуючи що це “єдина точка входу і так” - то можна в принципі і винести її кудись в базовий тест клас…
    але все одно - треба створити його окремо, з гарним іменем типу AtTodoMVCPage
    вже й плюс заодно і винести туди прекондішен відкриття сторінкиі очистки даних…

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


    > Тоді якщо пейджа одна, чому її інітити в різних класах?

    тому що це ООП

    а в ньому нормально все інітити скрізь де потрібно заюзати

    якби це було модульне програмування - то там не треба нічого інітити

    > Прекондішини вже використовуються з хелпера, того виходить городити окремий клас в ієрархії
    наслідування заради пейджи тільки....

    ну так… правильно мислиш…
    ієрархіх наслідування - це складно, це не КІСС
    але точно така сама логіка з вбудовою пейджі в BaseTest
    ти теж ускладнюєш ієрархію наслідування - тим що “роздуваєш” один з класів в цій ієрархії,
    при цьому порушуючи SRP (Single Responsibility Principle)

    таке… тут звісно можна довго сперечатися...

*/
**************************************
public class TaskManagerPage extends Helpers{

/*
    а вот это неожиданно)

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

    но - для чего пейджу наследовать это все? )

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

    и она не будет противоречивой

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

    строим такую фразу
    TaskManagerPage - это тоже GivenHelpers
    ну, нет, я не могу согласиться с таким утверждением)

        можно GivenHelpers - реализовать как второй пейдж
        будет у тебя 2 пейджа
        GivenPage & TaskManagerPage

        а можно - внутри пейджа оперировать объектом GivenHelpers givens;
        тогда - немного придется подправить имена методов в GivenHelpers

        отложи пока это на конец работы с ревью
        потом поймешь, почему
*/
******************************************
http://joxi.ru/52akqzoUGMRwzr

/*
    1 - предков тест-класса - собери в пекедже testconfigs
    тут же, возле тест-классов

    2 - сомнительная польза от пекеджа test
    у нас же ветка =  src \ test \....
    т е  - для тестов

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

    public void testEdit(){

        page.given(build("a", ACTIVE));
        page.filterActive();

        page.startEdit("a", "a edited").pressEnter();
        page.assertVisibleTasks("a edited");
    }

    @Test
    public void testEdit(){

        page.given(build("a", COMPLETED));
        page.filterCompleted();

        page.startEdit("a", "a edited").pressEnter();
        page.assertVisibleTasks("a edited");
    }

    @Test
    public void testEditAtAll() {

        //given
        page.create("a");

        page.startEdit("a", "a edited").pressEnter();
        page.assertVisibleTasks("a edited");
        page.assertItemsLeft(1);
    }
/*
    смотрим на фиче-тесты для edit

    во всех 3-х методах - оперируем единственной таской

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

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

    примени это и к другим фиче-тестам

    еще момент

       мы получаем - что предварительные действия мы делаем в 2 шага
       page.given(build("a", ACTIVE));
       page.filterActive();

       сначала - готовим локалсторидж, потом - если надо - переходим на фильтр

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

       в GivenHelpers - реализуй например prepareTasks (фактически - это твой  given(Task... tasks))
       а набор гивен-методов - реализуй в пейдже TaskManagerPage
       и в методах - используй prepareTasks и методы переходов по фильтрам

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

*/
****************************
public class TodoMVCTest extends BaseTest {

/*
    тут у тебя останется - только е2е
    остальные - распредели по соответствующим тест-классам

    этот тест-класс можно будет назвать более детально -
    TodoMVCGeneralTest or TodoMVCIntegrationTest
*/
**********************************
CompletedFilterFTest
/*
    F - я бы не стала вот это F лепить в имя тест-класса

    это не устоявшаяся аббревиатура
    да и эта информация особо ничего не дает на этом уровне

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

    а отчет - покажет что там за тесты

    касается и других тест-классов
*/
**********************************
/*
    в задании не требуется, но можно оптимизировать покрытие

    в чем оптимизация
        - что покрыто в е2е - не покрываем фиче-тестами
        - низкоприоритетное - покрываем единожды, на каком-то из контекстов (пример - delete by emptying text)
        - низкоприоритетные варианты фичи, у которой есть покрытые варианты - не покрываем (пример - reopen all & add)

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

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

    полезнее оптимизировать

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

*/