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

[SDTEST-158] Browser tests support via selenium integration #183

Merged
merged 11 commits into from
May 29, 2024

Conversation

anmarchenko
Copy link
Member

@anmarchenko anmarchenko commented May 24, 2024

What does this PR do?
Automatically recognises tests that use selenium and marks them as browser tests. Integrates with Datadog RUM to automatically show browser sessions in the test run view (see screenshots below).

Additional Notes

  • this PR introduces the concept of automatically instrumenting external libraries without requiring users to change configuration: Selenium should be required and configured by the time when test session starts, so I added Contrib. auto_instrument_on_session_start! method that automatically instruments it when test session starts
  • it turned out not to be enough to instrument selenium-webdriver gem because capybara has its own logic to reset browser session between test runs. In this PR both Selenium::WebDriver::Driver and Capybara::Selenium::Driver (not that capybara is de-facto the default testing library for e2e tests in Ruby)

How to test the change?
Unit tests are provided.

Tested on DD staging using the following test:

require_relative "helper"

class SeleniumTest < Minitest::Test
  include Capybara::DSL
  include Capybara::Minitest::Assertions

  def teardown
    Capybara.reset_sessions!
    Capybara.use_default_driver
  end

  def test_selenium
    visit "https://dd.datad0g.com/"
    assert page.has_content?("Datadog")
  end
end

With the following results

Browser tags and is_rum_active tag:
image

Browser session replay connected to a test:
image

@codecov-commenter
Copy link

codecov-commenter commented May 24, 2024

Codecov Report

Attention: Patch coverage is 95.00000% with 13 lines in your changes are missing coverage. Please review.

Project coverage is 98.87%. Comparing base (1c997fd) to head (691bf86).
Report is 2 commits behind head on main.

Files Patch % Lines
lib/datadog/ci/contrib/selenium/capybara_driver.rb 65.21% 8 Missing ⚠️
lib/datadog/ci/contrib/selenium/navigation.rb 93.93% 2 Missing ⚠️
lib/datadog/ci/contrib/selenium/driver.rb 95.65% 1 Missing ⚠️
lib/datadog/ci/contrib/selenium/patcher.rb 94.44% 1 Missing ⚠️
lib/datadog/ci/contrib/selenium/rum.rb 95.23% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #183      +/-   ##
==========================================
- Coverage   98.95%   98.87%   -0.09%     
==========================================
  Files         209      220      +11     
  Lines        9758    10011     +253     
  Branches      452      465      +13     
==========================================
+ Hits         9656     9898     +242     
- Misses        102      113      +11     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@anmarchenko anmarchenko changed the title [CIVIS-9359] Add selenium support [SDTEST-158] Add selenium support May 28, 2024
@anmarchenko anmarchenko changed the title [SDTEST-158] Add selenium support [SDTEST-158] Browser tests support via selenium integration May 28, 2024
@anmarchenko anmarchenko marked this pull request as ready for review May 28, 2024 13:20
@anmarchenko anmarchenko requested review from a team as code owners May 28, 2024 13:20
lib/datadog/ci/contrib/selenium/ext.rb Outdated Show resolved Hide resolved
def self.is_rum_active?(script_executor)
is_rum_active_script_result = script_executor.execute_script(Ext::SCRIPT_IS_RUM_ACTIVE)
Datadog.logger.debug { "[Selenium] SCRIPT_IS_RUM_ACTIVE result is #{is_rum_active_script_result.inspect}" }
is_rum_active_script_result == "true" || is_rum_active_script_result == true

Choose a reason for hiding this comment

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

Should the script_executor be ensuring its result's a bool, rather than checking for both here?

Copy link
Member Author

Choose a reason for hiding this comment

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

script_executor is coming from selenium-webdriver, it could be Selenium::WebDriver::Manager or Selenium::WebDriver::Driver

One could say that we need to be exactly sure what type this returns but in Ruby case one can never be sure. Another more beatiful way to write it would be is_rum_active_script_result.to_s == "true", I think this would look better

Choose a reason for hiding this comment

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

hehe, I would be more likely to try to cast the string to a bool :) but anyway, it's not very important, I think the code's fine as is.

@anmarchenko anmarchenko merged commit c9fccc9 into main May 29, 2024
25 checks passed
@anmarchenko anmarchenko deleted the anmarchenko/selenium_support branch May 29, 2024 10:35
@github-actions github-actions bot added this to the 1.0.0 milestone May 29, 2024
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.

3 participants