thoughtbot / action_dispatch-testing-integration-capybara

MIT License
14 stars 0 forks source link

in Minitest, Capybara's `has_field?` works, but `assert_field` gives error for hidden field #4

Open nimmolo opened 2 years ago

nimmolo commented 2 years ago

The Capybara Minitest Assertions don't seem to be available in a simple integration test, although the Capybara matchers are available. I've raised the issue on SO, and Capybara's author has tested it and cannot reproduce the bug locally.

So - In my page, I have a form input like this: (Note: It doesn't matter if the input is hidden or not, the results are the same.)

<input value="project" autocomplete="off" type="hidden" 
       name="description[source_type]" id="description_source_type" />

The Capybara matcher has_field? finds the input, but Capybara's Minitest assertion built on that matcher, assert_field, gets an error.

(ruby) has_field?("description_source_type", type: :hidden, with: "project")
true
(ruby) assert_field("description_source_type", type: :hidden, with: "project")
eval error: Unused parameters passed to Capybara::Queries::SelectorQuery : [:field, "description_source_type"]
            /home/v/.rvm/gems/ruby-3.0.4/gems/capybara-3.37.1/lib/capybara/queries/selector_query.rb:52:in `initialize'
            /home/v/.rvm/gems/ruby-3.0.4/gems/capybara-3.37.1/lib/capybara/node/matchers.rb:842:in `new'
            /home/v/.rvm/gems/ruby-3.0.4/gems/capybara-3.37.1/lib/capybara/node/matchers.rb:842:in `_verify_selector_result'
            /home/v/.rvm/gems/ruby-3.0.4/gems/capybara-3.37.1/lib/capybara/node/matchers.rb:110:in `assert_selector'
            /home/v/.rvm/gems/ruby-3.0.4/gems/capybara-3.37.1/lib/capybara/session.rb:771:in `assert_selector'
            /home/v/.rvm/gems/ruby-3.0.4/gems/capybara-3.37.1/lib/capybara/dsl.rb:52:in `call'
            /home/v/.rvm/gems/ruby-3.0.4/gems/capybara-3.37.1/lib/capybara/dsl.rb:52:in `assert_selector'
            /home/v/.rvm/gems/ruby-3.0.4/gems/capybara-3.37.1/lib/capybara/minitest.rb:288:in `block (2 levels) in <module:Assertions>'
            /v/mo/test/integration/capybara_student_test.rb:52:in `block in test_creating_drafts'
            /home/v/.rvm/gems/ruby-3.0.4/gems/capybara-3.37.1/lib/capybara.rb:372:in `using_session'
            /home/v/.rvm/gems/ruby-3.0.4/gems/capybara-3.37.1/lib/capybara/dsl.rb:22:in `using_session'
            /v/mo/test/integration/capybara_student_test.rb:37:in `test_creating_drafts'

The rubydocs for Capybara::Minitest::Assertions#assert_field say that the method is built straight off Node::Matchers#has_field?. I wonder why it isn't working with the same syntax? I've tried a lot of variations on the syntax, too.

Does this gem do any modification of the Capybara methods? I've read through the code and it doesn't seem that it does. Is my test maybe set up incorrectly?

in my test_helper.rb:

require("capybara/rails")
require("capybara/minitest")

module ActionDispatch
  class IntegrationTest
    # Make the Capybara DSL available in all integration tests
    include Capybara::DSL
    # not including the assertions here, so I don't have to rewrite all my other Integration Tests
    # include Capybara::Minitest::Assertions

Note above that I didn't include Capybara::Minitest::Assertions in IntegrationTest directly, because I have a lot of other integration tests that would have to be immediately rewritten with Capybara assumptions. I'd like to tackle that one test at a time. So, to use the Capybara Minitest assertions in this test only, I made a sub class to derive Capybara Integration Tests from, in capybara_integration_test_case.rb

class CapybaraIntegrationTestCase < ActionDispatch::IntegrationTest
  # Make `assert_*` methods behave like Minitest assertions
  include Capybara::Minitest::Assertions

Does it matter that i'm including Assertions here, in the class I'm deriving the test from, rather than the parent ActionDispatch::IntegrationTest? It doesn't seem to make any difference, in both cases, the assertion fails.

Here's the derivation of the test in question, capybara_student_test.rb

require("test_helper")

# Test typical sessions of university student who is writing descriptions.
class CapybaraStudentTest < CapybaraIntegrationTestCase

finally, my Gemfile:

  # Use Capybara from within ActionDispatch::IntegrationTest
  gem("action_dispatch-testing-integration-capybara",
      github: "thoughtbot/action_dispatch-testing-integration-capybara",
      tag: "v0.1.0",
      require: "action_dispatch/testing/integration/capybara/minitest")

This app is on Rails 6.1.6, Capybara 3.37.1

nimmolo commented 2 years ago

@seanpdoyle sorry to ping you, and thanks for your work to get an API to enable this to be available in Rails.

I've doublechecked my test setup, and I believe everything's right, but Capybara Minitest assertions are still not available in my integration tests. Do you see anything obviously wrong in my setup?

seanpdoyle commented 2 years ago

@nimmolo thank you for opening this issue.

Including the require: "action_dispatch/testing/integration/capybara/minitest" option in your Gemfile should mean that your test configuration shouldn't require lines like include Capybara::DSL or include Capybara::Minitest::Assertions. Including them and the side-effects of action_dispatch/testing/integration/capybara/minitest might be at the root of your issues.

Are you able to reproduce your issues if you remove the include calls to both Capybara::DSL and Capybara::Minitest::Assertions?

nimmolo commented 2 years ago

Hi Sean - Thanks for considering it, and for the suggestion.

If I keep the Gemfile require option, and remove both include Capybara::DSL and include Capybara::Minitest::Assertions from the test ancestor classes, i get an error the first time I use Capybara syntax in the test.

NoMethodError: undefined method `using_session'

I tried other combinations — just removing include Capybara::DSL, just removing Assertions, and omitting the require option for this gem, and they all produce errors relating to missing methods or classes.

New question: My Gemfile adds Capybara on a separate line, above this gem. Maybe this is superfluous as long as the Thoughtbot gem is bundled, since Capybara is a dependency. Should I remove the line gem "capybara", "~> 3.37", ">= 3.37.1"?

I'm trying this in a legacy app, and test_helper.rb has a lot of requires. I wonder if any of the require sequence is wrong here. Does order matter? (Sorry for the super-long code block here).

# frozen_string_literal: true

#
#  = Base Test Case
#
#  Does some basic site-wide configuration for all the tests.  This is part of
#  the base class that all unit tests must derive from.
#
#  *NOTE*: This must go directly in Test::Unit::TestCase because of how Rails
#  tests work.
#
################################################################################

# Code to allow Coveralls exor local coverage reports.  See:
# https://github.com/coverallsapp/github-action/issues/29#issuecomment-701934460
require("rails")
require("simplecov")
require("simplecov-lcov")

if ENV["CI"] == "true"
  SimpleCov::Formatter::LcovFormatter.config do |config|
    config.report_with_single_file = true
    config.lcov_file_name = "lcov.info"
  end

  SimpleCov.formatter = SimpleCov::Formatter::LcovFormatter
else
  SimpleCov.formatter = SimpleCov::Formatter::HTMLFormatter
end

SimpleCov.start("rails")

# Allow test results to be reported back to runner IDEs.
# Enable progress bar output during the test running.
require("minitest/reporters")
MiniTest::Reporters.use!

require("minitest/autorun")

# Allow stubbing and setting expectations on HTTP, and selective
#  disabling of internet requests.
require("webmock/minitest")

# Disable external requests while allowing localhost
WebMock.disable_net_connect!(allow_localhost: true)

ENV["RAILS_ENV"] ||= "test"
require(File.expand_path("../config/environment", __dir__))
require("rails/test_help")

# Enable mocking and stubbing in Ruby (must be required after rails/test_help).
require("mocha/minitest")

# Allow simuluation of user-browser interaction with capybara
require("capybara/rails")
require("capybara/minitest")

I18n.enforce_available_locales = true

module ActiveSupport
  class TestCase
    # Run tests in parallel with specified workers
    # parallelize(workers: :number_of_processors)

    ##########################################################################
    #  Transactional fixtures
    ##########################################################################
    # Transactional fixtures accelerate your tests by wrapping each
    # test method in a transaction that's rolled back on completion.
    # This ensures that the test database remains unchanged so your
    # fixtures don't have to be reloaded between every test method.
    # Fewer database queries means faster tests.
    #
    # Read Mike Clark's excellent walkthrough at
    #   http://clarkware.com/cgi/blosxom/2005/10/24#Rails10FastTesting
    #
    # Every Active Record database supports transactions except MyISAM
    # tables in MySQL.  Turn off transactional fixtures in this case;
    # however, if you don't care one way or the other, switching from
    # MyISAM to InnoDB tables is recommended.
    #
    # The only drawback to using transactional fixtures is when you
    # actually need to test transactions.  Since your test is
    # bracketed by a transaction, any transactions started in your
    # code will be automatically rolled back.
    self.use_transactional_tests = true

    # Instantiated fixtures are slow, but give you @david where
    # otherwise you would need people(:david).  If you don't want to
    # migrate your existing test cases which use the @david style and
    # don't mind the speed hit (each instantiated fixtures translates
    # to a database query per test method), then set this back to
    # true.
    self.use_instantiated_fixtures = false
    ##########################################################################

    # Setup all fixtures in test/fixtures/*.(yml|csv) for all tests in
    # alphabetical order.
    #
    # Note: You'll currently still have to declare fixtures explicitly
    # in integration tests -- they do not yet inherit this setting
    fixtures :all

    # Add more helper methods to be used by all tests here...

    # Standard setup to run before every test.  Sets the locale, timezone,
    # and makes sure User doesn't think a user is logged in.
    def setup
      # Disable cop; there's no block in which to limit the time zone change
      I18n.locale = :en if I18n.locale != :en # rubocop:disable Rails/I18nLocaleAssignment
      # Disable cop; there's no block in which to limit the time zone change
      Time.zone = "America/New_York" # rubocop:disable Rails/TimeZoneAssignment
      User.current = nil
      start_timer if false
      clear_logs unless defined?(@@cleared_logs)
      Symbol.missing_tags = []
    end

    # Standard teardown to run after every test.  Just makes sure any
    # images that might have been uploaded are cleared out.
    def teardown
      assert_equal([], Symbol.missing_tags, "Language tag(s) are missing.")
      FileUtils.rm_rf(MO.local_image_files)
      UserGroup.clear_cache_for_unit_tests
      stop_timer if false
    end

    # Record time this test started to run.
    def start_timer
      @@times = {} unless defined?(@@times)
      @@times[method_name] = Time.zone.now
    end

    # Report time this test took to run.
    def end_timer
      ellapsed = Time.zone.now - @@times[method_name]
      puts("\rTIME: #{ellapsed}\t#{self.class.name}::#{method_name}")
    end

    # This will ensure that the logs stay a reasonable size.  If you forget to
    # clear these logs periodically, they can get freaking huge, and that
    # causes this test to take up to several minutes to complete.
    def clear_logs
      %w[development test email-debug process_image].each do |file|
        file = Rails.root.join("log", "#{file}.log")
        next unless File.exist?(file)

        File.truncate(file, 0)
      end
      @@cleared_logs = true
    end
  end
end

module ActionDispatch
  class IntegrationTest
    # Make the Capybara DSL available in all integration tests
    include Capybara::DSL

    # Make `assert_*` methods behave like Minitest assertions
    #
    # include Capybara::Minitest::Assertions
    #
    # Note: Including Capybara assertions in all IntegrationTests causes some
    # assertions in existing integration tests (built on rails-dom-testing) to
    # fail. For experimenting with Capybara+Integration i'm making a new class
    # tests can inherit from: CapybaraIntegrationTestCase. (Nimmo - 09/2022)

    # Reset sessions and driver between tests
    teardown do
      Capybara.reset_sessions!
      Capybara.use_default_driver
    end
  end
end

%w[
  bullet_helper

  general_extensions
  flash_extensions
  controller_extensions
  integration_extensions
  capybara_integration_extensions
  language_extensions
  session_extensions
  session_form_extensions

  check_for_unsafe_html
  uploaded_string

  unit_test_case
  functional_test_case
  integration_test_case
  capybara_integration_test_case
].each do |file|
  require_relative(file)
end

Note: the derived CapybaraIntegrationTestCase class has the include Capybara::Minitest::Assertions, and that class is required after the block for ActionDispatch::IntegrationTest, which has include Capybara::DSL. These are the includes i tried deleting at your suggestion, but they are restored now.

On the other hand, maybe my approach here needs to be reconsidered.

This Rails app has a bunch of integration tests already written via extending rails-dom-testing, and if I include Capybara::Minitest::Assertions in my IntegrationTestCase, many assertions fail due to the assert_select name clash. I'm trying to keep the old integration tests separate from my experimental new Capybara integration tests by including the assertions only in the latter, but maybe i need another approach.

Theoretically though, if my Gemfile has require: "action_dispatch/testing/integration/capybara/minitest" I should be experiencing that method name clash no matter what, and this is not the case. Without including the Capybara::Minitest::Assertions explicitly, I don't get the name clash error.

nimmolo commented 2 years ago

On the other hand, maybe my approach here needs to be reconsidered.

My Rails app has a bunch of integration tests already written via extending rails-dom-testing, and if I include Capybara::Minitest::Assertions in my IntegrationTestCase, many assertions fail due to being written with rails-dom-testing syntax.

I'm trying to keep the old integration tests separate from the experimental new Capybara integration tests I'm writing, by including the assertions only in the latter. This gem seems to include Capybara methods in all integration tests.

Because I'm not ready to rewrite all my integration tests at once, i would prefer a way to only include the methods in certain test classes. I think the solution for me is to add the code from this gem into my Rails app directly, and modify it so that the Capybara classes are only added to the test case sub-class for Capybara integration tests.

If I understood your article on Thoughtbot correctly, it outlines how to do this in detail. Let me know if that sounds right to you, if you get a chance. In any case I'll be trying it out.

nimmolo commented 2 years ago

@seanpdoyle I found the problem. Stepping backward, I realized that the problem was present already had before adding the gem, so I deactivated it and resumed trying to get my tests to work using Capybara's normal recommended require/include.

The core of it was class ancestry, having require capybara/rails,require capybara/minitest and include Capybara::DSL in my test_helper.rb, but include Capybara::Minitest::Assertions in my separate CapybaraIntegrationTestCase.rb from which i derived my new Capybara integration tests, seems to have put the call stack out of order. Moving all Capybara requires and includes to the CapybaraIntegrationTestCase cleared this up.

I believe this gem is intended to be compatible with existing integration tests, but I believe that this Rails app's extensions to rails-dom-testing methods are what is causing the gem to make existing integration tests fail. Even removing all require and include of Capybara classes everywhere (except the gem), I couldn't get the existing integration tests to pass with this gem bundled. For now, instead of having to rewrite all these existing integration tests to be compatible with the gem, i'm just going with separate test case classes for rails-dom-testing and Capybara, and requiring/including Capybara classes only in the latter, while keeping the app's Integration method extensions restricted to the former.

If you're curious about the conflicts, i'm happy to point you to the app's repo, where you can inspect these test failures by activating the gem. I say this only in case you think it might make the gem more robust.