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

Fix Applitools::Selenium::Element equality #46

Merged
merged 2 commits into from Oct 18, 2016

Conversation

@islemaster
Copy link
Contributor

islemaster commented Oct 3, 2016

The wrapper class Applitools::Selenium::Element's equality function does not match the behavior of Selenium::Webdriver::Element, which it is supposed to transparently wrap. In particular, it violates this test:

From selenium/webdriver/element_spec.rb

      it 'should know when two elements are equal' do
        driver.navigate.to url_for('simpleTest.html')

        body = driver.find_element(tag_name: 'body')
        xbody = driver.find_element(xpath: '//body')

        expect(body).to eq(xbody)
        expect(body).to eql(xbody)
      end

I've added this test to the driver passthrough spec and fixed it by adding support for comparing two Applitools::Selenium::Elements to one another, which was not previously possible.

@islemaster

This comment has been minimized.

Copy link
Contributor Author

islemaster commented Oct 3, 2016

More context:

Discovered the problem when one of our Cucumber steps (which we share between Applitools tests and plain Selenium tests) generated this output when we tried to use it in an Applitools test:

expected: #<Applitools::Selenium::Element(#<Selenium::WebDriver::Element:0xef7ccc6987cff14 id="0.5569987882778864-3">)>
     got: #<Applitools::Selenium::Element(#<Selenium::WebDriver::Element:0xef7ccc6987cff14 id="0.5569987882778864-3">)>

(compared using ==)
 (RSpec::Expectations::ExpectationNotMetError)

It'd be a lot more work, but it might be possible and useful to pull the whole set of selenium integration tests in and verify that they pass using the wrapped driver.

@islemaster islemaster mentioned this pull request Oct 3, 2016
@danielputerman

This comment has been minimized.

Copy link
Member

danielputerman commented Oct 12, 2016

Hi, I'll review the change later today. Thanks :)

@danielputerman danielputerman merged commit afc5082 into applitools:master Oct 18, 2016
1 check passed
1 check passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@danielputerman

This comment has been minimized.

Copy link
Member

danielputerman commented Oct 18, 2016

Looks great, and was merged. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.