Skip to content
This repository has been archived by the owner on May 9, 2021. It is now read-only.

Test front end #32

Closed
dyuvzhenko opened this issue Oct 10, 2017 · 92 comments
Closed

Test front end #32

dyuvzhenko opened this issue Oct 10, 2017 · 92 comments
Assignees
@dhilt @dyuvzhenko

Comments

@dyuvzhenko
Copy link
Collaborator

Обсуждение
@dyuvzhenko dyuvzhenko mentioned this issue Oct 10, 2017
Closed
@dyuvzhenko
Copy link
Collaborator Author

@dhilt У меня пока что нет ни малейшего понимания как тестировать front end. Но я так понимаю главной целью будет протрясти все существующие actions и то, как должен выглядеть reducers после actions? Как тестировать внешний вид или какое-то пользовательское взаимодействие с UI - совсем не представляю.

@dyuvzhenko
Copy link
Collaborator Author

@dhilt И собственно где в файловой структуре должны будут писаться эти тесты. Возможно в самой верхней папке ./test по соседству c back end? Допустим, создадим папку ./test/frontend, далее создадим _shared.js, в который будем импортировать файл ./app/index.js. А дальше, хм, возможно надо с помощью webpack'а собирать все приложение и как-то внутри браузера запускать тест...

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Oct 10, 2017 •
edited
Loading

@dhilt Больше всего я пока что уверен только в том, что весь тестирующий код должен как-то запуститься именно в браузере. Как если бы написали весь этот код и просто за-inject'или в html-страничку.

@dhilt
Copy link
Owner

dhilt commented Oct 10, 2017

@bitden На сервере мы используем mocha+chai, на клиенте я бы посмотрел в сторону jest+chai. Надо брать готовую инфраструктуру, посмотри https://blog.logrocket.com/testing-react-applications-part-1-of-3-ebd8397917f3 например...

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Oct 10, 2017 •
edited
Loading

@dhilt Пока что сделал только первый бессмысленный тест - bdfe2b7.
Папка test/frontend должна помимо папки components еще и actions содержать, но пока что без подключенного redux просто было нечего там написать как пример.

Jest'а там пока нет, но двигался я по документации от этой библиотеки - http://facebook.github.io/jest/docs/en/tutorial-react.html#content.

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Нужно как-нибудь в пределах frontend-тестов создавать store, чтобы можно было connect проводить к компонентам в тестах.

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Кажется нашел подход к этому вопросу - http://redux.js.org/docs/recipes/WritingTests.html.
Сегодня попробую закоммитить нормальные тесты для actions и components.

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Oct 11, 2017 •
edited
Loading

@dhilt Сделал первый вроде бы вменяемый образец тестирования actions и отражения данных в redux - 803f89f.
Только не могу никак заставить работать nock.js, не получается корректно сымитировать http-запрос (803f89f#diff-a811e103fdc6b10d9ada1f7b20d515e8R99), застрял пока что на этом...

@dhilt
Copy link
Owner

dhilt commented Oct 11, 2017

@bitden Я надеюсь сегодня-завтра добраться до кода, рад, что у тебя получается! главное, следовать устоявшимся практикам

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Тесты вроде бы доделал-починил - 05a2b27.

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Хотя с этим имитированием запроса запутался, может это и неверный сам по себе тест. В общем, тут нужно экспертное мнение :)

@dyuvzhenko
Copy link
Collaborator Author

@dhilt И еще добавил тест на случай неудачного добавления термина - de41af2.
Плюс немного упростил код.

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Oct 13, 2017 •
edited
Loading

@dhilt Покрыл тестами actions/common.js - f59fe1f.
Плюс добавил защиту на сервер, чтоб ответ гарантированно приходил, даже если это ошибка (до этого запрос GET_COMMON_DATA_ASYNC зависал на неограниченное время не выдавая ни ошибки, ни результата).

@dhilt
Copy link
Owner

dhilt commented Oct 13, 2017

@bitden Это все звучит очень обнадеживающе! я еще не смотрел эту часть, надеюсь, скоро добраться

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Oct 13, 2017 •
edited
Loading

@dhilt Ну, лучше не торопитесь, еще есть некоторые нюансы которые надо доделать. Сейчас вот понял в чем была проблема о которой говорил. Жаловался на nock и якобы неправильное имитирование запроса на сервер, на самом деле я просто по глупости подсовывал пустой state в коде и функции, которые по идее берут данные внутри себя с помощью getState, оставались ни с чем.
Сегодня постараюсь все что есть довести до нормального вида.

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Oct 13, 2017 •
edited
Loading

@dhilt На NewTerm.spec теперь точно можно посмотреть, мне кажется теперь там все так, как и должно быть - 6deded3.

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Хотя вру, нужно еще кое-что сделать :)

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Хотя нет, не врал, наверное все же сделал все что мог.
Просто заметил что возможно не было смысла добавлять кучу store.dispatch'ей, ибо они судя по всему никак не меняют store. Но с другой стороны без них nock-запрос не хочет работать...

@dhilt
Copy link
Owner

dhilt commented Oct 14, 2017 •
edited
Loading

@bitden Я начал смотреть тесты, работа проделана большая... Для того, чтобы запускаться под Win, я перевел работу с окружением на cross-env, это потребует npm install перед следующим запуском. Также подготовил global.window.localStorage для работы в тестовом окружении, пока через выставление объекта в _shared, который импортируется теперь всегда первым. У меня вопрос: как выставить последовательность выполнения спек (наподобие mocha.opts)?

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Вроде как всю документацию просмотрел, погуглил, но ничего не нашел на этот счет. Нашел только ключ --runInBand, позволяющий запускать тесты последовательно.

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Работа может и большая, но очень возможно по части имитации асинхронного запроса на сервера с помощью nock - совершенно бесполезная.

@dhilt
Copy link
Owner

dhilt commented Oct 14, 2017

@bitden Мне очень понравилась концепция тестирования actions, поскольку там сосредоточена бизнес-логика, и с этой позиции имитирование fetch'ей делает это полноценным: мы полностью проходим каждый отдельный action creator как независимую часть бизнес-логики приложения.

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Oct 14, 2017 •
edited
Loading

@dhilt С самой концепцией тестирования все в порядке, я говорю о реализации. Есть пока что много сомнений насчет nock, правильно ли написан код, связанный с ним. Нужно наверное поподробнее эту библиотеку поизучать.

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Насчет nock'а ошибался, проблемы были с созданием store.

    // test async actions
    let store = mockStore(initialState);
    store.dispatch(actions.setUserId(userId));
    store.dispatch(actions.changeAdminUserPassword({password: password}));
    store.dispatch(actions.changeAdminUserPassword({confirmPassword: confirmPassword}));
    
    nock('http://localhost/')
      .patch('/api/users/' + userId, {
        payload: {
          password: store.getState().admin.editUserPassword.password,
          confirmPassword: store.getState().admin.editUserPassword.confirmPassword
        }
      })
      .reply(500, expectedErrorResponse);
    
    return store.dispatch(actions.updateAdminUserPasswordAsync()).then(() => {
      expect(store.getActions()).toEqual(expectedErrorActions);
    });

После подобного кода ожидалось что в store отразятся новые данные после трех dispatch'ей, но там увы ничего не было. Но store.getActions() показывал все прошедшие actions. Если же в store пропихнуть уже обработанный initialState, то все получается вполне здорово:

let expectedStateBeforeRequest = JSON.parse(JSON.stringify(initialState));
    expectedStateBeforeRequest.admin.editUserPassword.id = userId;
    expectedStateBeforeRequest.admin.editUserPassword.password = password;
    expectedStateBeforeRequest.admin.editUserPassword.confirmPassword = confirmPassword;
    let store = mockStore(expectedStateBeforeRequest);

    nock('http://localhost')
      .patch('/api/users/' + userId, {
        payload: {
          password: store.getState().admin.editUserPassword.password,
          confirmPassword: store.getState().admin.editUserPassword.confirmPassword
        }
      })
      .reply(200, expectedSuccessResponse);

    return store.dispatch(actions.updateAdminUserPasswordAsync()).then(() => {
      const successAction = store.getActions().find(elem => elem.type === types.UPDATE_ADMIN_USER_PASSWORD_END);
      let successNotification = store.getActions().find(elem => elem.type === types.CREATE_NOTIFICATION);
      delete successNotification.notification.timer;
      expect(successAction).toEqual(expectedSuccessActions[1]);
      expect(successNotification).toEqual(expectedSuccessActions[2]);
    });

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Oct 15, 2017 •
edited
Loading

@dhilt Это всё в общем означает что по части асинхронных actions все тесты какие есть написаны неверно и бессмысленно. Буду думать как бы иначе store инициализировать, чтобы после dispatch'ей все данные отражались в state.

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Судя по всему библиотека redux-mock-store и не подразумевает изменений в state: reduxjs/redux-mock-store#71

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Oct 15, 2017 •
edited
Loading

@dhilt Значит у меня остается только один вопрос насчет тестов. Следующий код будет считаться подходящим?

  it('should work correctly: function updateAdminUserPasswordAsync', () => {
    const userId = translators[0].id;
    const password = 'new_password';
    const confirmPassword = 'new_password';
    const expectedSuccessResponse = {
      success: true,
      user: translators[0]
    };
    const expectedSuccessActions = [
      { type: types.UPDATE_ADMIN_USER_PASSWORD_START },
      {
        type: types.UPDATE_ADMIN_USER_PASSWORD_END,
        error: null,
        result: true
      },
      {
        type: types.CREATE_NOTIFICATION,
        idLast: 1,
        notification: {
          id: 1,
          text: 'EditUserPassword.new_password_success',
          ttl: 3000,
          type: 'success',
          values: {}
        }
      }
    ];

    // test reducers
    let expectedState = JSON.parse(JSON.stringify(initialState));
    let changedInitialState = JSON.parse(JSON.stringify(initialState));
    changedInitialState.admin.editUserPassword.password = password;
    changedInitialState.admin.editUserPassword.confirmPassword = confirmPassword;
    expectedState.admin.editUserPassword.error = null;
    expectedState.admin.editUserPassword.pending = false;
    expectedState.admin.editUserPassword.password = '';
    expectedState.admin.editUserPassword.confirmPassword = '';
    expect(reducer(changedInitialState, expectedSuccessActions[1])).toEqual(expectedState);

    // test async actions
    let expectedStateBeforeRequest = JSON.parse(JSON.stringify(initialState));
    expectedStateBeforeRequest.admin.editUserPassword.id = userId;
    expectedStateBeforeRequest.admin.editUserPassword.password = password;
    expectedStateBeforeRequest.admin.editUserPassword.confirmPassword = confirmPassword;
    let store = mockStore(expectedStateBeforeRequest);

    nock('http://localhost')
      .patch('/api/users/' + userId, {
        payload: {
          password: store.getState().admin.editUserPassword.password,
          confirmPassword: store.getState().admin.editUserPassword.confirmPassword
        }
      })
      .reply(200, expectedSuccessResponse);

    return store.dispatch(actions.updateAdminUserPasswordAsync()).then(() => {
      const successAction = store.getActions().find(elem => elem.type === types.UPDATE_ADMIN_USER_PASSWORD_END);
      let successNotification = store.getActions().find(elem => elem.type === types.CREATE_NOTIFICATION);
      delete successNotification.notification.timer;
      expect(successAction).toEqual(expectedSuccessActions[1]);
      expect(successNotification).toEqual(expectedSuccessActions[2]);
    });
  });

@dhilt
Copy link
Owner

dhilt commented Oct 24, 2017

@bitden Вот что надо сделать, Денис! #34

@dhilt
Copy link
Owner

dhilt commented Oct 30, 2017

@bitden Ты бы не мог в рамках этой задачи разобраться с

Warning: Accessing PropTypes via the main React package is deprecated, and will be removed in React v16.0. Use the latest available v15.* prop-types package from npm instead. For info on usage, compatibility, migration and more, see https://fb.me/prop-types-docs

?

Я бы ввобще ликвилировал все эти проверки на PropTypes. И можно ли вынести конфигурацию jest в, скажем ./test/frontend/jest.config.js?

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Oct 30, 2017 •
edited
Loading

@dhilt Можно, вынес - c89e006.
Только ключи --runInBand и --watchAll нельзя в конфиг перенести, как я понял. Ключ --runInBand и вовсе возможно стоит убрать.

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Да, с PropTypes конечно же попробую разобраться, это так и так было в планах, просто временно сосредоточился исключительно на тестах и забросил эти предупреждения.
Я так понимаю, версии библиотек React'а и PropTypes менять нельзя при решении этой задачи?

@dhilt
Copy link
Owner

dhilt commented Oct 30, 2017

@bitden Можно, главное чтобы приложение работало так же, как раньше. Чем свежее stable версия используемого пакета, тем лучше.

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Nov 1, 2017 •
edited
Loading

@dhilt Немного обновил версии библиотек - 6efde5e. И у меня зародилось ощущение, будто с каждым моим обновлением библиотек в package-lock.json добавляется куча requires: {..., с вашим же обновлением все эти requires: {... удаляются.

Но проблемы с предупреждениями по PropTypes и createClass остались. И как я понял, проблема в библиотеке react-bootstrap (по крайней мере если проследить по стек-трейсу через браузерную консоль откуда сыпется ошибка, я оказывался в дебрях именно bootstrap'а. Ну, и если из components/App убрать компоненты, содержащие react-bootstrap, все становится здорово). Обновил react-bootstrap - предупреждения не пропали. Или возможно какая-то другая библиотека испольует необновленный bootstrap, надо еще пошерстить это все.

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Nov 1, 2017 •
edited
Loading

@dhilt Сглупил немного и не последнюю версию react-bootstrap'а поставил. Сейчас все в порядке, поставил самую свежую и все warning'и пропали - 8f76762.

@dhilt
Copy link
Owner

dhilt commented Nov 1, 2017

@bitden Я знал, что дело в Бутстрапе и я рад, что тебе удалось разобраться с этим. С package-lock.json я поступаю периодически таким образом: удаляю node_modules и package-lock.json и выполняю npm install. Я пока еще не очень доверяю package-lock.json, и когда в package.json происходят хоть сколько-нибудь серьезные изменения, я полностью обновляю все зависимости.

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

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Nov 1, 2017 •
edited
Loading

@dhilt Я тут кстати вспомнил, что хотел весь сервер тщательно протрясти на предмет корректных reject/resolve (консоль ругалась что reject'ы не обрабатываются должным образом или что-то вроде того) еще после создания тестов на actions. И плюс мы хотели CRUD-методы организовать. Может стоит сейчас породить бранч crud-methods от test-frontend и там над этими двумя задачами подумать? Думаю, проблема с корреткным перехватом reject/resolve будет приоритетнее тестов, тем более что скоро планируете передавать права админа и все такое.

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Nov 1, 2017 •
edited
Loading

@dhilt Ну и параллельно в бранче test-frontend продолжу писать тесты. Конфликтов между бранчами быть вроде не должно.

@dhilt
Copy link
Owner

dhilt commented Nov 1, 2017 •
edited
Loading

@bitden Я сделал мерж в мастер. До конца недели я планирую закончить review по функциональной части, не трогая тестирование. Ты можешь продолжать работу вне мастера.

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Могу продолжать работу именно по тестированию, или по двум направлениям, которые описал?

@dhilt
Copy link
Owner

dhilt commented Nov 2, 2017

@bitden Это на твое усмотрение. Основные работы я считаю законченными, и мне надо привести это в порядок, хотя бы в собственной голове.

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Мне кажется еще не хватает одной детали. Сейчас на клиенте только админ может менять описание/имя/язык у переводчика. Может стоит такие права дать и самому переводчику?

@dhilt
Copy link
Owner

dhilt commented Nov 2, 2017

@bitden Нет, этого делать надо. Все подобные изменения должны проходить через Админа.

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Хорошо, не буду.

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Nov 3, 2017 •
edited
Loading

@dhilt Так, вроде бы дописал все тесты для клиента. Только компоненты Login и Languages не до конца поддались тестированию. В Login'е не выходит нащупать по data-test-id все элементы из modal'а. Languages же не показывает функцию onSelected, чтобы отработать в тестах нажатие кнопки по смене языка. И возможно еще есть какие-то несущественные мелочи в других спеках, еще буду смотреть, проверять это все.

Не хватает для полноты картины только тестирования роутов, и проверки на то, может ли текущий пользователь смотреть на роут /translator/:id/edit, допустим. Было бы здорово это сделать как end-to-end тестирование. По одной спеке для админа/пользователя/переводчика. И в этих спеках делать проход по всем существующим роутам.

@dyuvzhenko
Copy link
Collaborator Author

@dhilt В общем почти дописал более внятную версию тестов для компонентов в бранче test-frontend-2, осталось только с Login.spec.js разобраться и все будет готово.

Сам проект собственно еще живой как таковой? Как вижу dharmadict.ru не работает, да и вы затаились)

@dhilt
Copy link
Owner

dhilt commented Dec 11, 2017 via email

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Dec 11, 2017 •
edited
Loading

@dhilt Убрал зависимости, которые нигде не используются - 2c87a01.
Surge, как я понял, используется для деплоя на сервер, и если вы его не использовали в этих целях, видимо он лишний в зависимостях.
Также убрал ava и ava-redux, которые по-видимому должны применяться для тестирования как аналог mocha.
Пакет react-addons-test-utils должен был применяться при чистом тестировании с помощью jest, но поскольку все тесты компонентов выстроились на enzyme, тот пакет как бы лишний. Забыл его удалить похоже, пока искал подходы и библиотеки для тестирования компонентов.
Пакеты snazzy и standard предназначены для красивого вывода в консоль, но вроде как нигде их не применяли и не собирались.

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

  1. Пакет bcryptjs похоже подразумевался для создания хешей и прочих штук, связанных с шифрованием или чего-то в этом роде. Но у нас в проекте вроде как есть passwordhash для таких дел. И почему этот пакет лежал не в ./prod-директории...
  2. Назначение пакета btoa, честно говоря, не до конца понимаю пока. Перегон символов разных шифровок в base64-кодировку и обратно, зачем это собственно нужно.

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Dec 11, 2017 •
edited
Loading

@dhilt И поздравляю с новой версией angular-ui-scroll 👍
Если есть еще какие-то идеи или предположения что можно было бы сделать, то я буду рад постараться.

@dhilt
Copy link
Owner

dhilt commented Dec 11, 2017 via email

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Конечно же хочу посмотреть. Завтра постараюсь разобраться в коде, что, где, и как.
Мне значит делать форк, и там пытаться делать что-либо с кодом?

@dhilt
Copy link
Owner

dhilt commented Dec 11, 2017 via email

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Желание то есть всегда, а вот насчет толка не шибко уверен.
В любом случае сделал форк, еще нужно будет мне вспомнить все эти сущности из angular'а и как они работают вместе и начну пытаться что-то сделать. И думаю это может затянуться надолго.

@dhilt
Copy link
Owner

dhilt commented Dec 12, 2017 via email

@dyuvzhenko
Copy link
Collaborator Author

dyuvzhenko commented Dec 15, 2017 •
edited
Loading

@dhilt Чтобы обсуждать всяческие моменты, нужно понимание всего процесса - с этим дела все еще плохи)
Пока что думаю над надобностью тега <ng-content></ng-content> в template, у ./ui-scroll-directive/ui-scroll.component.ts. Вроде как этот тег просто передает содержимое, вложенное в тег с директивой *uiScroll (сейчас как я понимаю с ng-content можно получить всего одну строку на ui-scroll.component.ts, а не массив с данными, которые можно скроллить). А по затее, наверное, то что вложено в тег с директивой, должно просто показывать как пользователь этой библиотеки хочет отображать данные в одной строке из многих в скроллере. Ну и что-то вроде *ngFor должно быть наверное на ./ui-scroll-directive/ui-scroll.component.ts, а не на app.component.ts. Воот, это пока первые мысли, возможно не очень понятные.

@dhilt
Copy link
Owner

dhilt commented Dec 15, 2017 via email

@dyuvzhenko
Copy link
Collaborator Author

@dhilt Issue создал - dyuvzhenko/ngx-ui-scroll#1.
И нет, не правильно. Я говорил о том, что вижу в бранче dir, в папке ./src/app/ui-scroll-directive, и выше лежащих файлах ./src/app/app.component.ts и ./src/app/app.component.html.

@dhilt dhilt closed this as completed Jan 22, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@dhilt dhilt

@dyuvzhenko dyuvzhenko

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dhilt @dyuvzhenko