Skip to content
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

lighthouse for current browser session #420

Closed
wants to merge 2 commits into from

Conversation

imkalvin
Copy link

@imkalvin imkalvin commented Jul 6, 2021

Hi @pmeenan,

In my org, we need to test against pages behind authentication (which was identified with a cookie with max-age=session). With these changes, WPT is now able to do that (tested on Windows+Ubuntu 18.04).

Pls help to take a look whether this is something WPT wants to take in.

This might also address the issue #395

@pmeenan
Copy link
Contributor

pmeenan commented Jul 6, 2021

It might be better if the capability was an option so it could be enabled explicitly when desired. Otherwise there is no way to get a "clean" lighthouse test with no cookies or cache populated.

@imkalvin
Copy link
Author

imkalvin commented Jul 7, 2021

It might be better if the capability was an option so it could be enabled explicitly when desired. Otherwise there is no way to get a "clean" lighthouse test with no cookies or cache populated.

Then I assume we will need to enhance the server part to add a new option, then pass along to agents...right?

@pmeenan
Copy link
Contributor

pmeenan commented Jul 7, 2021

Either an explicit option or only use this mode when a script is provided. I'm happy to plumb the test setting if you're worried about the php side of things (it's just index.php and runtest.php and can be copied from some of the other options)

@imkalvin
Copy link
Author

imkalvin commented Jul 9, 2021

Either an explicit option or only use this mode when a script is provided. I'm happy to plumb the test setting if you're worried about the php side of things (it's just index.php and runtest.php and can be copied from some of the other options)

Thanks @pmeenan !

I think using this mode when a script is provided will make better sense. I will update the PR later.

@imkalvin
Copy link
Author

Hi @pmeenan ,

I had wrapped the new logic with a flag check upon task['script']. Pls help to review and proceed. Thanks!

@M-Z
Copy link

M-Z commented Jul 26, 2021

Hi,
I gave this a try and it worked as it should for me.
Would be great if this is confirmed to patch my agents with it.

@eugeneidoronin
Copy link

If it works that would be great to have it merged. Thank you.

@pmeenan
Copy link
Contributor

pmeenan commented Jul 27, 2021

I need to double-check but I think the non-script case may also trigger it because a simple URL test is also converted into a 1-step navigate script. I think the reuse check need to check that the script length > 1 instead of > 0

@M-Z
Copy link

M-Z commented Aug 27, 2021

Hi @pmeenan ,
any updates on this PR?

@pmeenan pmeenan closed this Jan 25, 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