From f721e88af80afad4c0962c6a3787809641a71ea0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Fri, 25 Oct 2024 19:56:44 +0200 Subject: [PATCH 1/3] Remove instance_double usage in CommentsHelper tests Lately (not sure since when), from time to time we've been getting these failures in our CI: ``` Failures: 1) CommentsHelper#comment_author_class returns is-author if author is the commenting user Failure/Error: comment = instance_double(Comment, user_id: author_id) the Comment class does not implement the instance method: user_id # ./spec/helpers/comments_helper_spec.rb:48:in `block (3 levels) in ' # ./spec/spec_helper.rb:40:in `block (3 levels) in ' # ./spec/spec_helper.rb:39:in `block (2 levels) in ' 2) CommentsHelper#comment_author_class returns an empty string if commenter is not the author Failure/Error: comment = instance_double(Comment, user_id: author_id - 1) the Comment class does not implement the instance method: user_id # ./spec/helpers/comments_helper_spec.rb:55:in `block (3 levels) in ' # ./spec/spec_helper.rb:40:in `block (3 levels) in ' # ./spec/spec_helper.rb:39:in `block (2 levels) in ' ``` It might be related to the upgrade of rspec-rails done in commit 6fe222148 or maybe due to a change in github actions that caused some tests to fail, as described in commits bedcb5bca and 3e44eeaee. What might be causing the issue is the usage of `instance_double` stubbing different methods in different tests (not sure this is the cause, though). We've seen that somebody got a similar error [1] (although it might not have been for the same reason) and one of the maintainers of rspec-mocks replied: > I would recommend switching to double (as you mentioned) or > refactoring to use something more defined. So we're simply using `double`, which is what we usually use when stubbing objects in the tests. Doing so is faster than further investigating why the `instance_double` isn't reliable 100% of the time. [1] See issue 1587 in https://github.com/rspec/rspec-mocks/ --- spec/helpers/comments_helper_spec.rb | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/spec/helpers/comments_helper_spec.rb b/spec/helpers/comments_helper_spec.rb index f0d8989d5..24bc26267 100644 --- a/spec/helpers/comments_helper_spec.rb +++ b/spec/helpers/comments_helper_spec.rb @@ -12,25 +12,26 @@ require "rails_helper" # end RSpec.describe CommentsHelper do describe "#user_level_class" do - def comment_double(as_administrator: false, as_moderator: false, official: false) - user = instance_double(User, official?: official, official_level: "Y") - instance_double(Comment, as_administrator?: as_administrator, as_moderator?: as_moderator, user: user) + def comment_double(**attributes) + defaults = { as_administrator?: false, as_moderator?: false, user: double(official?: false) } + + double(defaults.merge(attributes)) end it "returns is-admin for comment done as administrator" do - comment = comment_double(as_administrator: true) + comment = comment_double(as_administrator?: true) expect(helper.user_level_class(comment)).to eq("is-admin") end it "returns is-moderator for comment done as moderator" do - comment = comment_double(as_moderator: true) + comment = comment_double(as_moderator?: true) expect(helper.user_level_class(comment)).to eq("is-moderator") end it "returns level followed by official level if user is official" do - comment = comment_double(official: true) + comment = comment_double(user: double(official?: true, official_level: "Y")) expect(helper.user_level_class(comment)).to eq("level-Y") end @@ -45,14 +46,14 @@ RSpec.describe CommentsHelper do describe "#comment_author_class" do it "returns is-author if author is the commenting user" do author_id = 42 - comment = instance_double(Comment, user_id: author_id) + comment = double(user_id: author_id) expect(helper.comment_author_class(comment, author_id)).to eq("is-author") end it "returns an empty string if commenter is not the author" do author_id = 42 - comment = instance_double(Comment, user_id: author_id - 1) + comment = double(user_id: author_id - 1) expect(helper.comment_author_class(comment, author_id)).to eq("") end From bb50f02ba1b6a871dcd88f6a8bd342825020affc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 6 Nov 2024 15:27:21 +0100 Subject: [PATCH 2/3] Replace instance_double usages with double After the previous commit, we were using `double` in many places but were only using `instance_double` in one file. So, for consistency, we're using `double` in this file as well. --- .../verification/management/document_spec.rb | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/spec/models/verification/management/document_spec.rb b/spec/models/verification/management/document_spec.rb index e9eb02552..8a43a49fd 100644 --- a/spec/models/verification/management/document_spec.rb +++ b/spec/models/verification/management/document_spec.rb @@ -84,40 +84,34 @@ describe Verification::Management::Document do describe "#valid_age?" do it "returns false when the user is younger than the user's minimum required age" do - census_response = instance_double(CensusApi::Response, - date_of_birth: under_minium_age_date_of_birth) + census_response = double(date_of_birth: under_minium_age_date_of_birth) expect(Verification::Management::Document.new.valid_age?(census_response)).to be false end it "returns true when the user has the user's minimum required age" do - census_response = instance_double(CensusApi::Response, - date_of_birth: just_minium_age_date_of_birth) + census_response = double(date_of_birth: just_minium_age_date_of_birth) expect(Verification::Management::Document.new.valid_age?(census_response)).to be true end it "returns true when the user is older than the user's minimum required age" do - census_response = instance_double(CensusApi::Response, - date_of_birth: over_minium_age_date_of_birth) + census_response = double(date_of_birth: over_minium_age_date_of_birth) expect(Verification::Management::Document.new.valid_age?(census_response)).to be true end end describe "#under_age?" do it "returns true when the user is younger than the user's minimum required age" do - census_response = instance_double(CensusApi::Response, - date_of_birth: under_minium_age_date_of_birth) + census_response = double(date_of_birth: under_minium_age_date_of_birth) expect(Verification::Management::Document.new.under_age?(census_response)).to be true end it "returns false when the user is user's minimum required age" do - census_response = instance_double(CensusApi::Response, - date_of_birth: just_minium_age_date_of_birth) + census_response = double(date_of_birth: just_minium_age_date_of_birth) expect(Verification::Management::Document.new.under_age?(census_response)).to be false end it "returns false when the user is older than user's minimum required age" do - census_response = instance_double(CensusApi::Response, - date_of_birth: over_minium_age_date_of_birth) + census_response = double(date_of_birth: over_minium_age_date_of_birth) expect(Verification::Management::Document.new.under_age?(census_response)).to be false end end From 7f40029a263a1496cc12f64638f05b81571e127b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Wed, 6 Nov 2024 15:33:29 +0100 Subject: [PATCH 3/3] Fix typo in document management verification tests --- .../verification/management/document_spec.rb | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/spec/models/verification/management/document_spec.rb b/spec/models/verification/management/document_spec.rb index 8a43a49fd..78d41c486 100644 --- a/spec/models/verification/management/document_spec.rb +++ b/spec/models/verification/management/document_spec.rb @@ -75,43 +75,43 @@ describe Verification::Management::Document do end describe "Allowed Age" do - let(:min_age) { User.minimum_required_age } - let(:over_minium_age_date_of_birth) { Date.new((min_age + 10).years.ago.year, 12, 31) } - let(:under_minium_age_date_of_birth) { Date.new(min_age.years.ago.year, 12, 31) } - let(:just_minium_age_date_of_birth) do + let(:min_age) { User.minimum_required_age } + let(:over_minimum_age_date_of_birth) { Date.new((min_age + 10).years.ago.year, 12, 31) } + let(:under_minimum_age_date_of_birth) { Date.new(min_age.years.ago.year, 12, 31) } + let(:just_minimum_age_date_of_birth) do Date.new(min_age.years.ago.year, min_age.years.ago.month, min_age.years.ago.day) end describe "#valid_age?" do it "returns false when the user is younger than the user's minimum required age" do - census_response = double(date_of_birth: under_minium_age_date_of_birth) + census_response = double(date_of_birth: under_minimum_age_date_of_birth) expect(Verification::Management::Document.new.valid_age?(census_response)).to be false end it "returns true when the user has the user's minimum required age" do - census_response = double(date_of_birth: just_minium_age_date_of_birth) + census_response = double(date_of_birth: just_minimum_age_date_of_birth) expect(Verification::Management::Document.new.valid_age?(census_response)).to be true end it "returns true when the user is older than the user's minimum required age" do - census_response = double(date_of_birth: over_minium_age_date_of_birth) + census_response = double(date_of_birth: over_minimum_age_date_of_birth) expect(Verification::Management::Document.new.valid_age?(census_response)).to be true end end describe "#under_age?" do it "returns true when the user is younger than the user's minimum required age" do - census_response = double(date_of_birth: under_minium_age_date_of_birth) + census_response = double(date_of_birth: under_minimum_age_date_of_birth) expect(Verification::Management::Document.new.under_age?(census_response)).to be true end it "returns false when the user is user's minimum required age" do - census_response = double(date_of_birth: just_minium_age_date_of_birth) + census_response = double(date_of_birth: just_minimum_age_date_of_birth) expect(Verification::Management::Document.new.under_age?(census_response)).to be false end it "returns false when the user is older than user's minimum required age" do - census_response = double(date_of_birth: over_minium_age_date_of_birth) + census_response = double(date_of_birth: over_minimum_age_date_of_birth) expect(Verification::Management::Document.new.under_age?(census_response)).to be false end end