Skip to content
This repository has been archived by the owner on Sep 21, 2022. It is now read-only.

Allow to launch one WD session per suite #147

Merged
merged 3 commits into from Apr 16, 2015
Merged

Conversation

SevInf
Copy link
Contributor

@SevInf SevInf commented Apr 14, 2015

sessionMode option added which allows to switch between
perBrowser (1 session for browser, current behavior) and
perSuite (1 session for each suite).
To support both old and new behavior along with parallelLimit
setting, browser launcher was replaced with 3 pool classes.

New browser is requested from pool before each suite.

In perSuite mode, one of the following classes will be used:

  • UnlimitedPool, when parallelLimit is not set
  • LimitedPool when parallelLimit is set

In perBrowser mode, limited or unlimited pool will be wrapped
in CachingPool which allows to share a session between multiple
suites.

As long as calibration is needed to be performed only once for
each browser id, it was moved to separate Calibrator class. This
allows to perform only once for each browser even in perSuite mode.

@SevInf
Copy link
Contributor Author

SevInf commented Apr 14, 2015

/cc @j0tunn

@coveralls
Copy link

Coverage Status

Coverage increased (+1.3%) to 63.8% when pulling 048e483 on feature/session-per-suite into 3f13891 on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+1.3%) to 63.8% when pulling 048e483 on feature/session-per-suite into 3f13891 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.3%) to 63.8% when pulling bec1d32 on feature/session-per-suite into 3f13891 on master.

@@ -73,6 +73,9 @@ browsers:
* `debug` (CLI: `--debug`, env: `GEMINI_DEBUG`) – включить отладочный вывод в терминал.
* `parallelLimit` – число браузеров запускаемых `Gemini` параллельно.
По умолчанию запускаются все, но иногда (например, при использовании облачных сервисов вроде Sauce Labs) это число необходимо контролировать.
* `sessionMode` — позволяет указать, как именно запускать новые сессии WebDriver:
- `perBrowser` (по умолчанию) — запуск одной сессии для каждого браузера. Эта сессия будет переиспользована для всех наборов.
- `perSuite` — запуск 1 сессия в каждом браузере для каждого набора. Сессия будет закрыта как только набор протестирован.
Copy link
Contributor

Choose a reason for hiding this comment

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

запуск одной сессии

@coveralls
Copy link

Coverage Status

Coverage increased (+2.14%) to 64.64% when pulling 332a54f on feature/session-per-suite into 8a89a6a on master.

1 similar comment
@coveralls
Copy link

Coverage Status

Coverage increased (+2.14%) to 64.64% when pulling 332a54f on feature/session-per-suite into 8a89a6a on master.

* @param {String} id
*/
BasicPool.prototype.finalizeBrowsers = function(id) {
};
Copy link

Choose a reason for hiding this comment

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

Почему бы не так?

module.exports = _.assign(BasicPool.prototype, {
  /**
   * @param {String} id
   * @returns {Promise.<Browser>}
   */
  getBrowser: function(id) {
  },

  /**
   * @param {Browser} browser
   * @returns {Promise}
   */
  freeBrowser: function(browser) {
  },

  /**
   * @param {String} id
   */
  finalizeBrowsers: function(id) {
  }
});

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Можно и так, но у меня tern с таким вариантом лучше работает.

@SevInf SevInf force-pushed the feature/session-per-suite branch 2 times, most recently from ac02eca to 63c9661 Compare April 16, 2015 09:43
@coveralls
Copy link

Coverage Status

Coverage increased (+2.1%) to 64.6% when pulling 63c9661 on feature/session-per-suite into 8a89a6a on master.

})
.then(function() {
return browser.captureFullscreenImage();
})
Copy link

Choose a reason for hiding this comment

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

Может тут тоже забиндить эти 2 метода?

})
.then(function() {
return _this.browserPool.freeBrowser(browser);
});
Copy link

Choose a reason for hiding this comment

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

Тут вроде тоже можно 3 then'a набиндить

@SevInf
Copy link
Contributor Author

SevInf commented Apr 16, 2015

🆙
@hatroman, посомотри тоже, если время есть

@coveralls
Copy link

Coverage Status

Coverage increased (+2.2%) to 64.7% when pulling 2f9776e on feature/session-per-suite into 8b49677 on master.

.then(function() {
return session.runHook(suite.afterHook);
})
.then(function() {
Copy link
Contributor

Choose a reason for hiding this comment

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

а тут не надо это делать в fin?

@j0tunn
Copy link
Contributor

j0tunn commented Apr 16, 2015

Есть предложение сделать LimitedPool враппером над UnlimitedPool. Тогда будет единая точка собственно поднятия/закрытия браузера - это UnlimitedPool.

Sergey Tatarintsev added 2 commits April 16, 2015 17:08
sessionMode option added which allows to switch between
perBrowser (1 session for browser, current behavior) and
perSuite (1 session for each suite).
To support both old and new behavior along with parallelLimit
setting, browser launcher was replaced with 3 pool classes.

New browser is requested from pool before each suite.

`Pool` class is the most simple variant of the pool which just opens
and closes session every time it requested.

`LimitedPool` wraps another pool and enforces the limit on the number
of currently open sessions. It is used when parallelLimit is set.

In perBrowser mode, limited or unlimited pool will be wrapped
in CachingPool which allows to share a session between multiple
suites.

As long as calibration is needed to be performed only once for
each browser id, it was moved to separate Calibrator class. This
allows to perform only once for each browser even in perSuite mode.
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling f6e7cab on feature/session-per-suite into * on master*.

2 similar comments
@coveralls
Copy link

Coverage Status

Changes Unknown when pulling f6e7cab on feature/session-per-suite into * on master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling f6e7cab on feature/session-per-suite into * on master*.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 34aac8c on feature/session-per-suite into * on master*.

@j0tunn
Copy link
Contributor

j0tunn commented Apr 16, 2015

🆗

SevInf pushed a commit that referenced this pull request Apr 16, 2015
Allow to launch one WD session per suite
@SevInf SevInf merged commit fada945 into master Apr 16, 2015
@SevInf SevInf deleted the feature/session-per-suite branch April 16, 2015 14:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants