-
Notifications
You must be signed in to change notification settings - Fork 63
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 disable tests isolation #821
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,7 +91,7 @@ module.exports = { | |
key: null, | ||
region: null, | ||
headless: null, | ||
isolation: false, | ||
isolation: null, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Если оставить |
||
}; | ||
|
||
module.exports.configPaths = [".hermione.conf.ts", ".hermione.conf.js"]; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -1200,6 +1200,46 @@ describe("config browser-options", () => { | |
|
||
assert.isFalse(config.browsers.b1.isolation); | ||
}); | ||
|
||
describe("should set to 'false' by user even if browser support isolation", () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Добавил тестов, которые не работали со старым кодом. |
||
it("in top level config", () => { | ||
const readConfig = { | ||
isolation: false, | ||
browsers: { | ||
b1: mkBrowser_({ | ||
desiredCapabilities: { | ||
browserName: "chrome", | ||
browserVersion: "101.0", | ||
}, | ||
}), | ||
}, | ||
}; | ||
Config.read.returns(readConfig); | ||
|
||
const config = createConfig(); | ||
|
||
assert.isFalse(config.browsers.b1.isolation); | ||
}); | ||
|
||
it("in browser config", () => { | ||
const readConfig = { | ||
browsers: { | ||
b1: mkBrowser_({ | ||
isolation: false, | ||
desiredCapabilities: { | ||
browserName: "chrome", | ||
browserVersion: "101.0", | ||
}, | ||
}), | ||
}, | ||
}; | ||
Config.read.returns(readConfig); | ||
|
||
const config = createConfig(); | ||
|
||
assert.isFalse(config.browsers.b1.isolation); | ||
}); | ||
}); | ||
}); | ||
|
||
describe("saveHistoryMode", () => { | ||
|
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.
Тут ошибся и заюзал старое название поля
isSpecified
(раньше так называлось). Но позже мы его переименовали вisSetByUser
. Поправил.Кроме этого дефолт переделал в
null
иначе не получается корректно обработать значение.Тут сначала
map
выполняется для top level конфига, а потом для каждого браузера. И проблема в том, что еслиisolation
указать в top level, то при его обработке флагisSetByUser
будет выставлен корректно. Но затемmap
будет вызван для каждого браузера и для негоisSetByUser: false
и при этом дефолтное значение прилетает из топ левела (если оно было указано). Поэтому тут приходится писать чуть сложнее решение.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.
Печально, что приходится к
boolean
значениям примешиватьnull
, но дешевых вариантов "как сделать тут лучше" я не вижу.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.
Я пока более простого решения не нашел. По дефолту в конфиге браузера у юзера все равно будет валидный boolean. Так как capabilities при разборе конфига браузера - обязательное значение. Т.е. всегда будем проваливаться в
isSupportIsolation
, который возвращаетboolean
.