Failure in browser_ui/3_create/merge_request/add_batch_comments_in_merge_request_spec.rb | Create batch comments in merge request user submits a non-diff review

added Enterprise Edition QA Quality failureinvestigating found:main priority2 test labels

mentioned in issue gitlab-org/quality/pipeline-triage#98 (closed)

Looks like we've got an orphaned element. The review is submitted and the button does disappear, but the test framework still finds it as a stale element.

The feature still works so it doesn't seem to be a critical bug.

It started failing after !67967 (merged) was merged.

I can reproduce the failure locally on GDK. If I revert the commit from that MR (1bdfa789) the test passes.

@iamphill I think one of the changes in !67967 (merged) might result in the Submit review element being left behind after being removed from the DOM after the button is clicked and it disappears.

Do you know what might be the cause?

@mlapierre Could it be because we async load in the review bar that the test checks before the JS has loaded?

Strangely when I get to the point where we are checking for the non-existence of the selector, if I just try to grab all selectors from page, I get the error. I'm not sure why we get into this state

    138: def submit_pending_reviews
    139:   within_element(:review_bar_content) do
    140:     click_element(:review_preview_dropdown)
    141:     click_element(:submit_review_button)
    142:     require 'pry'
    143:     binding.pry
    144:     # After clicking the button, wait for it to disappear
    145:     # before moving on to the next part of the test
 => 146:     has_no_element?(:submit_review_button)
    147:   end
    148: end

[1] pry(#<QA::Page::MergeRequest::Show>)> selectors = page.all('[data-qa-selector]')
/Users/willmeek/.asdf/installs/ruby/2.7.2/lib/ruby/gems/2.7.0/gems/pry-0.11.3/lib/pry/exceptions.rb:29: warning: $SAFE will become a normal global variable in Ruby 3.0
Selenium::WebDriver::Error::StaleElementReferenceError: stale element reference: element is not attached to the page document
  (Session info: chrome=93.0.4577.63)
from 0   chromedriver                        0x00000001079296d9 chromedriver + 2758361

I think my confusion lies a bit in how it worked previously

The review_bar_content is gone after the click

If I move the checks for the submit_review_dropdown outside of this within_element the test is successful

    138: def submit_pending_reviews
    139:   has_element?(:submit_review_button)
    140:   within_element(:review_bar_content) do
    141:     click_element(:review_preview_dropdown)
    142:     click_element(:submit_review_button)
    143:   end
    144:   require 'pry'
    145:   binding.pry
    146:     # After clicking the button, wait for it to disappear
    147:     # before moving on to the next part of the test
 => 148:   has_no_element?(:submit_review_button)
    149: end

2 examples, 0 failures

@iamphill That could be it, but it should be fine it with the updated test. It doesn't look like the change would affect users.

@willmeek Nice work. I guess that's why Capybara didn't automatically resolve the stale element exception as it usually does? Maybe it can't retry finding the within element for review_bar_content? Or maybe our framework is getting in the way somehow?