-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: ability to accept image on no reference image error #136
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
в остальном ок
@@ -157,6 +157,8 @@ module.exports = class ReportBuilder { | |||
|
|||
if (!hasFails(stateInBrowser)) { | |||
stateInBrowser.result.status = SUCCESS; | |||
} else if (hasNoRefImageErrors(stateInBrowser.result)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
какое-то разное апи у хелперов получается: для hasFails
мы передаем весь стэйт, а для hasNoRefImageErrors
- сразу result
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
у нас hasFails
более сложный хелпер, он еще ходит по результатам чайлдов. А мне здесь нужен только result
. Не вижу в этом проблемы.
@@ -109,6 +107,10 @@ class Body extends Component { | |||
); | |||
} | |||
|
|||
_shouldAddErrorTab({multipleTabs, status, screenshot}) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
название метода утверждающее, без намека на то, что оно что-то проверяет
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
не понял. Мой метод по русски переводится примерно так - "следует ли добавить вкладку с ошибкой". Где здесь утверждающее?
@@ -13,6 +13,10 @@ function hasFailedImages(result) { | |||
|| isErroredStatus(status) || isFailStatus(status); | |||
} | |||
|
|||
function hasNoRefImageErrors({imagesInfo = []}) { | |||
return Boolean(imagesInfo.filter((v) => get(v, 'reason.stack', '').startsWith(NO_REF_IMAGE_ERROR)).length); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
зачем ты здесь так жестко матчишься? у объекта ошибки есть нужный тип и название
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
как минимум, можно написать v.name === NO_REF_IMAGE_ERROR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
потому, что у меня есть только stack
, message
и actualPath
-
reason = _.pick(assertResult, ['message', 'stack']); |
name
который будет в итоге сохраняться в data.js
вообще не хочется.
Плюс у нас уже есть такой же самый матчинг -
html-reporter/lib/static/modules/utils.js
Line 30 in eeddd07
return isErroredStatus(status) && stack.startsWith(NO_REF_IMAGE_ERROR) || isFailStatus(status); |
563b80d
to
f4c5890
Compare
No description provided.