Add unit test and integration test #34

madurmanov · 2018-10-22T10:13:24Z

No description provided.

dima117 · 2018-11-03T20:00:43Z

.hermione.conf.js

+    'html-reporter/hermione': {
+      path: 'hermione/hermione-html-report'
+    },
+    [path.resolve(__dirname, './hermione/custom-commands/index.js')]: true


удобнее было настроить в package.json обращение к собственному плагину как к пакету npm
https://medium.com/@the1mills/how-to-test-your-npm-module-without-publishing-it-every-5-minutes-1c4cb4b369be

dima117 · 2018-11-03T20:04:09Z

controllers/__test__/contentController.test.js

+  it('В представление прокидывается набор данных, состоящий из заголовка, хлебных крошек и содержимого файла коммита коммита', async () => {
+    await controller(request, response, () => {}, stubs);
+
+    const params = response.render.getCall(1).args[1];


если запустить этот тест отдельно от первого, то он упадет

dima117 · 2018-11-03T20:04:52Z

controllers/__test__/contentController.test.js

+
+    const params = response.render.getCall(1).args[1];
+
+    expect(params).to.have.all.keys(


не проверяется контент полученного объекта

dima117 · 2018-11-03T20:06:15Z

controllers/__test__/contentController.test.js

+    { type: '', hash: '', path: '' }
+  ]));
+  stubs.gitFileContent = sinon.stub();
+  stubs.buildBreadcrumbs = sinon.stub();


эти переменные - общие для всех тестов на контроллер
тесты не должны использовать общие экземпляры объектов

dima117 · 2018-11-03T20:07:03Z

controllers/__test__/contentController.test.js

@@ -0,0 +1,38 @@
+const { expect } = require('chai');
+const sinon = require('sinon');


круто, что получилось разобраться с библиотекой sinon.js

dima117 · 2018-11-03T20:13:57Z

controllers/contentController.js

-            content
-          });
-        } else {
-          next();


кажется, во время рефакторинга сломалась 404 ошибка, если запросить несуществующий путь

dima117 · 2018-11-03T20:14:54Z

controllers/contentController.js

-  const path = req.params[0].split('/').filter(Boolean);
+  const path = req.params[0].split('/').filter(Boolean).join('/');
+
+  const content = await _gitFileTree(hash, path).then(


здесь смешиваются промисы и async/await
лучше придерживаться одного синтаксиса

еще, кажется, если при получении контента произойдет 500 ошибка, то все равно вызовется render (из 19 строки)

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

dima117 · 2018-11-03T20:17:10Z

controllers/__test__/contentController.test.js

+
+describe('Контроллер содержимого файла коммита', () => {
+
+  const request = { params: { hash: '', '0': '' } };


чтобы создавать заглушки для запроса и ответа express, удобно использовать готовые библиотеки вроде https://www.npmjs.com/package/sinon-express-mock

dima117 · 2018-11-03T20:18:28Z

controllers/__test__/filesController.test.js

+
+const controller = require('../filesController');
+
+describe('Контроллер файловой системы коммита', () => {


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

dima117 · 2018-11-03T20:20:22Z

controllers/contentController.js

+const { buildBreadcrumbs } = require('../utils/navigation');
+
+module.exports = async function(req, res, next, ...stubs) {
+  const _stubs = (stubs && stubs[0]) || {};


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

dima117 · 2018-11-03T20:27:06Z

controllers/filesController.js

@@ -16,26 +17,32 @@ function buildObjectUrl(parentHash, { path, type }) {
  }
 }

-module.exports = function(req, res, next) {
+module.exports = async function(req, res, next, ...stubs) {


те же ошибки, что и в других контроллерах

dima117 · 2018-11-03T20:30:37Z

hermione/custom-commands/index.js

@@ -0,0 +1,18 @@
+const assert = require('assert');
+
+module.exports = (hermione, opts) => {


круто, что получилось написать плагин для гермионы

dima117 · 2018-11-03T20:31:58Z

hermione/navigation.hermione.js

+  it('Переход со списка файлов на вложенную папку', function() {
+    return this.browser
+      .url('/files/f135c5ab9197d1d0e3f5d6eb5e3da2a3a2125f3a/')
+      .assertNavigation('.files__link--tree', '.content--files');


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

dima117 · 2018-11-03T20:37:10Z

package.json

+    "html-reporter": "^2.22.0",
+    "mocha": "^5.2.0",
+    "ndb": "^1.0.26",
+    "nyc": "^13.1.0",


хорошо, что получилось попробовать настроить code coverage report

dima117 · 2018-11-03T20:38:35Z

utils/__test__/git.test.js

+  it('Количество возвращаемых элементов соответствует переданным параметрам', () => {
+    gitHistory(2, 1, executeGit);
+
+    expect(executeGit.getCall(0).args[1][4]).to.be.equal(1);


по записи .args[1][4] не понятно, что это за данные
лучше присвоить в промежуточную переменную с понятным названием

dima117 · 2018-11-03T20:39:48Z

utils/__test__/git.test.js

+
+describe('Файловая система коммита', () => {
+
+  const executeGit = sinon.stub();


снова общий stub на все тесты
если запустить тесты по отдельности, они упадут

dima117 · 2018-11-03T20:40:07Z

utils/__test__/git.test.js

+  it('Правильно разбирается история коммитов', async () => {
+    const history = await gitHistory(1, 1, executeGit);
+
+    expect(history[0]).to.have.all.keys(


не проверяются значения полей

dima117 · 2018-11-03T20:41:58Z

utils/__test__/git.test.js

+  const executeGit = sinon.stub();
+  executeGit.returns(Promise.resolve('1\t2\t3\t4'));
+
+  it('Количество возвращаемых элементов соответствует переданным параметрам', () => {


насколько я понимаю, в тесте проверяется, что номер и размер страницы корректно прокидываются в git
если так, то название теста не соответствует его смыслу

dima117 · 2018-11-03T20:43:19Z

utils/__test__/git.test.js

+  });
+
+  it('При выборе папки из коммита, возвращается список файлов этой папки', () => {
+    const fileTree = gitFileTree('hash', 'path', executeGit);


название не соответствует содержимому теста

dima117 · 2018-11-03T20:44:20Z

utils/__test__/navigation.test.js

+
+  it('В хлебных крошках присутствует элемент, указывающий на главную страницу', () => {
+    const breadcrumbs = buildBreadcrumbs();
+    expect(breadcrumbs).to.have.lengthOf(1);


здесь не проверяется присутствие элемента history, как указано в названии теста

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

dima117 · 2018-11-03T20:45:08Z

utils/__test__/navigation.test.js

+
+  it('Последний элемент хлебных крошек никуда не ссылается', () => {
+    const breadcrumbs = buildBreadcrumbs('hash', 'path/path');
+    expect(breadcrumbs[breadcrumbs.length - 1]).to.not.have.property('href');


в этих тестах вообще не проверяется контент в полученных хлебных крошках

dima117 · 2018-11-03T20:50:38Z

utils/git.js

@@ -38,10 +43,10 @@ function gitHistory(page = 1, size = 10) {
    '-n',
    size
  ]).then(data => {
-    return data
+    return (DEV ? require('./gitHistoryStub') : data)


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

также получается, что в интеграционных тестах не проверяется взаимодействие с git

dima117 · 2018-11-03T20:53:00Z

controllers/__test__/indexController.test.js

+
+const controller = require('../indexController');
+
+describe('Контроллер истории коммитов', () => {


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

madurmanov added 19 commits October 20, 2018 23:18

Prepare project for testing

7067b6b

Add navigation test

fd190a8

Add git test

836c7b1

Prepare hermione for integration testing

f135c5a

Add pages view integration test

48069b3

Add result of pages view testing

89d2d14

Fix indent in README.md

426f26b

Fix commits history bug on testing

41b4852

Add navigation integration test

c33d743

Add firefox browser for integration test

d623e18

Add hermione custom plugin for navigation testing

e771faf

Update unit test

8aada0a

Update integration test

2c3ce0e

Update gitHistoryStub

5eca78f

Update logic blocks in README.md

45ef97f

Update navigation unit-test

1d46cbe

Update git unit-test

33dc103

Remove excess describe with file path from utils test

99ead7d

Add controllers unit-test

81c4929

dima117 reviewed Nov 3, 2018

View reviewed changes

Repository owner deleted a comment from madurmanov Nov 3, 2018

dima117 reviewed Nov 3, 2018

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add unit test and integration test #34

Add unit test and integration test #34

madurmanov commented Oct 22, 2018

dima117 Nov 3, 2018

dima117 Nov 3, 2018

dima117 Nov 3, 2018

dima117 Nov 3, 2018 •

edited

Loading

dima117 Nov 3, 2018

dima117 Nov 3, 2018 •

edited

Loading

dima117 Nov 3, 2018

dima117 Nov 3, 2018 •

edited

Loading

dima117 Nov 3, 2018

dima117 Nov 3, 2018 •

edited

Loading

dima117 Nov 3, 2018

dima117 Nov 3, 2018

dima117 Nov 3, 2018

dima117 Nov 3, 2018

dima117 Nov 3, 2018

dima117 Nov 3, 2018

dima117 Nov 3, 2018

dima117 Nov 3, 2018

dima117 Nov 3, 2018

dima117 Nov 3, 2018

dima117 Nov 3, 2018

dima117 Nov 3, 2018

madurmanov Nov 3, 2018

dima117 Nov 3, 2018

dima117 Nov 3, 2018 •

edited

Loading

dima117 Nov 3, 2018


		const params = response.render.getCall(1).args[1];

		expect(params).to.have.all.keys(

		@@ -0,0 +1,38 @@
		const { expect } = require('chai');
		const sinon = require('sinon');


		describe('Контроллер содержимого файла коммита', () => {

		const request = { params: { hash: '', '0': '' } };


		const controller = require('../filesController');

		describe('Контроллер файловой системы коммита', () => {

		@@ -0,0 +1,18 @@
		const assert = require('assert');

		module.exports = (hermione, opts) => {


		describe('Файловая система коммита', () => {

		const executeGit = sinon.stub();


		const controller = require('../indexController');

		describe('Контроллер истории коммитов', () => {

Add unit test and integration test #34

Are you sure you want to change the base?

Add unit test and integration test #34

Conversation

madurmanov commented Oct 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dima117 Nov 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dima117 Nov 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dima117 Nov 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dima117 Nov 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dima117 Nov 3, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dima117 Nov 3, 2018 •

edited

Loading

dima117 Nov 3, 2018 •

edited

Loading

dima117 Nov 3, 2018 •

edited

Loading

dima117 Nov 3, 2018 •

edited

Loading

dima117 Nov 3, 2018 •

edited

Loading