From: Andy Allan <git@gravitystorm.co.uk>
Date: Sun, 30 Oct 2016 17:28:01 +0000 (+0100)
Subject: Replace fixtures with factory for node_tags
X-Git-Tag: live~3633^2~4
X-Git-Url: https://git.openstreetmap.org/rails.git/commitdiff_plain/3026af170a18d04d624144251d7b3acdeee03fea

Replace fixtures with factory for node_tags
---

diff --git a/test/controllers/api_controller_test.rb b/test/controllers/api_controller_test.rb
index 0a185baae..bcc544263 100644
--- a/test/controllers/api_controller_test.rb
+++ b/test/controllers/api_controller_test.rb
@@ -53,6 +53,7 @@ class ApiControllerTest < ActionController::TestCase
 
   def test_map
     node = current_nodes(:used_node_1)
+    tag = create(:node_tag, :node => node)
     # Need to split the min/max lat/lon out into their own variables here
     # so that we can test they are returned later.
     minlon = node.lon - 0.1
@@ -70,7 +71,7 @@ class ApiControllerTest < ActionController::TestCase
       assert_select "bounds[minlon='#{minlon}'][minlat='#{minlat}'][maxlon='#{maxlon}'][maxlat='#{maxlat}']", :count => 1
       assert_select "node[id='#{node.id}'][lat='#{node.lat}'][lon='#{node.lon}'][version='#{node.version}'][changeset='#{node.changeset_id}'][visible='#{node.visible}'][timestamp='#{node.timestamp.xmlschema}']", :count => 1 do
         # This should really be more generic
-        assert_select "tag[k='test'][v='yes']"
+        assert_select "tag[k='#{tag.k}'][v='#{tag.v}']"
       end
       assert_select "way", :count => 2
       assert_select "way[id='1']", :count => 1
@@ -84,6 +85,7 @@ class ApiControllerTest < ActionController::TestCase
   # the same as the node we are looking at
   def test_map_inclusive
     node = current_nodes(:used_node_1)
+    tag = create(:node_tag, :node => node)
     bbox = "#{node.lon},#{node.lat},#{node.lon},#{node.lat}"
     get :map, :bbox => bbox
     assert_response :success, "The map call should have succeeded"
@@ -91,7 +93,7 @@ class ApiControllerTest < ActionController::TestCase
       assert_select "bounds[minlon='#{node.lon}'][minlat='#{node.lat}'][maxlon='#{node.lon}'][maxlat='#{node.lat}']", :count => 1
       assert_select "node[id='#{node.id}'][lat='#{node.lat}'][lon='#{node.lon}'][version='#{node.version}'][changeset='#{node.changeset_id}'][visible='#{node.visible}'][timestamp='#{node.timestamp.xmlschema}']", :count => 1 do
         # This should really be more generic
-        assert_select "tag[k='test'][v='yes']"
+        assert_select "tag[k='#{tag.k}'][v='#{tag.v}']"
       end
       assert_select "way", :count => 2
       assert_select "way[id='1']", :count => 1
diff --git a/test/controllers/node_controller_test.rb b/test/controllers/node_controller_test.rb
index 0cfc2267c..ffc86b2f6 100644
--- a/test/controllers/node_controller_test.rb
+++ b/test/controllers/node_controller_test.rb
@@ -437,13 +437,14 @@ class NodeControllerTest < ActionController::TestCase
   ##
   # test adding tags to a node
   def test_duplicate_tags
+    existing = create(:node_tag, :node => current_nodes(:public_visible_node))
     # setup auth
     basic_authorization(users(:public_user).email, "test")
 
     # add an identical tag to the node
     tag_xml = XML::Node.new("tag")
-    tag_xml["k"] = current_node_tags(:public_v_t1).k
-    tag_xml["v"] = current_node_tags(:public_v_t1).v
+    tag_xml["k"] = existing.k
+    tag_xml["v"] = existing.v
 
     # add the tag into the existing xml
     node_xml = current_nodes(:public_visible_node).to_xml
@@ -454,7 +455,7 @@ class NodeControllerTest < ActionController::TestCase
     put :update, :id => current_nodes(:public_visible_node).id
     assert_response :bad_request,
                     "adding duplicate tags to a node should fail with 'bad request'"
-    assert_equal "Element node/#{current_nodes(:public_visible_node).id} has duplicate tags with key #{current_node_tags(:t1).k}", @response.body
+    assert_equal "Element node/#{current_nodes(:public_visible_node).id} has duplicate tags with key #{existing.k}", @response.body
   end
 
   # test whether string injection is possible
diff --git a/test/factories/node_tags.rb b/test/factories/node_tags.rb
new file mode 100644
index 000000000..267309e90
--- /dev/null
+++ b/test/factories/node_tags.rb
@@ -0,0 +1,9 @@
+FactoryGirl.define do
+  factory :node_tag do
+    sequence(:k) { |n| "Key #{n}" }
+    sequence(:v) { |n| "Value #{n}" }
+
+    # Fixme requires node factory
+    node_id 1
+  end
+end
diff --git a/test/fixtures/current_node_tags.yml b/test/fixtures/current_node_tags.yml
deleted file mode 100644
index d299998b5..000000000
--- a/test/fixtures/current_node_tags.yml
+++ /dev/null
@@ -1,64 +0,0 @@
-t1:
-  node_id: 1
-  k: 'testvisible'
-  v: 'yes'
-
-t2:
-  node_id: 2
-  k: 'testused'
-  v: 'yes'
-
-t3:
-  node_id: 3
-  k: 'test'
-  v: 'yes'
-
-t4:
-  node_id: 4
-  k: 'test'
-  v: 'yes'
-
-nv_t1:
-  node_id: 15
-  k: 'testing'
-  v: 'added in node version 3'
-
-nv_t2:
-  node_id: 15
-  k: 'testing two'
-  v: 'modified in node version 4'
-  
-public_v_t1:
-  node_id: 16
-  k: 'testvisible'
-  v: 'yes'
-
-nwn_name:
-  node_id: 18
-  k: 'name'
-  v: 'Test Node'
-
-nwn_name_pt:
-  node_id: 18
-  k: 'name:pt'
-  v: 'Nó teste'
-
-nwn_building:
-  node_id: 18
-  k: 'building'
-  v: 'yes'
-
-nwn_tourism:
-  node_id: 18
-  k: 'tourism'
-  v: 'museum'
-
-nwn_shop:
-  node_id: 18
-  k: 'shop'
-  v: 'gift'
-
-nwrwn_ref:
-  node_id: 19
-  k: 'ref'
-  v: '3.1415926'
diff --git a/test/helpers/browse_helper_test.rb b/test/helpers/browse_helper_test.rb
index f7f3eaef8..32cf96c6b 100644
--- a/test/helpers/browse_helper_test.rb
+++ b/test/helpers/browse_helper_test.rb
@@ -17,8 +17,12 @@ class BrowseHelperTest < ActionView::TestCase
   end
 
   def test_printable_name
+    add_tags_selection(current_nodes(:node_with_name))
+    create(:node_tag, :node => current_nodes(:node_with_ref_without_name), :k => "ref", :v => "3.1415926")
     add_old_tags_selection(nodes(:node_with_name_current_version))
     add_old_tags_selection(nodes(:node_with_name_redacted_version))
+
+    # current_nodes(:redacted_node) is deleted, so has no tags.
     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))
@@ -59,6 +63,8 @@ class BrowseHelperTest < ActionView::TestCase
   end
 
   def test_link_class
+    add_tags_selection(current_nodes(:node_with_name))
+
     assert_equal "node", link_class("node", current_nodes(:visible_node))
     assert_equal "node deleted", link_class("node", current_nodes(:invisible_node))
     assert_equal "node deleted", link_class("node", current_nodes(:redacted_node))
@@ -71,6 +77,8 @@ class BrowseHelperTest < ActionView::TestCase
   end
 
   def test_link_title
+    add_tags_selection(current_nodes(:node_with_name))
+
     assert_equal "", link_title(current_nodes(:visible_node))
     assert_equal "", link_title(current_nodes(:invisible_node))
     assert_equal "", link_title(current_nodes(:redacted_node))
@@ -114,6 +122,8 @@ class BrowseHelperTest < ActionView::TestCase
   end
 
   def test_icon_tags
+    add_tags_selection(current_nodes(:node_with_name))
+
     tags = icon_tags(current_nodes(:node_with_name))
     assert_equal 3, tags.count
     assert tags.include?(%w(building yes))
@@ -327,4 +337,14 @@ class BrowseHelperTest < ActionView::TestCase
       create(:old_node_tag, :old_node => old_node, :k => key, :v => value)
     end
   end
+
+  def add_tags_selection(node)
+    { "building" => "yes",
+      "shop" => "gift",
+      "tourism" => "museum",
+      "name" => "Test Node",
+      "name:pt" => "Nó teste" }.each do |key, value|
+      create(:node_tag, :node => node, :k => key, :v => value)
+    end
+  end
 end
diff --git a/test/models/node_tag_test.rb b/test/models/node_tag_test.rb
index 0196b6fef..29eda2bff 100644
--- a/test/models/node_tag_test.rb
+++ b/test/models/node_tag_test.rb
@@ -1,64 +1,34 @@
 require "test_helper"
 
 class NodeTagTest < ActiveSupport::TestCase
-  api_fixtures
-
-  def test_tag_count
-    assert_equal 13, NodeTag.count
-    node_tag_count(:visible_node, 1)
-    node_tag_count(:invisible_node, 1)
-    node_tag_count(:used_node_1, 1)
-    node_tag_count(:used_node_2, 1)
-    node_tag_count(:node_with_versions, 2)
-  end
-
-  def node_tag_count(node, count)
-    nod = current_nodes(node)
-    assert_equal count, nod.node_tags.count
-  end
-
   def test_length_key_valid
-    key = "k"
+    tag = create(:node_tag)
     (0..255).each do |i|
-      tag = NodeTag.new
-      tag.node_id = current_node_tags(:t1).node_id
-      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(:node_tag)
     (0..255).each do |i|
-      tag = NodeTag.new
-      tag.node_id = current_node_tags(:t1).node_id
-      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 = NodeTag.new
-      tag.node_id = current_node_tags(:t1).node_id
-      tag.k = i
-      tag.v = "v"
-      assert !tag.valid?, "Key should be too long"
-      assert tag.errors[:k].any?
-    end
+    tag = create(:node_tag)
+    tag.k = "k" * 256
+    assert !tag.valid?, "Key should be too long"
+    assert tag.errors[:k].any?
   end
 
   def test_length_value_invalid
-    ["k" * 256].each do |i|
-      tag = NodeTag.new
-      tag.node_id = current_node_tags(:t1).node_id
-      tag.k = "k"
-      tag.v = i
-      assert !tag.valid?, "Value should be too long"
-      assert tag.errors[:v].any?
-    end
+    tag = create(:node_tag)
+    tag.v = "v" * 256
+    assert !tag.valid?, "Value should be too long"
+    assert tag.errors[:v].any?
   end
 
   def test_empty_node_tag_invalid
@@ -68,10 +38,11 @@ class NodeTagTest < ActiveSupport::TestCase
   end
 
   def test_uniqueness
+    existing = create(:node_tag)
     tag = NodeTag.new
-    tag.node_id = current_node_tags(:t1).node_id
-    tag.k = current_node_tags(:t1).k
-    tag.v = current_node_tags(:t1).v
+    tag.node_id = existing.node_id
+    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/node_test.rb b/test/models/node_test.rb
index ff833d0ed..68c45782d 100644
--- a/test/models/node_test.rb
+++ b/test/models/node_test.rb
@@ -316,20 +316,23 @@ class NodeTest < ActiveSupport::TestCase
 
   def test_node_tags
     node = current_nodes(:node_with_versions)
+    taglist = create_list(:node_tag, 2, :node => node)
     tags = Node.find(node.id).node_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.sort_by!(&:k).each_index do |i|
+      assert_equal taglist[i].k, tags[i].k
+      assert_equal taglist[i].v, tags[i].v
+    end
   end
 
   def test_tags
     node = current_nodes(:node_with_versions)
+    taglist = create_list(:node_tag, 2, :node => node)
     tags = Node.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.each do |tag|
+      assert_equal tag.v, tags[tag.k]
+    end
   end
 
   def test_containing_relation_members
diff --git a/test/models/old_way_test.rb b/test/models/old_way_test.rb
index b65963260..3acd7dba7 100644
--- a/test/models/old_way_test.rb
+++ b/test/models/old_way_test.rb
@@ -95,15 +95,18 @@ class OldWayTest < ActiveSupport::TestCase
 
   def test_get_nodes_undelete
     way = ways(:way_with_versions_v3)
+    node_tag = create(:node_tag, :node => current_nodes(:node_with_versions))
+    node_tag2 = create(:node_tag, :node => current_nodes(:used_node_1))
     nodes = OldWay.find(way.id).get_nodes_undelete
     assert_equal 2, nodes.size
-    assert_equal [1.0, 1.0, 15, 4, { "testing" => "added in node version 3", "testing two" => "modified in node version 4" }, true], nodes[0]
-    assert_equal [3.0, 3.0, 3, 1, { "test" => "yes" }, true], nodes[1]
+    assert_equal [1.0, 1.0, 15, 4, { node_tag.k => node_tag.v }, true], nodes[0]
+    assert_equal [3.0, 3.0, 3, 1, { node_tag2.k => node_tag2.v }, true], nodes[1]
 
     way = ways(:way_with_redacted_versions_v2)
+    node_tag3 = create(:node_tag, :node => current_nodes(:invisible_node))
     nodes = OldWay.find(way.id).get_nodes_undelete
     assert_equal 2, nodes.size
-    assert_equal [3.0, 3.0, 3, 1, { "test" => "yes" }, true], nodes[0]
-    assert_equal [2.0, 2.0, 2, 1, { "testused" => "yes" }, false], nodes[1]
+    assert_equal [3.0, 3.0, 3, 1, { node_tag2.k => node_tag2.v }, true], nodes[0]
+    assert_equal [2.0, 2.0, 2, 1, { node_tag3.k => node_tag3.v }, false], nodes[1]
   end
 end
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 933f040d5..f8fb69916 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -20,9 +20,6 @@ module ActiveSupport
       set_fixture_class :current_nodes => Node
       set_fixture_class :nodes => OldNode
 
-      fixtures :current_node_tags
-      set_fixture_class :current_node_tags => NodeTag
-
       fixtures :current_ways
       set_fixture_class :current_ways => Way