🐛 Gather current_session from Capybara rather than importing the DSL module
#168
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
wwilsman
reviewed
Jun 9, 2021
Rather than including the Capybara DSL into the module and then clobbering back onto the Capybara class. This is _much_ much better.
Robdel12
force-pushed
the
rd/better-page-session-gathering
branch
from
efce1d0 to
bc77a5e
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is this?
Issue: #167 Using matches on the
pageclass causes a infinite loop. 😱The way we're getting the current page is by importing the Capybara DSL into the Percy module. Then we apply the module back to the Capybara class. In the process, we're surly clobbering things.
Approach
We're only including the DSL module to gain access to the current
pagesession. We can copy exactly how the DSL defines thepagemethod: https://github.com/teamcapybara/capybara/blob/master/lib/capybara/dsl.rb#L45-L47 which will avoid clobbering classes, as we're doing now.Question
current_pagemethod name? #letsbikeshed There's currently no methods in capybara with that name, but it feels like it's prone to be broken in the future. Do we do something hyper specific with naming for Percy? (percy_pageor something? Something that won't clobber future method names?)