Facade for plugins #589

DudaGod · 2016-09-07T15:52:53Z

No description provided.

coveralls · 2016-09-07T16:12:08Z

Coverage increased (+0.09%) to 83.691% when pulling 473bb5e on add/facade-for-plugins into bb234cf on master.

coveralls · 2016-09-07T16:43:51Z

Coverage increased (+0.09%) to 83.691% when pulling 4b4ec2b on add/facade-for-plugins into bb234cf on master.

tormozz48 · 2016-09-07T19:50:15Z

lib/plugins.js

-    load: function(gemini, options) {
-        _.chain(options.system.plugins)
+    load: (gemini) => {
+        _.chain(gemini.config.system.plugins)


_.chain(gemini.config.system.plugins) => _(gemini.config.system.plugins)

coveralls · 2016-09-07T22:18:11Z

Coverage increased (+0.09%) to 83.691% when pulling 49a6e91 on add/facade-for-plugins into bb234cf on master.

tormozz48 · 2016-09-13T11:24:28Z

lib/plugins.js

-              return plugin.opts === false;
-          })
-          .map(function(plugin) {
+          .map((pair) => ({name: pair[0], opts: pair[1]}))


.map(_.curry(_.zipObject)(['name', 'opts']))

/cc @eGavr

Андрюх, +1 за знание лодаша :)

но исходный вариант более понятный

тут можно вообще проще:

_(gemini.config.system.plugins) .map((opts, name) => ({name, opts}))

tormozz48 · 2016-09-13T11:32:10Z

🆗

eGavr · 2016-09-13T11:42:56Z

doc/programmatic-api.md

+
+Plugin example with the listening on the events:
+```javascript
+module.exports = function(gemini, options) {


а че не стрелочные функции в примере?

coveralls · 2016-09-14T09:24:29Z

Coverage increased (+0.2%) to 83.784% when pulling 5281ba8 on add/facade-for-plugins into bb234cf on master.

coveralls · 2016-09-14T09:24:29Z

Coverage increased (+0.2%) to 83.784% when pulling 5281ba8 on add/facade-for-plugins into bb234cf on master.

coveralls · 2016-09-14T09:24:29Z

Coverage increased (+0.2%) to 83.784% when pulling 5281ba8 on add/facade-for-plugins into bb234cf on master.

coveralls · 2016-09-16T11:10:50Z

Coverage increased (+0.1%) to 83.799% when pulling 1813a7a on add/facade-for-plugins into c970c52 on master.

coveralls · 2016-09-16T11:22:31Z

Coverage increased (+0.1%) to 83.799% when pulling 1813a7a on add/facade-for-plugins into c970c52 on master.

sipayRT · 2016-09-19T12:05:05Z

lib/runner/index.js

-        .fin(() => this.emit(RunnerEvents.END));
+        .fin(() => {
+            this.emit(RunnerEvents.END);
+            this.emit(RunnerEvents.END_RUNNER, this);


событие END_RUNNER должно кидаться раньше, чем END

в текущей логике это не так. Вообще событие END не должно бросаться, но это дело версии 5.x, кстати куда мы там пишем что нужно оторвать в новой версии? Надо дописать

На BEGIN вообще не нашел, чтобы кто то подписывался в gemini, а на END подписывается только flat репортер.
Пишем в этот issue #555. Кажется ему номер 555 не случайно достался =)
Дописал, что нужно выпилить события BEGIN и END

sipayRT · 2016-09-19T12:35:23Z

test/unit/plugins.js

+
+const proxyquire = require('proxyquire');
+
+let plugins;


а зачем эта переменная вне дескрайба?

sipayRT · 2016-09-19T12:46:28Z

test/unit/runner/index.js


            return run_()
-                .then(() => assert.callOrder(begin, end));
+                .then(() => assert.callOrder(begin, startRunner, end, endRunner));


и тут у тебя должен тест упасть. Хотя, кажется, по этому тесту видно, что последовательность событий неправильная

нет, не падает.

j0tunn · 2016-09-19T13:01:08Z

lib/gemini.js

        return getTests(paths, options)
            .then((suiteCollection) => {
-                return this.emitAndWait(RunnerEvents.START_RUNNER, runner)


кстати, скорее всего этим мы сломали обратную совместимость. Теперь те, кто работают с gemini через API (например gemini-gui) не имеют возможности подписаться на события от gemini. Так что имеет смысл пробрасывать сообщения от раннера наверх. Но тогда встает вопрос о необходимости фасада как такового

Да, теперь gemini-gui не может подписаться на события от gemini, проверил.
Что делаем?
Костыляем? )

Костыльнул, но что то мне вообще не нравится так.

мы можем пробрасывать наружу только START_RUNNER и END_RUNNER как это было раньше

То есть я правильно понял. Что нужно, чтобы не раннер эмитил события START_RUNNER и END_RUNNER, а gemini?

скажем так: gemini тоже должен их эммитить. Ну типа ок, мы унесли их в runner, но gemini должен их пробрасывать наверх

coveralls · 2016-09-19T15:59:18Z

Coverage increased (+0.1%) to 83.95% when pulling faec51b on add/facade-for-plugins into 3eae590 on master.

DudaGod · 2016-09-19T16:49:16Z

/cc @j0tunn @sipayRT

coveralls · 2016-09-19T17:07:15Z

Coverage increased (+0.1%) to 83.985% when pulling 9bc435d on add/facade-for-plugins into 3eae590 on master.

DudaGod · 2016-09-20T08:15:38Z

lib/runner/index.js

@@ -37,6 +41,7 @@ module.exports = class TestsRunner extends Runner {
        const suites = suiteCollection.allSuites();

        return q.fcall(() => {
+            this.emit(RunnerEvents.START_RUNNER, this);
            this.emit(RunnerEvents.BEGIN, {


/cc @SevInf привет. Не помнишь кто подписывается на данное событие? В gemini и плагинах не получается найти.

coveralls · 2016-09-20T08:55:49Z

Coverage increased (+0.1%) to 83.985% when pulling 3906430 on add/facade-for-plugins into 3eae590 on master.

coveralls · 2016-09-20T08:55:49Z

Coverage increased (+0.1%) to 83.985% when pulling 3906430 on add/facade-for-plugins into 3eae590 on master.

coveralls · 2016-09-20T08:55:49Z

Coverage increased (+0.1%) to 83.985% when pulling 3906430 on add/facade-for-plugins into 3eae590 on master.

coveralls · 2016-09-20T08:55:49Z

Coverage increased (+0.1%) to 83.985% when pulling 3906430 on add/facade-for-plugins into 3eae590 on master.

eGavr · 2016-09-21T09:24:15Z

test/unit/gemini.js

    });

    afterEach(() => sandbox.restore());

+    it('should passthrough "START_RUNNER" and "END_RUNNER" events', () => {
+        sandbox.stub(temp, 'init');


давай стаб наверх вынесем

я не могу его вынести, он тогда ломает другие тесты.

eGavr · 2016-09-21T09:26:04Z

test/unit/runner/index.js

+            const onStartRunner = sandbox.spy().named('onStartRunner');
+            runner.on(RunnerEvents.START_RUNNER, onStartRunner);
+
+            return run_().then(() => {


странное форматирование :)

eGavr · 2016-09-21T09:26:47Z

test/unit/runner/index.js

+            const onEndRunner = sandbox.spy().named('onEndRunner');
+            runner.on(RunnerEvents.END_RUNNER, onEndRunner);
+
+            return run_().then(() => {


и снова странное форматирование

eGavr · 2016-09-21T09:27:24Z

/ok

j0tunn · 2016-09-21T10:00:03Z

/ok

coveralls · 2016-09-21T11:46:08Z

Coverage increased (+0.1%) to 83.985% when pulling ab7ea60 on add/facade-for-plugins into 712ce38 on master.

coveralls · 2016-09-21T12:01:28Z

Coverage increased (+0.1%) to 83.985% when pulling ab7ea60 on add/facade-for-plugins into 712ce38 on master.

coveralls · 2016-09-21T16:02:38Z

Coverage increased (+0.1%) to 83.985% when pulling ceaea7f on add/facade-for-plugins into 712ce38 on master.

j0tunn · 2016-09-30T15:28:20Z

lib/runner/index.js

@@ -37,6 +41,7 @@ module.exports = class TestsRunner extends Runner {
        const suites = suiteCollection.allSuites();

        return q.fcall(() => {
+            this.emit(RunnerEvents.START_RUNNER, this);


@DudaGod Дима, вот это беда. Мы эммитили это событие (как и END_RUNNER) через emitAndWait. А теперь мы не ждем результата. Это очень плохо, т.к. на это событие может выполняться (и выполняется) всякая тяжелая инициализация + если в этой инициализации есть ошибки, мы их не видим

levonet added the review label Sep 7, 2016

tormozz48 reviewed Sep 7, 2016
View reviewed changes

DudaGod force-pushed the add/facade-for-plugins branch from 4b4ec2b to 49a6e91 Compare September 7, 2016 22:00

tormozz48 reviewed Sep 13, 2016
View reviewed changes

eGavr reviewed Sep 13, 2016
View reviewed changes

sipayRT suggested changes Sep 19, 2016

View reviewed changes

j0tunn suggested changes Sep 19, 2016

View reviewed changes

DudaGod force-pushed the add/facade-for-plugins branch from c656c61 to faec51b Compare September 19, 2016 14:54

j0tunn changed the title ~~Add/facade for plugins~~ Facade for plugins Sep 20, 2016

DudaGod commented Sep 20, 2016

View reviewed changes

sipayRT approved these changes Sep 20, 2016

View reviewed changes

eGavr reviewed Sep 21, 2016

View reviewed changes

j0tunn approved these changes Sep 21, 2016

View reviewed changes

add gemini facade for plugins

ab7ea60

DudaGod force-pushed the add/facade-for-plugins branch from eac2182 to ab7ea60 Compare September 21, 2016 11:34

DudaGod force-pushed the add/facade-for-plugins branch from ceaea7f to ab7ea60 Compare September 21, 2016 23:08

DudaGod merged commit e3c4fc0 into master Sep 21, 2016

DudaGod deleted the add/facade-for-plugins branch September 21, 2016 23:24

j0tunn reviewed Sep 30, 2016

View reviewed changes

Facade for plugins #589

Facade for plugins #589

Conversation

DudaGod commented Sep 7, 2016

coveralls commented Sep 7, 2016 • edited Loading

coveralls commented Sep 7, 2016 • edited Loading

Choose a reason for hiding this comment

coveralls commented Sep 7, 2016 • edited Loading

tormozz48 Sep 13, 2016 • edited Loading

Choose a reason for hiding this comment

eGavr Sep 13, 2016 • edited Loading

Choose a reason for hiding this comment

j0tunn Sep 16, 2016 • edited Loading

Choose a reason for hiding this comment

tormozz48 commented Sep 13, 2016

Choose a reason for hiding this comment

coveralls commented Sep 14, 2016 • edited Loading

coveralls commented Sep 14, 2016

coveralls commented Sep 14, 2016

coveralls commented Sep 16, 2016 • edited Loading

coveralls commented Sep 16, 2016 • 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

DudaGod Sep 19, 2016 • 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

coveralls commented Sep 19, 2016 • edited Loading

DudaGod commented Sep 19, 2016

coveralls commented Sep 19, 2016 • edited Loading

Choose a reason for hiding this comment

coveralls commented Sep 20, 2016 • edited Loading

coveralls commented Sep 20, 2016

coveralls commented Sep 20, 2016

coveralls commented Sep 20, 2016

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

eGavr commented Sep 21, 2016

j0tunn commented Sep 21, 2016

coveralls commented Sep 21, 2016 • edited Loading

coveralls commented Sep 21, 2016 • edited Loading

coveralls commented Sep 21, 2016 • edited Loading

j0tunn Sep 30, 2016 • edited Loading

Choose a reason for hiding this comment

coveralls commented Sep 7, 2016 •

edited

Loading

coveralls commented Sep 7, 2016 •

edited

Loading

coveralls commented Sep 7, 2016 •

edited

Loading

tormozz48 Sep 13, 2016 •

edited

Loading

eGavr Sep 13, 2016 •

edited

Loading

j0tunn Sep 16, 2016 •

edited

Loading

coveralls commented Sep 14, 2016 •

edited

Loading

coveralls commented Sep 16, 2016 •

edited

Loading

coveralls commented Sep 16, 2016 •

edited

Loading

DudaGod Sep 19, 2016 •

edited

Loading

coveralls commented Sep 19, 2016 •

edited

Loading

coveralls commented Sep 19, 2016 •

edited

Loading

coveralls commented Sep 20, 2016 •

edited

Loading

coveralls commented Sep 21, 2016 •

edited

Loading

coveralls commented Sep 21, 2016 •

edited

Loading

coveralls commented Sep 21, 2016 •

edited

Loading

j0tunn Sep 30, 2016 •

edited

Loading