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

feat(config): introduce shouldRerty config option#878

Merged
miripiruni merged 1 commit intomasterfrom
FEI-8660__retries
Feb 14, 2018
Merged

feat(config): introduce shouldRerty config option#878
miripiruni merged 1 commit intomasterfrom
FEI-8660__retries

Conversation

@miripiruni
Copy link
Copy Markdown
Member

@miripiruni miripiruni commented Feb 7, 2018

Config should provide to users option to set custom rule whether test retry
needed or not.

Related to:

@miripiruni miripiruni requested a review from j0tunn February 7, 2018 16:10
* `shouldRetry` — function which defines whether retry needed or not. Should returns `Boolean` value. First argument is `data` which is result of test run. `data` will contains fields:
* `attempt {Number}` number of retries performed for the test
* `retriesLeft {Number}` how many attempts left for retries
* `[equal] {Boolean}` notice that if the test fails with critical error equal can be absent
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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


_submitForRetry(data) {
Object.assign(data, {
attempt: this._retriesPerformed,
Copy link
Copy Markdown
Member Author

@miripiruni miripiruni Feb 7, 2018

Choose a reason for hiding this comment

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

Честно, я не понял где это используется… Возможно, это поле можно убрать?

@miripiruni miripiruni force-pushed the FEI-8660__retries branch 5 times, most recently from c3f72ec to cd98ec6 Compare February 8, 2018 22:55
doc/config.md Outdated

Note that the same test never be performed in the same browser session.

* `shouldRetry` — function which defines whether retry needed or not. Should returns `Boolean` value. First argument is `data` which is result of test run. `data` will contains fields:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

...whether to make a retry. Should return...
which is the result of the test run, it's an Object with following fields:

doc/config.md Outdated

* `shouldRetry` — function which defines whether retry needed or not. Should returns `Boolean` value. First argument is `data` which is result of test run. `data` will contains fields:
* `attempt {Number}` number of retries performed for the test
* `retriesLeft {Number}` how many attempts left for retries
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

number of retries left

* `shouldRetry` — function which defines whether retry needed or not. Should returns `Boolean` value. First argument is `data` which is result of test run. `data` will contains fields:
* `attempt {Number}` number of retries performed for the test
* `retriesLeft {Number}` how many attempts left for retries
* `[equal] {Boolean}` notice that if the test fails with critical error equal can be absent
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Непонятное описание.
См. мои комментарии в PR в retry-limiter.

if (retriesLeft <= 0) {
_shouldRetry(data) {
if (typeof this._config.shouldRetry === 'function') {
return Boolean(this._config.shouldRetry(data));
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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


_submitForRetry(data) {
Object.assign(data, {
attempt: this._retriesPerformed,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

retriesMade/retriesLeft будет симметричнее.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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

Config should provide to users option to set custom rule whether test retry
needed or not. See: FEI-8660
@miripiruni miripiruni merged commit 1d7f6f9 into master Feb 14, 2018
@miripiruni miripiruni deleted the FEI-8660__retries branch February 14, 2018 15:19
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.

3 participants