-
-
Notifications
You must be signed in to change notification settings - Fork 43
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
feat: add option to ignore urls for auto waiting #575
Conversation
Could you hint me where in the Docs this should be documented ? |
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.
main thing is my naming suggestion (don't hate me, please)
src/types/wdi5.types.ts
Outdated
* Regex for XHR/Fetch requests to be ignored by the auto waiter | ||
* Ideal for long polling as this would result in the waiter waiting forever | ||
*/ | ||
autoWaitUrlIgnoreRegex?: string[] |
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.
don't hate me, please, but I suggest to keep config options as short as possible - would it be possible to drop the "Regex" and just make it ignoreAutoWaitUrls
?
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.
haha no hate here :D my naming skills are way to verbal. I am fine with that.
@@ -135,6 +136,7 @@ export async function injectUI5(config: wdi5Config, browserInstance) { | |||
const version = await (browserInstance as WebdriverIO.Browser).getUI5Version() | |||
await checkUI5Version(version) | |||
await clientSide_injectTools(browserInstance) // helpers for wdi5 browser scope | |||
await clientSide_injectXHRPatch(config, browserInstance) |
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.
would it be possible to do a await Promise.all([clientSide_injectTools(browserInstance), clientSide_injectXHRPatch(config, browserInstance)])
in order to have those run in parallel and to squeeze out some performance?
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.
hi sadly not the clientSide_injectTools function add the compare version function to the browser and that is needed in clientSide_injectXHRPatch and checkUI5Version is actualy not async.
client-side-js/injectXHRPatch.cjs
Outdated
return await browserInstance.executeAsync((config, done) => { | ||
const originalFetch = window.fetch | ||
|
||
const autoWaitUrlIgnoreRegex = config.wdi5.autoWaitUrlIgnoreRegex |
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.
see my other comment with the renaming suggestion
docs/configuration.md
Outdated
@@ -26,7 +26,8 @@ exports.config = { | |||
logLevel: "verbose", // [optional] error | verbose | silent, default: "error" | |||
skipInjectUI5OnStart: false, // [optional] {boolean}, default: false; true when UI5 is not on the start page, you need to later call <wdioUI5service>.injectUI5() manually | |||
waitForUI5Timeout: 15000, // [optional] {number}, default: 15000; maximum waiting time in milliseconds while checking for UI5 availability | |||
btpWorkZoneEnablement: false // [optional] {boolean}, default: false; whether to instruct wdi5 to inject itself in both the SAP Build Workzone, standard edition, shell and app | |||
btpWorkZoneEnablement: false, // [optional] {boolean}, default: false; whether to instruct wdi5 to inject itself in both the SAP Build Workzone, standard edition, shell and app | |||
autoWaitUrlIgnoreRegex: [] // [optional] {string[]}, default: []; Array of regex to ignore certain XHR/Fetch calls wile autowaiting |
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.
see my other comment about renaming this setting, please
docs/configuration.md
Outdated
@@ -133,6 +134,12 @@ Number in milliseconds (default: `15000`) to wait for UI5-related operations wit | |||
Boolean setting to trigger injecting `wdi5` into both the shell and the app when used with the SAP Build Workzone, standard edition. | |||
Recommended complement is to also [configure IAS Authentication](authentication?id=sap-cloud-identity-services-identity-authentication): as SAP Build requires its own Identity Provider (most likely provided by using an IAS tenant), you'll have to configure authentication against that as well in `wdi5`. | |||
|
|||
### `autoWaitUrlIgnoreRegex` |
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.
see my rename comment
@@ -0,0 +1,13 @@ | |||
const femiddleware = require("@sap-ux/ui5-middleware-fe-mockserver") | |||
|
|||
module.exports = async function middleware(middlewareConfig) { |
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.
this is so cool. well well done!!!
@@ -10,3 +10,19 @@ server: | |||
configuration: | |||
baseUri: "https://services.odata.org/V2" | |||
strictSSL: false | |||
- name: delayed-mockserver |
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.
again: cool idea, well done 😀
}) | ||
} | ||
startXHR() | ||
const startFetch = () => { |
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.
i keep my fingers crossed that the gh runners are equipped with enough memory for the CI runs 😁
Co-authored-by: Volker Buzek <[email protected]>
Co-authored-by: Volker Buzek <[email protected]>
Add the option to skip the XHRWaiter an the fetchWaiter based on an regex in the wdi5 config.