Оптимизировал процесс подсчета coverage #120

eGavr · 2016-01-26T12:29:33Z

Описание

В текущей реализации coverage считается каждый раз после запуска тестов в определенном бандле, то есть после запуска тестов на desktop считается coverage для desktop, после запуска тестов на touch-pad считается coverage для desktop и touch-pad (хотя ранее уже был посчитан coverage для desktop), после запуска тестов на touch-phone считается coverage для desktop, touch-pad и touch-phone и т. д.

Решение

Запускать все тесты и подсчет coverage один раз, после сборки всех бандлов с тестами.

Тесты

Протестировал интеграционно

Важно

Ломает обратную совместимость!

Ревью

/cc @blond

anton-rudeshko · 2016-01-26T13:30:51Z

lib/plugin.js

@@ -11,10 +11,17 @@ var path = require('path'),
    XJST_EXPORT_NAME = 'BEMHTML';

 module.exports = function (helper) {
+    // необходимо знание про имена всех банлов, в которых запускаются тесты,


бандлов

С большой буквы коммент? А еще у нас часть шаблонизаторов может не иметь возможности считать каверадж. Такие, как старый bemhtml и handlebars. Мы это можем как-то учитывать?

А еще у нас часть шаблонизаторов может не иметь возможности считать каверадж

Уточни... Мы такие мы же явно указываем в опции для чего считать coverage.

anton-rudeshko · 2016-01-26T13:31:35Z

Ох костыль-то, костыль! :)

qfox · 2016-01-26T13:32:15Z

lib/runner.js

@@ -26,7 +26,8 @@ Runner.prototype.run = function (files) {
            failures ? defer.reject(new Error('tmpl-specs: ' + failures + ' failing')) : defer.resolve();
        }

-        if (opts.coverage.engines.length > 0) {
+        // если это последний бандл, в котором запускались тесты, то необходимо собрать coverage


Снова с маленькой и без точки

Посмотрел по уже имеющимся комментам, @blond пишет с большой буквы без точки

eGavr · 2016-01-26T13:59:33Z

Нашел багу, пока рано ревью делать.

Ох костыль-то, костыль! :)

В связи с текущей архитектурой, пока не нашли лучшего способа :(

Там скорее в istanbul-е проблема .

но зато ускоряет сборку :)

eGavr · 2016-01-26T14:34:42Z

@blond 🆙

eGavr · 2016-01-26T19:41:58Z

И снова нашел багу :)
На этот раз при интеграционном тестировании, пока рано делать ревью.

qfox · 2016-01-26T19:52:59Z

Оставь потомкам что-нибудь, что ты творишь!!!

blond · 2016-01-26T20:10:26Z

Я, наверное, не вовремя... но у Instrumenter есть опция для того, чтобы записывать результат в нужную глобальную переменную.

Может проще при сборке генерировать разное имя переменной для каждой платформы? Тогда всё останется примерно как есть, но лишних операций не будет.

Единственное над чем надо будет подумать, когда запускать репортеры для coverage: как сейчас на каждую платформу или один раз в конце.

eGavr · 2016-01-26T20:43:41Z

Я, наверное, не вовремя... но у Instrumenter есть опция для того, чтобы записывать результат в нужную глобальную переменную.

👍

Не вовремя – это когда все все влито, выпущены версии... с багами :)

а ты предалагаешь отличное решение на этапе разработки :)

Единственное над чем надо будет подумать, когда запускать репортеры для coverage: как сейчас на каждую платформу или один раз в конце.

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

blond · 2016-01-26T21:53:26Z

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

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

eGavr · 2016-01-26T22:04:23Z

Я немного не так понял твой последний вопрос про то, когда запускать репортеры для coverage :)

Давай вернемся снова к нему:

Единственное над чем надо будет подумать, когда запускать репортеры для coverage: как сейчас на каждую платформу или один раз в конце.

Полагаю, что в любом случае надо в конце ОДИН раз, а по другому вроде и никак, но теперь (раз coverage можно записывать в отдельные переменные) API не поломаем.

В общем идея реализации без поломки обратной совместимости уже появилась, кину код, будем ревьюить ;)

eGavr · 2016-01-27T14:06:50Z

@blond 🆙

blond · 2016-01-30T09:27:05Z

lib/runner.js

+        grep: (typeof process.env.BEM_TMPL_SPECS_GREP === 'undefined' ?
+            opts.grep :
+            process.env.BEM_TMPL_SPECS_GREP) || false,
+        diffOpts: diffOpts


Раз уж лодашим, давай по полной )

_.defaults(opts.htmlDiffer, { preset: 'bem' })

Вот так будет работать:

_.defaults(opts.htmlDiffer || {}, {preset: 'bem'});

eGavr · 2016-02-01T14:37:49Z

@blond 🆙 , please :)

eGavr · 2016-02-03T08:31:48Z

@blond 🆙

blond · 2016-02-03T09:38:21Z

CHANGELOG.md

+1.0.0
+-----
+
+### Крупные изменения


Давай назовём раздел API.

blond · 2016-02-03T09:47:05Z

Оптимизировал процесс подсчета coverage

blond added the in progress label Jan 26, 2016

anton-rudeshko reviewed Jan 26, 2016
View reviewed changes

qfox reviewed Jan 26, 2016
View reviewed changes

eGavr force-pushed the fix/coverage-collection branch 2 times, most recently from 7aff833 to afe8677 Compare January 26, 2016 14:32

eGavr force-pushed the fix/coverage-collection branch 2 times, most recently from 8b95549 to 0ef6bb8 Compare January 26, 2016 15:53

eGavr force-pushed the fix/coverage-collection branch 6 times, most recently from 6b05306 to 83b9a9c Compare January 27, 2016 14:06

eGavr force-pushed the fix/coverage-collection branch from 83b9a9c to e279e43 Compare January 27, 2016 14:32

blond reviewed Jan 30, 2016
View reviewed changes

eGavr force-pushed the fix/coverage-collection branch 11 times, most recently from 01c7dea to 63f280f Compare February 1, 2016 13:48

eGavr force-pushed the fix/coverage-collection branch 6 times, most recently from 68e1a1c to ef71a5b Compare February 3, 2016 08:28

eGavr added 2 commits February 3, 2016 11:30

Fix collection of coverage

af85764

Update README for v1.0.0

28ce721

eGavr force-pushed the fix/coverage-collection branch from ef71a5b to 077b452 Compare February 3, 2016 08:30

blond reviewed Feb 3, 2016
View reviewed changes

CHANGELOG.md

1.0.0

-----

### Крупные изменения

Copy link

Member

blond Feb 3, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Давай назовём раздел API.

Changelog for v1.0.0

f9e4cc0

eGavr force-pushed the fix/coverage-collection branch from 077b452 to f9e4cc0 Compare February 3, 2016 10:16

eGavr added a commit that referenced this pull request Feb 3, 2016

Merge pull request #120 from enb-bem/fix/coverage-collection

eb7df18

Оптимизировал процесс подсчета coverage

eGavr merged commit eb7df18 into master Feb 3, 2016

eGavr deleted the fix/coverage-collection branch February 3, 2016 14:25

blond removed the in progress label Feb 3, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Оптимизировал процесс подсчета coverage #120

Оптимизировал процесс подсчета coverage #120

eGavr commented Jan 26, 2016

anton-rudeshko Jan 26, 2016

qfox Jan 26, 2016

eGavr Jan 26, 2016

anton-rudeshko commented Jan 26, 2016

qfox Jan 26, 2016

eGavr Jan 26, 2016

eGavr commented Jan 26, 2016

eGavr commented Jan 26, 2016

eGavr commented Jan 26, 2016

qfox commented Jan 26, 2016

blond commented Jan 26, 2016

eGavr commented Jan 26, 2016

blond commented Jan 26, 2016

eGavr commented Jan 26, 2016

eGavr commented Jan 27, 2016

blond Jan 30, 2016

eGavr Jan 30, 2016

eGavr commented Feb 1, 2016

eGavr commented Feb 3, 2016

blond Feb 3, 2016

blond commented Feb 3, 2016

Оптимизировал процесс подсчета coverage #120

Оптимизировал процесс подсчета coverage #120

Conversation

eGavr commented Jan 26, 2016

Описание

Решение

Тесты

Важно

Ревью

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anton-rudeshko commented Jan 26, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eGavr commented Jan 26, 2016

eGavr commented Jan 26, 2016

eGavr commented Jan 26, 2016

qfox commented Jan 26, 2016

blond commented Jan 26, 2016

eGavr commented Jan 26, 2016

blond commented Jan 26, 2016

eGavr commented Jan 26, 2016

eGavr commented Jan 27, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eGavr commented Feb 1, 2016

eGavr commented Feb 3, 2016

Choose a reason for hiding this comment

blond commented Feb 3, 2016