From 49751f46ec2cf25c3b6ec1c64f2495c2416c153c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Sat, 7 Sep 2019 16:31:00 +0200 Subject: [PATCH] Fix tests for uppercase tags The tests in the `spec/lib/graphql_spec.rb` failed sometimes because creating a record with a tag list of ["health"] when both "health" and "Health" tags exist might assign either one of them. These tests usually pass because we create two records and just by chance usually one of the records gets one tag and the other one gets the other tag. However, the test was written as if we expected the first record to get the first tag and the second record to get the second tag, while very often the tests were passing because the first record got the second tag and the second record got the first tag. And when both records get the same tag, the tests fail. So I've changed these tests to tags are assigned directly and, since we want to test the `tag_list` method, I've also added some tests to the Tag model, which reflect the current behaviour: a random tag is assigned when several tags with the same case-insensitive name exist. Another option to assign the right tag to the record we're creating would be to add `ActsAsTaggableOn.strict_case_match = true` to an initializer. However, that would also create new tags on the database when we accidentally assign a tag like "hEaLth" (like in the test we add in this commit). Ideally we would have a strict case match for existing tags and a non-strict case match for new tags, but I haven't found a way to do it. --- spec/lib/graphql_spec.rb | 35 ++++++++++++++++++----------------- spec/models/tag_spec.rb | 19 +++++++++++++++++++ 2 files changed, 37 insertions(+), 17 deletions(-) diff --git a/spec/lib/graphql_spec.rb b/spec/lib/graphql_spec.rb index aad7624f4..18d80aa41 100644 --- a/spec/lib/graphql_spec.rb +++ b/spec/lib/graphql_spec.rb @@ -511,28 +511,29 @@ describe "Consul Schema" do expect(received_tags).to match_array ["Parks", "Health"] end - it "uppercase and lowercase tags work ok together for proposals" do - create(:tag, name: "Health") - create(:tag, name: "health") - create(:proposal, tag_list: "health") - create(:proposal, tag_list: "Health") + context "uppercase and lowercase tags" do + let(:uppercase_tag) { create(:tag, name: "Health") } + let(:lowercase_tag) { create(:tag, name: "health") } - response = execute("{ tags { edges { node { name } } } }") - received_tags = extract_fields(response, "tags", "name") + it "works OK when both tags are present for proposals" do + create(:proposal).tags = [uppercase_tag] + create(:proposal).tags = [lowercase_tag] - expect(received_tags).to match_array ["Health", "health"] - end + response = execute("{ tags { edges { node { name } } } }") + received_tags = extract_fields(response, "tags", "name") - it "uppercase and lowercase tags work ok together for debates" do - create(:tag, name: "Health") - create(:tag, name: "health") - create(:debate, tag_list: "Health") - create(:debate, tag_list: "health") + expect(received_tags).to match_array ["Health", "health"] + end - response = execute("{ tags { edges { node { name } } } }") - received_tags = extract_fields(response, "tags", "name") + it "works OK when both tags are present for proposals" do + create(:debate).tags = [uppercase_tag] + create(:debate).tags = [lowercase_tag] - expect(received_tags).to match_array ["Health", "health"] + response = execute("{ tags { edges { node { name } } } }") + received_tags = extract_fields(response, "tags", "name") + + expect(received_tags).to match_array ["Health", "health"] + end end it "does not display tags for hidden proposals" do diff --git a/spec/models/tag_spec.rb b/spec/models/tag_spec.rb index 2cbdb0951..95dea8cb8 100644 --- a/spec/models/tag_spec.rb +++ b/spec/models/tag_spec.rb @@ -34,4 +34,23 @@ describe Tag do expect(tag).to be_valid end end + + context "Same tag uppercase and lowercase" do + before do + create(:tag, name: "Health") + create(:tag, name: "health") + end + + it "assigns only one of the existing tags (we can't control which one)" do + debate = create(:debate, tag_list: "Health") + + expect([["Health"], ["health"]]).to include debate.reload.tag_list + end + + it "assigns existing tags instead of creating new similar ones" do + debate = create(:debate, tag_list: "hEaLth") + + expect([["Health"], ["health"]]).to include debate.reload.tag_list + end + end end