core(fr): separate audit phase for flows#13623
Conversation
|
|
||
| await flow.endFlow(); | ||
|
|
||
| const flowResult = flow.getFlowResult(); |
There was a problem hiding this comment.
is there a reason to have separate functions for endFlow and getFlowResult?
There was a problem hiding this comment.
I didn't want to make getFlowResult async but that concern is kinda thin. I'm up for either.
| const name = this.name || `User flow (${url.hostname})`; | ||
| return {steps: this.steps, name}; | ||
| async getFlowResult() { | ||
| if (this.flowResult) return this.flowResult; |
There was a problem hiding this comment.
Seems like if the user calls getFlowResult, followed by more gathering, then getFlowResult again, they will get a cached result and no indication that something they did was wrong.
Should either guard against that (throw if already called getFlowResult), or just not cache. If the latter, might be good to rename to createFlowResult.
also nit: rename getUserFlowReport?
There was a problem hiding this comment.
Should either guard against that (throw if already called getFlowResult), or just not cache. If the latter, might be good to rename to createFlowResult.
I'm thinking we remove the cache.
also nit: rename getUserFlowReport?
This function returns a FlowResult object not a string containing the user flow report.
There was a problem hiding this comment.
I meant getUserFlowResult.
There was a problem hiding this comment.
I think it makes more sense since the object type is FlowResult. I also think it's pretty clear that "flow" refers to a "user flow" and not some other type of flow. The constructor for UserFlow isn't exposed on the api either.
| /** | ||
| * @param {LH.FlowResult} flowResult | ||
| */ | ||
| async function generateFlowReport(flowResult) { |
There was a problem hiding this comment.
I added this api function for 2 reasons:
- Allows us to generate the flow result just once when we generate flow fixtures with
--viewflag - We will eventually need a way to generate a flow report for a result object generated from artifacts and not the
UserFlowclass
Step 3 of #13364
Ref
#11313