From d8f1c462dec83f63f7e1a3382660764eb04e47b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 22 Jan 2021 12:31:48 +0100 Subject: [PATCH] Try to avoid PG::ProtocolViolation error in tests While running our test suite, we were getting an exception sometimes: ``` Proposal Notifications In-app notifications from the proposal's author Followers should receive a notification Failure/Error: notification_for_user2 = Notification.find_by(user: user2) ActiveRecord::StatementInvalid: PG::ProtocolViolation: ERROR: bind message supplies 0 parameters, but prepared statement "" requires 2 : SELECT "notifications".* FROM "notifications" WHERE "notifications"."user_id" = $1 LIMIT $2 # ./spec/system/proposal_notifications_spec.rb:268 ``` Sometimes we were getting a similar exception in a different test: ``` Commenting legislation questions Merged comment threads Reply on a multiple annotation thread and display it in the single annotation thread And sometimes we were getting a different one: Failure/Error: annotation.comments.roots.sort_by_most_voted.limit(Legislation::Annotation::COMMENTS_PAGE_SIZE).each do |comment| ActionView::Template::Error: PG::ProtocolViolation: ERROR: bind message supplies 0 parameters, but prepared statement "" requires 3 ``` My best (wild) guess is these exceptions might take place because the test is accessing the database and at the same time the browser (chromedriver) process is accessing the database, with code like: ``` find(".icon-notification").click notification_for_user2 = Notification.find_by(user: user2) ``` Or: ``` first(:css, ".annotator-hl").click (...) comment = annotation1.comments.first click_link "Reply" ``` This behavior happened sometimes while using transactional fixtures and a shared database connection with feature specs (before system specs were introduced in Rails 5.1) when some queries were triggered from the test after the browser process was started. So we're avoiding the situation by writing the tests from the user's point of view. This is just an attempt at fixing the issue; I don't know whether these changes will fix it since I've only seen this exception on Github Actions (never on my machine). Worst case scenario, we're still improving the tests legibililty. --- .../comments/legislation_annotations_spec.rb | 14 ++++----- spec/system/proposal_notifications_spec.rb | 30 ++++++++++--------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/spec/system/comments/legislation_annotations_spec.rb b/spec/system/comments/legislation_annotations_spec.rb index dbbb41294..9d0de369d 100644 --- a/spec/system/comments/legislation_annotations_spec.rb +++ b/spec/system/comments/legislation_annotations_spec.rb @@ -618,6 +618,8 @@ describe "Commenting legislation questions" do create(:legislation_annotation, draft_version: draft_version, text: "my other annotation", ranges: [{ "start" => "/p[1]", "startOffset" => 1, "end" => "/p[1]", "endOffset" => 10 }]) end + let!(:comment1) { annotation1.comments.first } + let!(:comment2) { annotation2.comments.first } before do login_as user @@ -648,15 +650,14 @@ describe "Commenting legislation questions" do first(:link, "0 replies").click end - comment = annotation1.comments.first click_link "Reply" - within "#js-comment-form-comment_#{comment.id}" do + within "#js-comment-form-comment_#{comment1.id}" do fill_in "Leave your comment", with: "replying in single annotation thread" click_button "Publish reply" end - within "#comment_#{comment.id}" do + within "#comment_#{comment1.id}" do expect(page).to have_content "replying in single annotation thread" end @@ -685,17 +686,16 @@ describe "Commenting legislation questions" do find(".icon-expand").click end - comment = annotation2.comments.first - within("#comment_#{comment.id}") do + within("#comment_#{comment2.id}") do click_link "Reply" end - within "#js-comment-form-comment_#{comment.id}" do + within "#js-comment-form-comment_#{comment2.id}" do fill_in "Leave your comment", with: "replying in multiple annotation thread" click_button "Publish reply" end - within "#comment_#{comment.id}" do + within "#comment_#{comment2.id}" do expect(page).to have_content "replying in multiple annotation thread" end diff --git a/spec/system/proposal_notifications_spec.rb b/spec/system/proposal_notifications_spec.rb index 2471198d1..7a9377962 100644 --- a/spec/system/proposal_notifications_spec.rb +++ b/spec/system/proposal_notifications_spec.rb @@ -200,10 +200,11 @@ describe "Proposal Notifications" do find(".icon-notification").click - notification_for_user1 = Notification.find_by(user: user1) expect(page).to have_css ".notification", count: 1 - expect(page).to have_content "There is one new notification on #{proposal.title}" - expect(page).to have_xpath "//a[@href='#{notification_path(notification_for_user1)}']" + + click_link text: "There is one new notification on #{proposal.title}" + + expect(page).to have_current_path(proposal_path(proposal)) logout login_as user2 @@ -211,10 +212,11 @@ describe "Proposal Notifications" do find(".icon-notification").click - notification_for_user2 = Notification.find_by(user: user2) expect(page).to have_css ".notification", count: 1 - expect(page).to have_content "There is one new notification on #{proposal.title}" - expect(page).to have_xpath "//a[@href='#{notification_path(notification_for_user2)}']" + + click_link text: "There is one new notification on #{proposal.title}" + + expect(page).to have_current_path(proposal_path(proposal)) logout login_as user3 @@ -251,10 +253,11 @@ describe "Proposal Notifications" do find(".icon-notification").click - notification_for_user1 = Notification.find_by(user: user1) expect(page).to have_css ".notification", count: 1 - expect(page).to have_content "There is one new notification on #{proposal.title}" - expect(page).to have_xpath "//a[@href='#{notification_path(notification_for_user1)}']" + + click_link text: "There is one new notification on #{proposal.title}" + + expect(page).to have_current_path(proposal_path(proposal)) logout login_as user2.reload @@ -262,10 +265,11 @@ describe "Proposal Notifications" do find(".icon-notification").click - notification_for_user2 = Notification.find_by(user: user2) expect(page).to have_css ".notification", count: 1 - expect(page).to have_content "There is one new notification on #{proposal.title}" - expect(page).to have_xpath "//a[@href='#{notification_path(notification_for_user2)}']" + + click_link text: "There is one new notification on #{proposal.title}" + + expect(page).to have_current_path(proposal_path(proposal)) logout login_as user3.reload @@ -301,10 +305,8 @@ describe "Proposal Notifications" do find(".icon-notification").click - notification_for_user = Notification.find_by(user: user) expect(page).to have_css ".notification", count: 1 expect(page).to have_content "This resource is not available anymore" - expect(page).not_to have_xpath "//a[@href='#{notification_path(notification_for_user)}']" end scenario "Proposal retired by author", :js do