From: Andy Allan <git@gravitystorm.co.uk>
Date: Sun, 30 Oct 2016 14:36:33 +0000 (+0100)
Subject: Replace fixtures with a factory for old_node_tags
X-Git-Tag: live~3685^2~7
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/acb7f440690b1f773f71ded256f3986a5606b334?hp=48eb8eb3690851f7f26afc68e1524341fcb6c187

Replace fixtures with a factory for old_node_tags

Also includes a slight simplification of the old_node_tags tests.
---

diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
index 95ddea8f8..b5834b8ce 100644
--- a/.rubocop_todo.yml
+++ b/.rubocop_todo.yml
@@ -64,7 +64,7 @@ Metrics/BlockNesting:
 # Offense count: 62
 # Configuration parameters: CountComments.
 Metrics/ClassLength:
-  Max: 1652
+  Max: 1653
 
 # Offense count: 69
 Metrics/CyclomaticComplexity:
diff --git a/test/controllers/changeset_controller_test.rb b/test/controllers/changeset_controller_test.rb
index a9a838aff..9c77040fe 100644
--- a/test/controllers/changeset_controller_test.rb
+++ b/test/controllers/changeset_controller_test.rb
@@ -1389,7 +1389,10 @@ EOF
   end
 
   def test_changeset_download
+    tag = create(:old_node_tag, :old_node => nodes(:used_node_2))
+
     get :download, :id => changesets(:normal_user_first_change).id
+
     assert_response :success
     assert_template nil
     # print @response.body
@@ -1397,7 +1400,7 @@ EOF
     assert_select "osmChange[version='#{API_VERSION}'][generator='#{GENERATOR}']" do
       assert_select "create", :count => 5
       assert_select "create>node[id='#{nodes(:used_node_2).node_id}'][visible='#{nodes(:used_node_2).visible?}'][version='#{nodes(:used_node_2).version}']" do
-        assert_select "tag[k='#{node_tags(:t3).k}'][v='#{node_tags(:t3).v}']"
+        assert_select "tag[k='#{tag.k}'][v='#{tag.v}']"
       end
       assert_select "create>node[id='#{nodes(:visible_node).node_id}']"
     end
diff --git a/test/controllers/old_node_controller_test.rb b/test/controllers/old_node_controller_test.rb
index f2658b123..1fa82717a 100644
--- a/test/controllers/old_node_controller_test.rb
+++ b/test/controllers/old_node_controller_test.rb
@@ -90,6 +90,9 @@ class OldNodeControllerTest < ActionController::TestCase
     xml_node = xml_doc.find("//osm/node").first
     nodeid = current_nodes(:node_with_versions).id
 
+    # Ensure that the current tags are propogated to the history too
+    propogate_tags(current_nodes(:node_with_versions), nodes(:node_with_versions_v4))
+
     # keep a hash of the versions => string, as we'll need something
     # to test against later
     versions = {}
@@ -161,6 +164,12 @@ class OldNodeControllerTest < ActionController::TestCase
   # Test that getting the current version is identical to picking
   # that version with the version URI call.
   def test_current_version
+    propogate_tags(current_nodes(:visible_node), nodes(:visible_node))
+    propogate_tags(current_nodes(:used_node_1), nodes(:used_node_1))
+    propogate_tags(current_nodes(:used_node_2), nodes(:used_node_2))
+    propogate_tags(current_nodes(:node_used_by_relationship), nodes(:node_used_by_relationship))
+    propogate_tags(current_nodes(:node_with_versions), nodes(:node_with_versions_v4))
+
     check_current_version(current_nodes(:visible_node))
     check_current_version(current_nodes(:used_node_1))
     check_current_version(current_nodes(:used_node_2))
@@ -377,4 +386,10 @@ class OldNodeControllerTest < ActionController::TestCase
   def precision(f)
     (f * GeoRecord::SCALE).round.to_f / GeoRecord::SCALE
   end
+
+  def propogate_tags(node, old_node)
+    node.tags.each do |k, v|
+      create(:old_node_tag, :old_node => old_node, :k => k, :v => v)
+    end
+  end
 end
diff --git a/test/factories/old_node_tags.rb b/test/factories/old_node_tags.rb
new file mode 100644
index 000000000..5c63f3242
--- /dev/null
+++ b/test/factories/old_node_tags.rb
@@ -0,0 +1,10 @@
+FactoryGirl.define do
+  factory :old_node_tag do
+    sequence(:k) { |n| "Key #{n}" }
+    sequence(:v) { |n| "Value #{n}" }
+
+    # Fixme requires old_node factory
+    node_id 1
+    version 1
+  end
+end
diff --git a/test/fixtures/node_tags.yml b/test/fixtures/node_tags.yml
deleted file mode 100644
index d9001b720..000000000
--- a/test/fixtures/node_tags.yml
+++ /dev/null
@@ -1,113 +0,0 @@
-t1:
-  node_id: 1
-  k: 'testvisible'
-  v: 'yes'
-  version: 1
-
-t2:
-  node_id: 3
-  k: 'test'
-  v: 'yes'
-  version: 1
-
-t3:
-  node_id: 4
-  k: 'test'
-  v: 'yes'
-  version: 1
-
-nv3_t1:
-  node_id: 15
-  k: 'testing'
-  v: 'added in node version 3'
-  version: 3
-
-nv3_t2:
-  node_id: 15
-  k: 'testing two'
-  v: 'added in node version 3'
-  version: 3
-
-nv3_t3:
-  node_id: 15
-  k: 'testing three'
-  v: 'added in node version 3'
-  version: 3
-
-nv4_t1:
-  node_id: 15
-  k: 'testing'
-  v: 'added in node version 3'
-  version: 4
-
-nv4_t2:
-  node_id: 15
-  k: 'testing two'
-  v: 'modified in node version 4'
-  version: 4
-  
-public_v_t1:
-  node_id: 16
-  k: 'testvisible'
-  v: 'yes'
-  version: 1
-
-nwnv1_name:
-  node_id: 18
-  k: 'name'
-  v: 'Test Node'
-  version: 1
-
-nwnv1_name_pt:
-  node_id: 18
-  k: 'name:pt'
-  v: 'Nó teste'
-  version: 1
-
-nwnv1_building:
-  node_id: 18
-  k: 'building'
-  v: 'yes'
-  version: 1
-
-nwnv1_tourism:
-  node_id: 18
-  k: 'tourism'
-  v: 'museum'
-  version: 1
-
-nwnv1_shop:
-  node_id: 18
-  k: 'shop'
-  v: 'gift'
-  version: 1
-
-nwnv2_name:
-  node_id: 18
-  k: 'name'
-  v: 'Test Node'
-  version: 2
-
-nwnv2_name_pt:
-  node_id: 18
-  k: 'name:pt'
-  v: 'Nó teste'
-  version: 2
-
-nwnv2_building:
-  node_id: 18
-  k: 'building'
-  v: 'yes'
-  version: 2
-
-nwnv2_tourism:
-  node_id: 18
-  k: 'tourism'
-  v: 'museum'
-  version: 2
-
-nwnv2_shop:
-  node_id: 18
-  k: 'shop'
-  v: 'gift'
-  version: 2
diff --git a/test/helpers/browse_helper_test.rb b/test/helpers/browse_helper_test.rb
index 798c69b43..f7f3eaef8 100644
--- a/test/helpers/browse_helper_test.rb
+++ b/test/helpers/browse_helper_test.rb
@@ -17,6 +17,8 @@ class BrowseHelperTest < ActionView::TestCase
   end
 
   def test_printable_name
+    add_old_tags_selection(nodes(:node_with_name_current_version))
+    add_old_tags_selection(nodes(:node_with_name_redacted_version))
     assert_dom_equal "17", printable_name(current_nodes(:redacted_node))
     assert_dom_equal "<bdi>Test Node</bdi> (<bdi>18</bdi>)", printable_name(current_nodes(:node_with_name))
     assert_dom_equal "<bdi>Test Node</bdi> (<bdi>18</bdi>)", printable_name(nodes(:node_with_name_current_version))
@@ -61,6 +63,9 @@ class BrowseHelperTest < ActionView::TestCase
     assert_equal "node deleted", link_class("node", current_nodes(:invisible_node))
     assert_equal "node deleted", link_class("node", current_nodes(:redacted_node))
     assert_equal "node building yes shop gift tourism museum", link_class("node", current_nodes(:node_with_name))
+
+    add_old_tags_selection(nodes(:node_with_name_current_version))
+    add_old_tags_selection(nodes(:node_with_name_redacted_version))
     assert_equal "node building yes shop gift tourism museum", link_class("node", nodes(:node_with_name_current_version))
     assert_equal "node deleted", link_class("node", nodes(:node_with_name_redacted_version))
   end
@@ -70,6 +75,9 @@ class BrowseHelperTest < ActionView::TestCase
     assert_equal "", link_title(current_nodes(:invisible_node))
     assert_equal "", link_title(current_nodes(:redacted_node))
     assert_equal "building=yes, shop=gift, and tourism=museum", link_title(current_nodes(:node_with_name))
+
+    add_old_tags_selection(nodes(:node_with_name_current_version))
+    add_old_tags_selection(nodes(:node_with_name_redacted_version))
     assert_equal "building=yes, shop=gift, and tourism=museum", link_title(nodes(:node_with_name_current_version))
     assert_equal "", link_title(nodes(:node_with_name_redacted_version))
   end
@@ -112,6 +120,9 @@ class BrowseHelperTest < ActionView::TestCase
     assert tags.include?(%w(tourism museum))
     assert tags.include?(%w(shop gift))
 
+    add_old_tags_selection(nodes(:node_with_name_current_version))
+    add_old_tags_selection(nodes(:node_with_name_redacted_version))
+
     tags = icon_tags(nodes(:node_with_name_current_version))
     assert_equal 3, tags.count
     assert tags.include?(%w(building yes))
@@ -306,4 +317,14 @@ class BrowseHelperTest < ActionView::TestCase
     link = telephone_link("phone", "+1 (234) 567-890")
     assert_equal "tel:+1(234)567-890", link
   end
+
+  def add_old_tags_selection(old_node)
+    { "building" => "yes",
+      "shop" => "gift",
+      "tourism" => "museum",
+      "name" => "Test Node",
+      "name:pt" => "Nó teste" }.each do |key, value|
+      create(:old_node_tag, :old_node => old_node, :k => key, :v => value)
+    end
+  end
 end
diff --git a/test/models/old_node_tag_test.rb b/test/models/old_node_tag_test.rb
index 2ac60ab42..f49be9256 100644
--- a/test/models/old_node_tag_test.rb
+++ b/test/models/old_node_tag_test.rb
@@ -3,56 +3,34 @@ require "test_helper"
 class OldNodeTagTest < ActiveSupport::TestCase
   api_fixtures
 
-  def test_old_node_tag_count
-    assert_equal 19, OldNodeTag.count, "Unexpected number of fixtures loaded."
-  end
-
   def test_length_key_valid
-    key = "k"
+    tag = create(:old_node_tag)
     (0..255).each do |i|
-      tag = OldNodeTag.new
-      tag.node_id = node_tags(:t1).node_id
-      tag.version = node_tags(:t1).version
-      tag.k = key * i
-      tag.v = "v"
+      tag.k = "k" * i
       assert tag.valid?
     end
   end
 
   def test_length_value_valid
-    val = "v"
+    tag = create(:old_node_tag)
     (0..255).each do |i|
-      tag = OldNodeTag.new
-      tag.node_id = node_tags(:t1).node_id
-      tag.version = node_tags(:t1).version
-      tag.k = "k"
-      tag.v = val * i
+      tag.v = "v" * i
       assert tag.valid?
     end
   end
 
   def test_length_key_invalid
-    ["k" * 256].each do |i|
-      tag = OldNodeTag.new
-      tag.node_id = node_tags(:t1).node_id
-      tag.version = node_tags(:t1).version
-      tag.k = i
-      tag.v = "v", "Key should be too long"
-      assert !tag.valid?
-      assert tag.errors[:k].any?
-    end
+    tag = create(:old_node_tag)
+    tag.k = "k" * 256
+    assert !tag.valid?
+    assert tag.errors[:k].any?
   end
 
   def test_length_value_invalid
-    ["k" * 256].each do |i|
-      tag = OldNodeTag.new
-      tag.node_id = node_tags(:t1).node_id
-      tag.version = node_tags(:t1).version
-      tag.k = "k"
-      tag.v = i
-      assert !tag.valid?, "Value should be too long"
-      assert tag.errors[:v].any?
-    end
+    tag = create(:old_node_tag)
+    tag.v = "v" * 256
+    assert !tag.valid?, "Value should be too long"
+    assert tag.errors[:v].any?
   end
 
   def test_empty_tag_invalid
@@ -62,11 +40,12 @@ class OldNodeTagTest < ActiveSupport::TestCase
   end
 
   def test_uniqueness
+    existing = create(:old_node_tag)
     tag = OldNodeTag.new
-    tag.node_id = node_tags(:t1).node_id
-    tag.version = node_tags(:t1).version
-    tag.k = node_tags(:t1).k
-    tag.v = node_tags(:t1).v
+    tag.node_id = existing.node_id
+    tag.version = existing.version
+    tag.k = existing.k
+    tag.v = existing.v
     assert tag.new_record?
     assert !tag.valid?
     assert_raise(ActiveRecord::RecordInvalid) { tag.save! }
diff --git a/test/models/old_node_test.rb b/test/models/old_node_test.rb
index 03aead2e9..6d25d7667 100644
--- a/test/models/old_node_test.rb
+++ b/test/models/old_node_test.rb
@@ -76,6 +76,9 @@ class OldNodeTest < ActiveSupport::TestCase
   end
 
   def test_node_tags
+    taglist_v3 = create_list(:old_node_tag, 3, :old_node => nodes(:node_with_versions_v3))
+    taglist_v4 = create_list(:old_node_tag, 2, :old_node => nodes(:node_with_versions_v4))
+
     node = nodes(:node_with_versions_v1)
     tags = OldNode.find(node.id).old_tags.order(:k)
     assert_equal 0, tags.count
@@ -87,23 +90,24 @@ class OldNodeTest < ActiveSupport::TestCase
     node = nodes(:node_with_versions_v3)
     tags = OldNode.find(node.id).old_tags.order(:k)
     assert_equal 3, tags.count
-    assert_equal "testing", tags[0].k
-    assert_equal "added in node version 3", tags[0].v
-    assert_equal "testing three", tags[1].k
-    assert_equal "added in node version 3", tags[1].v
-    assert_equal "testing two", tags[2].k
-    assert_equal "added in node version 3", tags[2].v
+    taglist_v3.sort_by(&:k).each_index do |i|
+      assert_equal taglist_v3[i].k, tags[i].k
+      assert_equal taglist_v3[i].v, tags[i].v
+    end
 
     node = nodes(:node_with_versions_v4)
     tags = OldNode.find(node.id).old_tags.order(:k)
     assert_equal 2, tags.count
-    assert_equal "testing", tags[0].k
-    assert_equal "added in node version 3", tags[0].v
-    assert_equal "testing two", tags[1].k
-    assert_equal "modified in node version 4", tags[1].v
+    taglist_v4.sort_by(&:k).each_index do |i|
+      assert_equal taglist_v4[i].k, tags[i].k
+      assert_equal taglist_v4[i].v, tags[i].v
+    end
   end
 
   def test_tags
+    taglist_v3 = create_list(:old_node_tag, 3, :old_node => nodes(:node_with_versions_v3))
+    taglist_v4 = create_list(:old_node_tag, 2, :old_node => nodes(:node_with_versions_v4))
+
     node = nodes(:node_with_versions_v1)
     tags = OldNode.find(node.id).tags
     assert_equal 0, tags.size
@@ -115,14 +119,15 @@ class OldNodeTest < ActiveSupport::TestCase
     node = nodes(:node_with_versions_v3)
     tags = OldNode.find(node.id).tags
     assert_equal 3, tags.size
-    assert_equal "added in node version 3", tags["testing"]
-    assert_equal "added in node version 3", tags["testing two"]
-    assert_equal "added in node version 3", tags["testing three"]
+    taglist_v3.each do |tag|
+      assert_equal tag.v, tags[tag.k]
+    end
 
     node = nodes(:node_with_versions_v4)
     tags = OldNode.find(node.id).tags
     assert_equal 2, tags.size
-    assert_equal "added in node version 3", tags["testing"]
-    assert_equal "modified in node version 4", tags["testing two"]
+    taglist_v4.each do |tag|
+      assert_equal tag.v, tags[tag.k]
+    end
   end
 end
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 0cf7c4afb..0fced7ba8 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -20,9 +20,8 @@ module ActiveSupport
       set_fixture_class :current_nodes => Node
       set_fixture_class :nodes => OldNode
 
-      fixtures :current_node_tags, :node_tags
+      fixtures :current_node_tags
       set_fixture_class :current_node_tags => NodeTag
-      set_fixture_class :node_tags => OldNodeTag
 
       fixtures :current_ways
       set_fixture_class :current_ways => Way