buildkite-config

buildkite-config copied to clipboard

Re-add system tests for action text (ended up reverting https://github.com/rails/buildkite-config/pull/26)

Rails PR that's needed to run the tests https://github.com/rails/rails/pull/47127

To repro, as mentioned in this comment https://github.com/rails/buildkite-config/pull/26#issuecomment-1384864919

git clone https://github.com/rails/rails
cd rails
git clone https://github.com/rails/buildkite-config .buildkite/
RUBY_IMAGE=ruby:3.2 docker-compose -f .buildkite/docker-compose.yml build base
CI=1 docker-compose -f .buildkite/docker-compose.yml run default runner actiontext 'rake test:system'

Jan 24 '23 22:01 hahmed

Actually... I still need to look into something. The change in this PR adds chrome:

# Chrome
     && touch /etc/default/google-chrome \
     && curl -sS https://dl.google.com/linux/linux_signing_key.pub | APT_KEY_DONT_WARN_ON_DANGEROUS_USAGE=1 apt-key add - \
     && echo "deb http://dl.google.com/linux/chrome/deb/ stable main" > /etc/apt/sources.list.d/chrome.list \

But the docker-compose file is already adding chrome:

chrome:
    image: selenium/standalone-chrome:latest

https://hub.docker.com/layers/selenium/standalone-chrome/latest/images/sha256-a1172cc6c8d2218d37fa40bef04d9e33a88f9407733ad9072244b503c6a615e7?context=explore

Dockerfile in Github: https://github.com/SeleniumHQ/docker-selenium/blob/trunk/NodeChrome/Dockerfile

Jan 25 '23 22:01 hahmed

ActionView and ActionCable both use karma to run the tests, I removed this line selenium/standalone-chrome:latest and the tests still completed successfully. If there is another place that runs tests on chrome, i'm not seeing it.

Here is how I ran the integration tests for action view + action cable;

• CI=1 docker-compose -f .buildkite/docker-compose.yml run default runner actionview 'rake test:ujs'
• CI=1 docker-compose -f .buildkite/docker-compose.yml run default runner actionview 'rake' the tests related to capybara are mostly checking drivers or config related code, but nothing actually has a page/ui and a button is selected, that's the part that needs chrome.
• CI=1 docker-compose -f .buildkite/docker-compose.yml run default runner actioncable 'rake test:integration'

I'm hesitant in removing selenium/standalone-chrome:latest from this PR, but can remove from the next PR, but your call? Don't want to break the build again.

Feb 01 '23 21:02 hahmed

cc/ @rafaelfranca @yahonda this PR is ready and if this is ok, then https://github.com/rails/rails/pull/47127 can also be merged.

Feb 01 '23 21:02 hahmed

Can these tests not use the separate standalone-chrome instance?

Feb 01 '23 21:02 matthewd

I could not really figure out how to get action text to use selenium/standalone-chrome:latest I tried setting the binary here https://github.com/rails/rails/pull/47127/files#diff-92bd81dd752a3bffacbc7de4ee005362d298dfc94905ae278b804f1c86097991R7

driver_option.add_option("binary", "/opt/selenium/chromedriver-google-chrome-stable") but I could not get it to work.

Error:

Error:
ActionText::SystemTestHelperTest#test_filling_in_a_rich-text_area_by_aria-label:
Selenium::WebDriver::Error::UnknownError: unknown error: no chrome binary at /opt/selenium/chromedriver-google-chrome-stable
    #0 0x56120465b303 <unknown>
    #1 0x56120442fd37 <unknown>
    #2 0x561204455a33 <unknown>
    #3 0x561204454330 <unknown>
    #4 0x5612044954a6 <unknown>
    #5 0x56120448c753 <unknown>
    #6 0x56120445fa14 <unknown>
    #7 0x561204460b7e <unknown>
    #8 0x5612046aa32e <unknown>
    #9 0x5612046adc0e <unknown>
    #10 0x561204690610 <unknown>
    #11 0x5612046aec23 <unknown>
    #12 0x561204682545 <unknown>
    #13 0x5612046cf6a8 <unknown>
    #14 0x5612046cf836 <unknown>
    #15 0x5612046ead13 <unknown>
    #16 0x7f68ba0c2ea7 start_thread
    /usr/local/bundle/gems/selenium-webdriver-4.8.0/lib/selenium/webdriver/remote/response.rb:55:in `assert_ok'
    /usr/local/bundle/gems/selenium-webdriver-4.8.0/lib/selenium/webdriver/remote/response.rb:34:in `initialize'
    /usr/local/bundle/gems/selenium-webdriver-4.8.0/lib/selenium/webdriver/remote/http/common.rb:83:in `new'
    /usr/local/bundle/gems/selenium-webdriver-4.8.0/lib/selenium/webdriver/remote/http/common.rb:83:in `create_response'
    /usr/local/bundle/gems/selenium-webdriver-4.8.0/lib/selenium/webdriver/remote/http/default.rb:104:in `request'
    /usr/local/bundle/gems/selenium-webdriver-4.8.0/lib/selenium/webdriver/remote/http/common.rb:59:in `call'
    /usr/local/bundle/gems/selenium-webdriver-4.8.0/lib/selenium/webdriver/remote/bridge.rb:618:in `execute'
    /usr/local/bundle/gems/selenium-webdriver-4.8.0/lib/selenium/webdriver/remote/bridge.rb:52:in `create_session'
    /usr/local/bundle/gems/selenium-webdriver-4.8.0/lib/selenium/webdriver/common/driver.rb:313:in `block in create_bridge'
    <internal:kernel>:90:in `tap'
    /usr/local/bundle/gems/selenium-webdriver-4.8.0/lib/selenium/webdriver/common/driver.rb:312:in `create_bridge'
    /usr/local/bundle/gems/selenium-webdriver-4.8.0/lib/selenium/webdriver/common/driver.rb:74:in `initialize'
    /usr/local/bundle/gems/selenium-webdriver-4.8.0/lib/selenium/webdriver/chrome/driver.rb:36:in `initialize'
    /usr/local/bundle/gems/selenium-webdriver-4.8.0/lib/selenium/webdriver/common/driver.rb:47:in `new'
    /usr/local/bundle/gems/selenium-webdriver-4.8.0/lib/selenium/webdriver/common/driver.rb:47:in `for'
    /usr/local/bundle/gems/selenium-webdriver-4.8.0/lib/selenium/webdriver.rb:88:in `for'
    /usr/local/bundle/gems/capybara-3.38.0/lib/capybara/selenium/driver.rb:83:in `browser'
    /rails/actionpack/lib/action_dispatch/system_testing/driver.rb:64:in `block in register_selenium'
    <internal:kernel>:90:in `tap'
    /rails/actionpack/lib/action_dispatch/system_testing/driver.rb:63:in `register_selenium'
    /rails/actionpack/lib/action_dispatch/system_testing/driver.rb:49:in `block in register'
    /usr/local/bundle/gems/capybara-3.38.0/lib/capybara/session.rb:105:in `driver'
    /usr/local/bundle/gems/capybara-3.38.0/lib/capybara/session.rb:91:in `initialize'
    /usr/local/bundle/gems/capybara-3.38.0/lib/capybara.rb:421:in `new'
    /usr/local/bundle/gems/capybara-3.38.0/lib/capybara.rb:421:in `block in session_pool'
    /usr/local/bundle/gems/capybara-3.38.0/lib/capybara.rb:317:in `current_session'
    /rails/actionpack/lib/action_dispatch/system_test_case.rb:175:in `app_host'
    /rails/actionpack/lib/action_dispatch/system_test_case.rb:171:in `url_options'
    /rails/actionpack/lib/action_dispatch/routing/route_set.rb:198:in `call'
    /rails/actionpack/lib/action_dispatch/routing/route_set.rb:327:in `block in define_url_helper'
    /rails/actionpack/lib/action_dispatch/system_test_case.rb:183:in `public_send'
    /rails/actionpack/lib/action_dispatch/system_test_case.rb:183:in `method_missing'
    /rails/actiontext/test/system/system_test_helper_test.rb:7:in `setup'

(I also tried /usr/bin/chromedriver it looks like from the screenshot below that's the dir that's being force linked but 🤷‍♂️)

Maybe there is another way to set the path, but I'm not sure... Here is the docker image https://hub.docker.com/layers/selenium/standalone-chrome/latest/images/sha256-c070bad288d2175b5af1a05f4db4426602eb4d1e76f6692be0808b2353776682?context=explore Line 66 onwards.

Feb 01 '23 22:02 hahmed

The binary won't be there on the machine running the test -- the point of the standalone image is that chrome runs in a separate docker image, and we communicate with it over the network (via SELENIUM_DRIVER_URL).

Feb 01 '23 22:02 matthewd

Ah, thanks for that context, I previously attempted the url option, PR https://github.com/rails/rails/pull/47030 but the tests were also failing

Feb 01 '23 22:02 hahmed

I can't for the life of me figure out where we actually use SELENIUM_DRIVER_URL... 🤔

Sep 14 '23 10:09 zzak

👀 https://github.com/rails/rails/commit/d9e79ce7f4eb86a8f74c0f583c3fc0105c399a71

Sep 14 '23 10:09 zzak

@zzak atm it's not actually used, I think we need to pass in SELENIUM_URL as a remote option to fix these action text system tests

Sep 19 '23 20:09 hahmed

I'm going to try again with that URL approach, because I feel like I'm missing something, I think the Capybara.current_session.server_url is returning a different port or URL.

(Keep in mind this is me attempting to do this in a github action), I re-added that url and remote option here https://github.com/hahmed/rails/pull/1/files#diff-92bd81dd752a3bffacbc7de4ee005362d298dfc94905ae278b804f1c86097991 but my build is still failing https://github.com/hahmed/rails/actions/runs/6240789656/job/16941645072

If you manage to get it working, even better 😀

Sep 19 '23 20:09 hahmed

This is the same error I was getting when trying this in a repo at work, that was what brought me here 😭

Sep 21 '23 10:09 zzak

@zzak just to clarify are you setting up cabybara tests for a rails app with that selenium docker image?

(If so here is an example of it working https://github.com/hahmed/capybaraciiii/blob/85ce3c15d7882209eecbdf9ad4cb103bc9fc5570/.github/workflows/ci.yml)

Sep 22 '23 22:09 hahmed

It looks like that chrome instance can be used after all 🤦🏽

From rails side, we don't have to do anything else but merge this PR. The only thing is I have it tested and working in a github ci action -- code: https://github.com/hahmed/rails/pull/1/commits/28d838527742c6004abf5893a93fb7fecc3f949a and CI action that shows it's all green https://github.com/hahmed/rails/actions/runs/6286139237/job/17069264032

Will run the repro steps locally to test this out to see if everything works for buildkite (should be straightforward), however last time we got this error https://github.com/rails/buildkite-config/pull/26#issuecomment-1384834031, need to figure out why.

I don't have docker on this machine, will retest again 😅 -- going to close that Rails PR

Sep 23 '23 22:09 hahmed

You should be able to test this in the CI job in this PR as well. It will create a full Buildkite build against main and 6.1

Sep 24 '23 02:09 skipkayhil

Even better @skipkayhil, although I think my builds are blocked/pending, might need approval or something but i'm not sure 🤔

Sep 24 '23 21:09 hahmed

If you are signed into Buildkite, you should see a button to review the build script and approve the job. I'm assuming you have the correct permissions though.

Sep 25 '23 07:09 zzak

Thanks @zzak @skipkayhil I OK'd and it's running, I'm assuming this change is all good to hit that ok button on. Got a failure now, so that's progress

Sep 25 '23 13:09 hahmed

@hahmed That was my bad, sorry. I had left a broken config on the meta-ci, rebuilding now. :pray:

Oct 05 '23 21:10 zzak

Also, I think the thing I realized is we're no longer using the chrome container (as of https://github.com/rails/rails/commit/d9e79ce7f4eb86a8f74c0f583c3fc0105c399a71) where running karma on saucelabs instead. At least for Action View.

I'm not sure if we want to go the same route for Action Text? But it feels like consistency might be a good argument.

Oct 05 '23 21:10 zzak

Here's the build failure: https://buildkite.com/rails/rails-ci/builds/153#018b01bb-95a4-4f6c-ad89-307057bf5fc8/1091-1099

/rails/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb:460:in `table_structure': Could not find table 'action_mailbox_inbound_emails' (ActiveRecord::StatementInvalid)
--
  | from /rails/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb:109:in `columns'
  | from /rails/activerecord/lib/active_record/connection_adapters/schema_cache.rb:335:in `block in columns'
  | from /rails/activerecord/lib/active_record/connection_adapters/schema_cache.rb:334:in `fetch'
  | from /rails/activerecord/lib/active_record/connection_adapters/schema_cache.rb:334:in `columns'
  | from /rails/activerecord/lib/active_record/connection_adapters/schema_cache.rb:343:in `block in columns_hash'
  | from /rails/activerecord/lib/active_record/connection_adapters/schema_cache.rb:342:in `fetch'
  | from /rails/activerecord/lib/active_record/connection_adapters/schema_cache.rb:342:in `columns_hash'
  | from /rails/activerecord/lib/active_record/connection_adapters/schema_cache.rb:58:in `columns_hash'
  | from /rails/activerecord/lib/active_record/connection_adapters/schema_cache.rb:184:in `columns_hash'
  | from /rails/activerecord/lib/active_record/model_schema.rb:616:in `load_schema!'
  | from /rails/activerecord/lib/active_record/attributes.rb:264:in `load_schema!'
  | from /rails/activerecord/lib/active_record/encryption/encryptable_record.rb:127:in `load_schema!'
  | from /rails/activerecord/lib/active_record/model_schema.rb:561:in `block in load_schema'
  | from /rails/activerecord/lib/active_record/model_schema.rb:558:in `synchronize'
  | from /rails/activerecord/lib/active_record/model_schema.rb:558:in `load_schema'
  | from /rails/railties/lib/rails/testing/maintain_test_schema.rb:13:in `block in <top (required)>'
  | from /rails/railties/lib/rails/testing/maintain_test_schema.rb:12:in `each'
  | from /rails/railties/lib/rails/testing/maintain_test_schema.rb:12:in `<top (required)>'
  | from <internal:/usr/local/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:38:in `require'
  | from <internal:/usr/local/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:38:in `require'
  | from /usr/local/bundle/gems/zeitwerk-2.6.12/lib/zeitwerk/kernel.rb:38:in `require'
  | from /rails/railties/lib/rails/test_help.rb:17:in `<top (required)>'
  | from <internal:/usr/local/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:38:in `require'
  | from <internal:/usr/local/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:38:in `require'
  | from /usr/local/bundle/gems/zeitwerk-2.6.12/lib/zeitwerk/kernel.rb:38:in `require'
  | from /rails/actiontext/test/test_helper.rb:10:in `<top (required)>'
  | from <internal:/usr/local/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:38:in `require'
  | from <internal:/usr/local/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:38:in `require'
  | from /rails/actiontext/test/application_system_test_case.rb:3:in `<top (required)>'
  | from <internal:/usr/local/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:38:in `require'
  | from <internal:/usr/local/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:38:in `require'
  | from /rails/actiontext/test/system/system_test_helper_test.rb:3:in `<top (required)>'
  | from <internal:/usr/local/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:38:in `require'
  | from <internal:/usr/local/lib/ruby/site_ruby/3.2.0/rubygems/core_ext/kernel_require.rb>:38:in `require'
  | from /usr/local/lib/ruby/gems/3.2.0/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:21:in `block in <main>'
  | from /usr/local/lib/ruby/gems/3.2.0/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:6:in `select'
  | from /usr/local/lib/ruby/gems/3.2.0/gems/rake-13.0.6/lib/rake/rake_test_loader.rb:6:in `<main>'
  | rake aborted!
  | Command failed with status (1): [ruby -w -I"lib:test" /usr/local/lib/ruby/gems/3.2.0/gems/rake-13.0.6/lib/rake/rake_test_loader.rb "test/system/system_test_helper_test.rb" ]

Oct 06 '23 05:10 zzak

Thanks @zzak

i think Karma tests in rails for actioview/action cable are for javascript tests? Whereas these actiontext tests are system tests, so it needs rails, but i think you are right it will work also w/ saucelabs (not checked that though).

There was a ton invested in buildkite, would love if we can just move everything over to github actions (I have this specific action working in a github action and did not need to change much). But that's prob a question for the team, maybe Matthew knows if it's ok to migrate to github 🤔

Oct 06 '23 16:10 hahmed

I think RF just fixed that failure about the action_mailbox_inbound_emails in rails/rails@a731dbdfb8940b657946b8febcf7f56d7227beb0

I'm going to issue a rebuild.

We should figure out how to get these tests passing on Buildkite.

Oct 06 '23 23:10 zzak

So it seems like chrome is just crashing, there is probably a flag we can add to fix that:

Selenium::WebDriver::Error::SessionNotCreatedError: session not created: Chrome failed to start: exited normally.

  | (session not created: DevToolsActivePort file doesn't exist)   | (The process started from chrome location /root/.cache/selenium/chrome/linux64/117.0.5938.149/chrome is no longer running, so ChromeDriver is assuming that Chrome has crashed.)

https://buildkite.com/rails/rails-ci/builds/155#018b0772-b4b1-4618-9ac4-3ad572693636

Oct 07 '23 00:10 zzak

Tests are passing for me now, with this config locally:

diff --git a/actiontext/test/application_system_test_case.rb b/actiontext/test/application_system_test_case.rb
index a24f473..dd1d2cb 100644
--- a/actiontext/test/application_system_test_case.rb
+++ b/actiontext/test/application_system_test_case.rb
@@ -3,7 +3,13 @@
 require "test_helper"

 class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
-  driven_by :selenium, using: :headless_chrome
+  options = {
+    browser: ENV["SELENIUM_DRIVER_URL"].blank? ? :chrome : :remote,
+    url: ENV["SELENIUM_DRIVER_URL"].blank? ? nil : ENV["SELENIUM_DRIVER_URL"]
+  }
+  driven_by :selenium, using: :headless_chrome, options: options
 end

 Capybara.server = :puma, { Silent: true }
+Capybara.server_host = "0.0.0.0" # bind to all interfaces
+Capybara.app_host = "http://#{IPSocket.getaddress(Socket.gethostname)}" if ENV["SELENIUM_DRIVER_URL"].present?

Oct 07 '23 01:10 zzak