Hyrax: Feature tests always pass

[elrayle]

Descriptive summary

A check for content in a feature test always passes even when the content is not there.

To test

I edited spec/features/users_spec.rb and added the following tests.

    it 'passes to have_content check when it should fail because the content does not exist' do
      visit profile_path
      expect(page).to have_content('GARBAGE_GARBAGE')     # Passes, but shouldn't
      expect(page).not_to have_content('GARBAGE_GARBAGE') # Also passes
      result = page.has_content?('GARBAGE_GARBAGE')       # correctly is false
      expect(result).to be false                          # so this passes, as expected
      expect(result).to be true                           # and this fails, as expected
    end

    it 'passes to have_content check when the content does exist' do
      visit profile_path
      expect(page).to have_content(user.email)     # Passes, as expected
      expect(page).not_to have_content(user.email) # Fails, as expected
    end

Related work

I ran into this in trying to fix Issue #1591. I was trying to write a failing test first so I could verify the fix. But I couldn't get the test to fail.

I wonder if this is also related to the periodically failing batch_edit test.

[h-parekh]

Hi @elrayle, I'm going to investigate this a bit based on some theories I have. Just starting to setup my workstation to contribute to Hyrax, would you like me to start off on a specific branch?

[h-parekh]

@laritakr and I did some testing and have the following conclusion:

have_content (alias for have_text) is a Capybara::RSpecMatchers::Matcher method, which delegates to assert_text method of Capybara::Node::Matchers
We believe this delegation is broken.

has_content? (alias for has_text?) is a Capybara::Node::Matchers method which delegates to the same assert_text method referred above.

Based on the testing it appears that using Capybara::Node::Matchers for assertion is the most reliable approach.

page.assert_text('garbage_garbage') works reliably and gives an ExpectationNotMet error.

[laritakr]

Based on testing within numerous spec files, have_content and have_text are consistently returning false passing results. In my testing, I did not find a single case where have_content or have_text produced errors as expected.

Acting on this issue involves the following:

• [ ] search for all instances of have_content and have_text
• [ ] replace all instances with assert_text as in the comment above.

[elrayle]

@h-parekh @laritakr Thanks for looking into this! Feature tests are very expensive. If we are going to incur the costs, we certainly want to get the benefit of them actually working.

[elrayle]

The question label was removed because we now know why the tests are inaccurate.

I set the effort to minor. Anyone looking for an easy way to contribute can follow @laritakr suggestions (previous comment) for fixing this.

[jcoyne]

Has anyone opened a bug upstream on this? We should fix this at it's source, not work around the issue

[h-parekh]

Agree, we should fix this at it's source. I'm going to do a more detailed analysis this morning.

[elrayle]

Does @laritakr's suggestions still apply with @jeremyf's PR #1602?