-
Notifications
You must be signed in to change notification settings - Fork 42
feat: multi step api runner #997
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
base: main
Are you sure you want to change the base?
Conversation
src/helpers.ts
Outdated
args: APIHooksArgs | ||
) { | ||
const promises = callbacks.map(cb => cb(args)); | ||
return await Promise.all(promises); |
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.
return await Promise.all(promises); | |
return await Promise.allSettled(promises); |
Accounting for unrelated rejections
src/dsl/api-journey.ts
Outdated
}; | ||
export type APIJourneyCallback = (options: APIJourneyCallbackOpts) => void; | ||
|
||
export class APIJourney { |
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.
Ideally I would like us to have a base Journey class that both BrowserJourney and APIJourney extends from and the runner should be a single entity that decides how to run each of them. I am not in favor of having multiple runners.
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.
Did a first pass.
@@ -324,4 +341,10 @@ export type JourneyEndResult = JourneyStartResult & | |||
options: RunOptions; | |||
}; | |||
|
|||
export type APIJourneyEndResult = JourneyStartResult & | |||
APIJourneyResult & { | |||
browserDelay: number; |
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.
browserDelay is not appropriate for apijourneys?
@@ -309,22 +316,24 @@ export default class Runner implements RunnerInfo { | |||
|
|||
async #startJourney(journey: Journey, options: RunOptions) { | |||
journey._startTime = monotonicTimeInSeconds(); | |||
this.#driver = await Gatherer.setupDriver(options); | |||
this.#driver = await Gatherer.setupDriver(options, journey.type); |
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 feels a bit weird , Maybe a better way would be to check if there are defined browser journeys and then launch the browser context etc based on that.
|
||
export class APIJourney extends Journey { | ||
#cb: APIJourneyCallback; | ||
#driver?: Driver | APIDriver; |
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.
can it have a Driver at any point?
); | ||
} | ||
|
||
async _interceptRequest(method, url, options: any) { |
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 needs some careful review as we need some extensive tests for cached, aborted, intercepted requests etc, Will do a detailed review later on when we have tests etc.
instance = new BrowserConsole(this.driver); | ||
break; | ||
|
||
if ('context' in this.driver) { |
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.
Plugin Manager should be transparent to the underlying journey that was run whether its browser/api based. There will be always a chance when browser/api based journeys coexisting. I would handle these and make them respective plugins transparent.
multi step api runner
can be tested with