-
Notifications
You must be signed in to change notification settings - Fork 48
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
Auto capture and insert cookie in asset discovery browser #1656
Conversation
Co-authored-by: Pradum Kumar <[email protected]>
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.
LGTM
@chinmay-browserstack @prklm10 I think that this PR is causing me to see the following warning in my logs:
There is no additional logging about what the specific invalid snapshot options are; only that log line is printed. Example here: https://github.com/davidrunger/david_runger/actions/runs/10013824950/job/27682261695#step:10:231 Part of the reason that I believe that this PR is the cause of that warning being logged is because if I open this file on my local machine -- delete data.domSnapshot.cookies; I tried this because when (before making the just-mentioned addition) I logged // ...
keyword: 'unevaluatedProperties',
params: { unevaluatedProperty: 'cookies' },
message: 'must NOT have unevaluated properties',
// ... |
Also, downgrading from |
@davidrunger Thanks for pointing this out. i have checked and found cookies field is missing from config as you mentioned. Created a fix PR for it #1669. it will be fixed in next release. |
document.cookie
in serialize-dom so can be used in asset discovery.Note: httponly cookie can't be captured so that needs to be passed using percy.yml