-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Add page.on('request')
#4290
base: master
Are you sure you want to change the base?
Add page.on('request')
#4290
Conversation
bd7b987
to
05b8969
Compare
This is where the request will be set for the page.on handler to read.
This method is to be called when a new request is about to be sent from chrome to the WuT. It takes the request and send it to the page.on handler where the user test script can read the request data from.
This is a temporary fix. When working with page.on('request') the timing values haven't yet been received, which are part of the response object. This will need to be fixed later when we can wait for a response while working with page.on('request').
05b8969
to
6ac9bd0
Compare
6ac9bd0
to
09fbe5d
Compare
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.
Nice bit of work 👏 Some suggestions.
7d131b6
to
2709a6c
Compare
2709a6c
to
522a411
Compare
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.
One more non-critical thing I noticed.
The issue was that HeadersArray was out of order. If they're put in order then the comparison can be made.
p.eventHandlersMu.RUnlock() | ||
defer p.eventHandlersMu.RLock() |
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.
Why do we need to lock again? Isn't the lock before already safe-guarding for it?
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.
Yeah, this looks a bit odd. The reason behind this is to allow the handler to be able to add more handlers. This is a behaviour that Playwright exhibits and also documents, so it's something we're replicating.
page.on('request', async () => {
page.on('response', async () => {
// Do something with the request and response data...
})
})
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.
@codebien I asked the same question before for another related page.on
method. Here's a little bit more detail @ankur22 previously answered: grafana/xk6-browser#1456 (comment)
What?
This implements the
page.on('request')
API from Playwright. When the handler is created, all subsequent requests will be directed to the handler, where the user can interact with the read only request. The request is intercepted just before it is sent from the browser to the website under test.An example usage is:
Why?
This can be useful to:
Checklist
make lint
) and all checks pass.make tests
) and all tests pass.Related PR(s)/Issue(s)
Updates: #4281