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

Feature to persist sessions to file #38

Closed
wants to merge 1 commit into from

Conversation

jgraef
Copy link

@jgraef jgraef commented Feb 18, 2021

This PR adds a feature (behind the feature-flag persist) that allows a WebDriver session to be persisted to disk.

WebDriver::persist will consume self and return a PersitedSession that contains the session ID and capabilities.

PersistedSession::connect allows to turn a persisted session back into a WebDriver.

PersistedSession is also Serialize and Deserialize, so it can easily be written to a file.

Copy link
Owner

@stevepryde stevepryde left a comment

Choose a reason for hiding this comment

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

The idea is interesting but I'm curious what the use-case is. Selenium browser sessions are typically short, at least in a testing context. It may be helpful to provide some documentation, particularly at the module level, so that future readers and contributors will know what the primary intention for this feature is. Thanks :)

src/webdriver.rs Show resolved Hide resolved
src/webdriver.rs Show resolved Hide resolved
}
}

impl WebDriver {
Copy link
Owner

Choose a reason for hiding this comment

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

Does this work? My thinking is that it should be impl GenericWebDriver instead.

Also, would you be happy to add a test or doctest for the persist() -> connect() round trip?

Copy link
Author

Choose a reason for hiding this comment

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

Hmm, it probably should be GenericWebDriver, but this definitely compiles and even runs!

I can add a test, when I have some time. It might take a while, but I guess we can keep the PR open until then.

Copy link
Owner

Choose a reason for hiding this comment

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

I don't think a test would be difficult at all. Have a look at the doctests for WebDriverCommands etc. All this would require is to create a session, persist it to a file, destroy the original, then reload the session from the file and issue some command to it. I think that would be sufficient as a minimal way to sanity check this feature.

Happy to keep the PR open :)

Copy link
Owner

@stevepryde stevepryde Feb 21, 2021

Choose a reason for hiding this comment

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

Note that you can use docker-compose up -d to run the demo web app that all doctests use. It will be accessible at http://localhost:8000 when it runs. By looking at the other tests you will see how this is being used. It's a fairly simple yet robust way to test everything against an actual selenium instance and browser.

You can then use cargo test --features=persist to run all tests. You can also add part of the method name at the end to only run your particular test.

Once there are doctests for this I'd also like this to be added to the automated build (Github Actions) so that this feature is regularly tested with every build, but I can add that later :)

@jgraef
Copy link
Author

jgraef commented Feb 19, 2021

My use-case: I use this to control a Chromium browser to browse through Instagram, but I imagine any website with a lot of dynamic content and good bot-detection is probably better scraped using a real browser.

During debugging I just didn't want to have it always open a new browser and logging in. With a persisted session I can run my client to first open a browser and login. And then with subsequent commands I can load different pages and extract information.

@stevepryde
Copy link
Owner

Thanks for outlining the use-case. There have been requests to support connecting to an existing session in the past so I see this as another variant of that idea.

@stevepryde
Copy link
Owner

Due to upcoming changes to thirtyfour (refactor to use fantoccini as a base), this change is no longer applicable.

@stevepryde stevepryde closed this Jan 24, 2022
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.

None yet

2 participants