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

ChromeDriver not clearing local storage #2233

Closed
CyborgMaster opened this issue Jul 26, 2019 · 18 comments
Closed

ChromeDriver not clearing local storage #2233

CyborgMaster opened this issue Jul 26, 2019 · 18 comments

Comments

@CyborgMaster
Copy link

Meta

Capybara Version:
3.26.0
Driver Information (and browser if relevant):
3.142.3 with Chrome 75 and ChromeDriver 75

Expected Behavior

Local Storage clears after every test.

Actual Behavior

Local Storage is not cleared at all

Proposed Fix

This would be a PR if I was certain that I had the correct way to fix this.

It appears that the patch to work with the new versions of ChromeDriver broke clearing local storage. The code in question is in the ChromeDriver specialization in the reset! method.

Specifically these lines:

 @browser.navigate.to('about:blank')
 clear_storage unless uniform_storage_clear?

because clear_storage looks like this:

def clear_storage
    # Chrome errors if attempt to clear storage on about:blank
    # In W3C mode it crashes chromedriver
    url = current_url
    super unless url.nil? || url.start_with?('about:')
  end

super is never called because we just navigated to about:blank.

Therefore I propose swapping the order of those two lines:

clear_storage unless uniform_storage_clear?
@browser.navigate.to('about:blank')

However, that wasn't sufficient for my project. I assume that the reason that the uniform_storage_clear? check is there is that the following line is supposed to handle clearing the storage in those cases:

execute_cdp('Storage.clearDataForOrigin', origin: '*', storageTypes: storage_types_to_clear)

However, it wasn't doing so for me. I had to drop the uniform_storage_clear? check to get it to work:

clear_storage
@browser.navigate.to('about:blank')

If I knew for sure why or if this was the correct change this would be a PR. If someone who understands the code can weigh in that would be great. I currently recommend changing lines 46 and 47 from:

 @browser.navigate.to('about:blank')
 clear_storage unless uniform_storage_clear?

to:

clear_storage
@browser.navigate.to('about:blank')
@twalpole
Copy link
Member

@CyborgMaster This is specifically tested in the Capybara test suite, so it definitely works in some cases. Are you using chromedriver in w3c mode or legacy mode? Can you provide any more info about how you're configuring your driver?

@CyborgMaster
Copy link
Author

It's in w3c mode. The current version is

ChromeDriver 75.0.3770.140 (2d9f97485c7b07dc18a74666574f19176731995c-refs/branch-heads/3770@{#1155})

running on macOS 10.14.5

We are setting it up with the following options:

Capybara.register_driver :phonegap do |app|
  browser_options = ::Selenium::WebDriver::Chrome::Options.new
  browser_options.args << '--headless'
  browser_options.args << '--disable-gpu'
  browser_options.args << "--window-size=#{SCREEN_WIDTH},#{SCREEN_HEIGHT}"
  browser_options.args << "--user-agent-=#{USER_AGENT}"
  Capybara::Selenium::Driver.new app,
    browser: :chrome,
    clear_local_storage: true,
    clear_session_storage: true,
    options: browser_options
end

We are seeing this issue on each of our team member's local machines as well as our TravisCI builds.

Are there any more details I can provide?

@twalpole
Copy link
Member

@CyborgMaster Could you please try without specifying the clear_local_storage and clear_session_storage options - since those should default to true.

@twalpole
Copy link
Member

twalpole commented Jul 26, 2019

FYI: The order of the two lines in reset! is definitely wrong and I've fixed that in 3c1f1de - my real concern is why the CDP call isn't clearing things for you - since it definitely should clear and should be more reliable and thorough than the other method

@twalpole
Copy link
Member

@CyborgMaster Also, you appear to have an extra - at the end of --user-agent -- I doubt that would have anything to do with this issue - but might be causing you other issues

@CyborgMaster
Copy link
Author

@twalpole. Thanks for jumping on this so fast. I was curious about the CDP call myself, which is why I didn't make this a PR, it seems like it should have been working.

Nice catch on the user agent. Agreed that it shouldn't be causing any problems, but I've fixed it in our codebase.

I'm going to hand some of this extra experimentation and investigation to one of my engineers, @phaedryx, as I have to jump onto some other tasks, but want to make sure this gets the time it deserves.

@twalpole
Copy link
Member

twalpole commented Jul 26, 2019

Ok - the line order swap is in master branch now and will ship in 3.27 later today/tomorrow (does not remove the unless uniform_storage_clear? though because that really shouldn't be necessary)

@twalpole
Copy link
Member

twalpole commented Jul 26, 2019

Another question that would be good to get answered is what (sub)domain you're setting the storage on.

@CyborgMaster
Copy link
Author

the subdomain is just localhost

@twalpole
Copy link
Member

@CyborgMaster Are you sure about that? The capybara default would be 127.0.0.1 which browsers treat as a separate storage than 'localhost'

@CyborgMaster
Copy link
Author

Good point, I just checked. We have two subdomains and we use for different testing.

Main: http://localhost:9876
For testing a mobile app: http://localhost:4001

We setup a different Capybara driver for each

Sometimes we run both during the same test two simulate interaction between the two, swapping in the middle of the test using Capybara.current_driver =.

@twalpole
Copy link
Member

Are you having Capybara start two servers (or specifying reuse_server = false))? or are you manually managing the server on the second port? Would it happen to be only the tests using both drivers where storage isn't getting reset (or just a specific one) ? Does the second driver registration not clear both local and session storage?

@CyborgMaster
Copy link
Author

We are managing the other server on the second port ourself, starting and stopping it as needed. Capybara isn't involved.

It does seem like it might be related to tests that use both drivers. We have capybara configured to let us choose which driver a test uses, and it seems that if a test is configured to use the main driver, but then temporarily switches to the mobile driver mid-test to simulate mobile / server interaction, then the mobile driver isn't getting it's local storage cleared. Then when the next mobile driver only test starts, it still has the local storage from the last test.

@twalpole
Copy link
Member

Ok -- at least that narrows it down a bit. Are you sure you're not manually creating the session used for the mobile session? Is the driver registration shown previously the one used for the mobile driver? Are you sure it's local storage and not cookies that are left in the mobile driver session?

@CyborgMaster
Copy link
Author

No, every time we start a test for the mobile session, we login. We count on the session being cleared each time. In fact that's the error we're seeing, we go to the mobile home page, and instead of getting a sign in prompt, we get the last users credentials.

The driver registration is from the mobile driver.

The mobile device doesn't use cookies, it uses a JWT stored in local storage, so I'm fairly confident it isn't cookies. Either way, shouldn't the CDP call clear cookies as well?

I really appreciate you taking the time to try and track this down and I appreciate how complex these set ups can be with how the individual projects are set up. I can't share the raw source because it's a company project, but if you wanted to, I could do a shared debug session. I'm not asking for it, but if you were interested, we could make that work.

@twalpole
Copy link
Member

twalpole commented Jul 29, 2019

Yes - Cookies should also be cleared, I just want to make sure I'm trying to track down the right thing.

https://gist.github.com/twalpole/bb472b4032e3a0bd1d80786d60c77d7a is standalone code that uses using_driver to switch between two sessions, and shows local storage being reset. If you can identify what that is doing differently than your tests then we could narrow it down, but as of now I can't replicate the behavior you're seeing. If you were actually creating a session yourself (instead of letting Capybara manage them - this refers to Capybara::Session objects, not session in your app) then I could understand the behavior you're seeing because Capybara wouldn't be resetting the session.

@CyborgMaster
Copy link
Author

Thanks for the test example. I'll see if we can modify it to make the same thing happen for us.

@twalpole
Copy link
Member

twalpole commented Aug 3, 2019

Closing this because the reproducible part has been fixed. I have been unable to find a way to replicate the other issue mentioned and am leaning towards it being a user issue rather than Capybara. If a way to replicate is provided we can reopen this.

@twalpole twalpole closed this as completed Aug 3, 2019
@lock lock bot locked and limited conversation to collaborators Sep 2, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants