Skip to content

add hybrid flow to analytics spec#9525

Merged
amirbey merged 4 commits intomainfrom
amirbey/LG-11427-add-hybrid-flow-to-analytics-events-spec
Nov 2, 2023
Merged

add hybrid flow to analytics spec#9525
amirbey merged 4 commits intomainfrom
amirbey/LG-11427-add-hybrid-flow-to-analytics-events-spec

Conversation

@amirbey
Copy link
Contributor

@amirbey amirbey commented Nov 2, 2023

🎫 Ticket

LG-11427

🛠 Summary of changes

Add hybrid flow test to analytics spec

@amirbey amirbey self-assigned this Nov 2, 2023
@amirbey amirbey marked this pull request as ready for review November 2, 2023 17:41
@amirbey amirbey force-pushed the amirbey/LG-11427-add-hybrid-flow-to-analytics-events-spec branch from c1bb614 to 55e1abc Compare November 2, 2023 17:49

it 'records all of the events' do
aggregate_failures 'analytics events' do
happy_path_events.each do |event, attributes|
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
happy_path_events.each do |event, attributes|
happy_hybrid_path_events.each do |event, attributes|

Copy link
Contributor

@charleyf charleyf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good reuse of the existing happy path spec code. I didn't examine it beyond checking that the flow of the test made sense to me.

One suggestion below that will make the tests pass, looks like a missed find/change.

end
end

it 'records all of the events' do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also reuse this for hybrid or non hybrid happy path as shared example, but not a blocker.

LGTM.

@aduth
Copy link
Contributor

aduth commented Nov 2, 2023

I was hoping this might include the "Link sent capture doc polling started" event, which #9481 touches. Is that possible? Not sure if there's going to be some timing issues between when the JavaScript runs and how quickly the test completes.

@amirbey
Copy link
Contributor Author

amirbey commented Nov 2, 2023

I was hoping this might include the "Link sent capture doc polling started" event, which #9481 touches. Is that possible? Not sure if there's going to be some timing issues between when the JavaScript runs and how quickly the test completes.

yes. i did try adding the polling events but there may be a Javascript timing issue like you mentioned 🤔
that can be explored in a follow up PR. For now, this is an improvement and moving this forward to support #9481

@amirbey amirbey merged commit 92b45cc into main Nov 2, 2023
@amirbey amirbey deleted the amirbey/LG-11427-add-hybrid-flow-to-analytics-events-spec branch November 2, 2023 22:26
@matthinz matthinz mentioned this pull request Nov 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants