From 15689af6bfa6220b9c539eb0717a3d75cc3c4799 Mon Sep 17 00:00:00 2001
From: Andy Allan <git@gravitystorm.co.uk>
Date: Sat, 29 Oct 2016 16:21:52 +0200
Subject: [PATCH 1/7] Replace trace-related fixtures with factories.

The mocking and stubbing in the controller tests is
unfortunate, but the models interact directly with the filesystem
using the trace id so that's hard to control any other way.
---
 test/controllers/amf_controller_test.rb   |   2 +-
 test/controllers/api_controller_test.rb   |  13 +-
 test/controllers/site_controller_test.rb  |   2 +-
 test/controllers/swf_controller_test.rb   |   8 +
 test/controllers/trace_controller_test.rb | 615 +++++++++++++---------
 test/controllers/user_controller_test.rb  |  10 +-
 test/factories/tracepoints.rb             |  11 +
 test/factories/traces.rb                  |  16 +
 test/factories/tracetags.rb               |   7 +
 test/fixtures/gps_points.yml              |  35 --
 test/fixtures/gpx_file_tags.yml           |  19 -
 test/fixtures/gpx_files.yml               | 129 -----
 test/integration/oauth_test.rb            |  12 +-
 test/models/trace_test.rb                 | 178 ++++---
 test/models/tracepoint_test.rb            |   9 +-
 test/models/tracetag_test.rb              |   8 +-
 test/test_helper.rb                       |   5 -
 17 files changed, 543 insertions(+), 536 deletions(-)
 create mode 100644 test/factories/tracepoints.rb
 create mode 100644 test/factories/traces.rb
 create mode 100644 test/factories/tracetags.rb
 delete mode 100644 test/fixtures/gps_points.yml
 delete mode 100644 test/fixtures/gpx_file_tags.yml
 delete mode 100644 test/fixtures/gpx_files.yml

diff --git a/test/controllers/amf_controller_test.rb b/test/controllers/amf_controller_test.rb
index 7c8c64ba0..8f58aa980 100644
--- a/test/controllers/amf_controller_test.rb
+++ b/test/controllers/amf_controller_test.rb
@@ -473,7 +473,7 @@ class AmfControllerTest < ActionController::TestCase
   end
 
   def test_findgpx_by_id
-    trace = gpx_files(:anon_trace_file)
+    trace = create(:trace, :visibility => "private", :user => users(:public_user))
 
     amf_content "findgpx", "/1", [trace.id, "test@example.com:test"]
     post :amf_read
diff --git a/test/controllers/api_controller_test.rb b/test/controllers/api_controller_test.rb
index 0a185baae..67469d094 100644
--- a/test/controllers/api_controller_test.rb
+++ b/test/controllers/api_controller_test.rb
@@ -132,7 +132,9 @@ class ApiControllerTest < ActionController::TestCase
   end
 
   def test_tracepoints
-    point = gpx_files(:public_trace_file)
+    point = create(:trace, :visibility => "public", :latitude => 1, :longitude => 1) do |trace|
+      create(:tracepoint, :trace => trace, :latitude => 1 * GeoRecord::SCALE, :longitude => 1 * GeoRecord::SCALE)
+    end
     minlon = point.longitude - 0.001
     minlat = point.latitude - 0.001
     maxlon = point.longitude + 0.001
@@ -148,7 +150,10 @@ class ApiControllerTest < ActionController::TestCase
   end
 
   def test_tracepoints_trackable
-    point = gpx_files(:trackable_trace_file)
+    point = create(:trace, :visibility => "trackable", :latitude => 51.51, :longitude => -0.14) do |trace|
+      create(:tracepoint, :trace => trace, :trackid => 1, :latitude => (51.510 * GeoRecord::SCALE).to_i, :longitude => (-0.140 * GeoRecord::SCALE).to_i)
+      create(:tracepoint, :trace => trace, :trackid => 2, :latitude => (51.511 * GeoRecord::SCALE).to_i, :longitude => (-0.141 * GeoRecord::SCALE).to_i)
+    end
     minlon = point.longitude - 0.002
     minlat = point.latitude - 0.002
     maxlon = point.longitude + 0.002
@@ -170,7 +175,9 @@ class ApiControllerTest < ActionController::TestCase
   end
 
   def test_tracepoints_identifiable
-    point = gpx_files(:identifiable_trace_file)
+    point = create(:trace, :visibility => "identifiable", :latitude => 51.512, :longitude => 0.142) do |trace|
+      create(:tracepoint, :trace => trace, :latitude => (51.512 * GeoRecord::SCALE).to_i, :longitude => (0.142 * GeoRecord::SCALE).to_i)
+    end
     minlon = point.longitude - 0.002
     minlat = point.latitude - 0.002
     maxlon = point.longitude + 0.002
diff --git a/test/controllers/site_controller_test.rb b/test/controllers/site_controller_test.rb
index 70eacb3aa..7e387f684 100644
--- a/test/controllers/site_controller_test.rb
+++ b/test/controllers/site_controller_test.rb
@@ -280,7 +280,7 @@ class SiteControllerTest < ActionController::TestCase
   # Test editing a specific GPX trace
   def test_edit_with_gpx
     user = users(:public_user)
-    gpx  = gpx_files(:public_trace_file)
+    gpx  = create(:trace, :latitude => 1, :longitude => 1)
 
     get :edit, { :gpx => gpx.id }, { :user => user.id }
     assert_response :success
diff --git a/test/controllers/swf_controller_test.rb b/test/controllers/swf_controller_test.rb
index 25ed41947..13322c6b5 100644
--- a/test/controllers/swf_controller_test.rb
+++ b/test/controllers/swf_controller_test.rb
@@ -15,6 +15,14 @@ class SwfControllerTest < ActionController::TestCase
   ##
   # basic test that trackpoints at least returns some sort of flash movie
   def test_trackpoints
+    create(:trace, :visibility => "trackable", :latitude => 51.51, :longitude => -0.14, :user => users(:public_user)) do |trace|
+      create(:tracepoint, :trace => trace, :trackid => 1, :latitude => (51.510 * GeoRecord::SCALE).to_i, :longitude => (-0.140 * GeoRecord::SCALE).to_i)
+      create(:tracepoint, :trace => trace, :trackid => 2, :latitude => (51.511 * GeoRecord::SCALE).to_i, :longitude => (-0.141 * GeoRecord::SCALE).to_i)
+    end
+    create(:trace, :visibility => "identifiable", :latitude => 51.512, :longitude => 0.142) do |trace|
+      create(:tracepoint, :trace => trace, :latitude => (51.512 * GeoRecord::SCALE).to_i, :longitude => (0.142 * GeoRecord::SCALE).to_i)
+    end
+
     get :trackpoints, :xmin => -1, :xmax => 1, :ymin => 51, :ymax => 52, :baselong => 0, :basey => 0, :masterscale => 1
     assert_response :success
     assert_equal "application/x-shockwave-flash", response.content_type
diff --git a/test/controllers/trace_controller_test.rb b/test/controllers/trace_controller_test.rb
index 5a5234cb5..9170d6e2e 100644
--- a/test/controllers/trace_controller_test.rb
+++ b/test/controllers/trace_controller_test.rb
@@ -1,8 +1,8 @@
 require "test_helper"
+require "minitest/mock"
 
 class TraceControllerTest < ActionController::TestCase
-  fixtures :users, :gpx_files
-  set_fixture_class :gpx_files => Trace
+  fixtures :users
 
   def setup
     @gpx_trace_dir = Object.send("remove_const", "GPX_TRACE_DIR")
@@ -249,97 +249,119 @@ class TraceControllerTest < ActionController::TestCase
 
   # Test viewing a trace
   def test_view
+    public_trace_file = create(:trace, :visibility => "public")
+
     # First with no auth, which should work since the trace is public
-    get :view, :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id
-    check_trace_view gpx_files(:public_trace_file)
+    get :view, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
+    check_trace_view public_trace_file
 
     # Now with some other user, which should work since the trace is public
-    get :view, { :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id }, { :user => users(:public_user).id }
-    check_trace_view gpx_files(:public_trace_file)
+    get :view, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:public_user).id }
+    check_trace_view public_trace_file
 
     # And finally we should be able to do it with the owner of the trace
-    get :view, { :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id }, { :user => users(:normal_user).id }
-    check_trace_view gpx_files(:public_trace_file)
+    get :view, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:normal_user).id }
+    check_trace_view public_trace_file
   end
 
   # Check an anonymous trace can't be viewed by another user
   def test_view_anon
+    anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
+
     # First with no auth
-    get :view, :display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id
+    get :view, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
     assert_response :redirect
     assert_redirected_to :action => :list
 
-    # Now with some other user, which should work since the trace is anon
-    get :view, { :display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id }, { :user => users(:normal_user).id }
+    # Now with some other user, which should not work since the trace is anon
+    get :view, { :display_name => users(:public_user).display_name, :id => anon_trace_file.id }, { :user => users(:normal_user).id }
     assert_response :redirect
     assert_redirected_to :action => :list
 
     # And finally we should be able to do it with the owner of the trace
-    get :view, { :display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id }, { :user => users(:public_user).id }
-    check_trace_view gpx_files(:anon_trace_file)
+    get :view, { :display_name => users(:public_user).display_name, :id => anon_trace_file.id }, { :user => users(:public_user).id }
+    check_trace_view anon_trace_file
   end
 
   # Test viewing a trace that doesn't exist
   def test_view_not_found
-    # First with no auth, which should work since the trace is public
+    # First with no auth
     get :view, :display_name => users(:public_user).display_name, :id => 0
     assert_response :redirect
     assert_redirected_to :action => :list
 
-    # Now with some other user, which should work since the trace is public
+    # Now with some other user
     get :view, { :display_name => users(:public_user).display_name, :id => 0 }, { :user => users(:public_user).id }
     assert_response :redirect
     assert_redirected_to :action => :list
 
-    # And finally we should be able to do it with the owner of the trace
-    get :view, { :display_name => users(:public_user).display_name, :id => 5 }, { :user => users(:public_user).id }
+    # And finally we should not be able to view a deleted trace
+    deleted_trace_file = create(:trace, :deleted)
+    get :view, { :display_name => users(:public_user).display_name, :id => deleted_trace_file.id }, { :user => users(:public_user).id }
     assert_response :redirect
     assert_redirected_to :action => :list
   end
 
   # Test downloading a trace
   def test_data
-    # First with no auth, which should work since the trace is public
-    get :data, :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id
-    check_trace_data gpx_files(:public_trace_file)
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+    # We need to stub both the trace_name, to control which file from test/traces is used,
+    # and also the Trace.find method so that our stubbed object is used by the controller.
+    public_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/1.gpx" do
+      Trace.stub :find, public_trace_file do
+        # First with no auth, which should work since the trace is public
+        get :data, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
+        check_trace_data public_trace_file
 
-    # Now with some other user, which should work since the trace is public
-    get :data, { :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id }, { :user => users(:public_user).id }
-    check_trace_data gpx_files(:public_trace_file)
+        # Now with some other user, which should work since the trace is public
+        get :data, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:public_user).id }
+        check_trace_data public_trace_file
 
-    # And finally we should be able to do it with the owner of the trace
-    get :data, { :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id }, { :user => users(:normal_user).id }
-    check_trace_data gpx_files(:public_trace_file)
+        # And finally we should be able to do it with the owner of the trace
+        get :data, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:normal_user).id }
+        check_trace_data public_trace_file
+      end
+    end
   end
 
   # Test downloading a compressed trace
   def test_data_compressed
-    # First get the data as is
-    get :data, :display_name => users(:public_user).display_name, :id => gpx_files(:identifiable_trace_file).id
-    check_trace_data gpx_files(:identifiable_trace_file), "application/x-gzip", "gpx.gz"
+    identifiable_trace_file = create(:trace, :visibility => "identifiable")
+    identifiable_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/4.gpx" do
+      Trace.stub :find, identifiable_trace_file do
+        # First get the data as is
+        get :data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id
+        check_trace_data identifiable_trace_file, "application/x-gzip", "gpx.gz"
 
-    # Now ask explicitly for XML format
-    get :data, :display_name => users(:public_user).display_name, :id => gpx_files(:identifiable_trace_file).id, :format => "xml"
-    check_trace_data gpx_files(:identifiable_trace_file), "application/xml", "xml"
+        # Now ask explicitly for XML format
+        get :data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id, :format => "xml"
+        check_trace_data identifiable_trace_file, "application/xml", "xml"
 
-    # Now ask explicitly for GPX format
-    get :data, :display_name => users(:public_user).display_name, :id => gpx_files(:identifiable_trace_file).id, :format => "gpx"
-    check_trace_data gpx_files(:identifiable_trace_file)
+        # Now ask explicitly for GPX format
+        get :data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id, :format => "gpx"
+        check_trace_data identifiable_trace_file
+      end
+    end
   end
 
   # Check an anonymous trace can't be downloaded by another user
   def test_data_anon
-    # First with no auth
-    get :data, :display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id
-    assert_response :not_found
+    anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
+    anon_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/2.gpx" do
+      # First with no auth
+      get :data, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
+      assert_response :not_found
 
-    # Now with some other user, which shouldn't work since the trace is anon
-    get :data, { :display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id }, { :user => users(:normal_user).id }
-    assert_response :not_found
+      # Now with some other user, which shouldn't work since the trace is anon
+      get :data, { :display_name => users(:public_user).display_name, :id => anon_trace_file.id }, { :user => users(:normal_user).id }
+      assert_response :not_found
 
-    # And finally we should be able to do it with the owner of the trace
-    get :data, { :display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id }, { :user => users(:public_user).id }
-    check_trace_data gpx_files(:anon_trace_file)
+      # And finally we should be able to do it with the owner of the trace
+      Trace.stub :find, anon_trace_file do
+        get :data, { :display_name => users(:public_user).display_name, :id => anon_trace_file.id }, { :user => users(:public_user).id }
+        check_trace_data anon_trace_file
+      end
+    end
   end
 
   # Test downloading a trace that doesn't exist
@@ -353,38 +375,49 @@ class TraceControllerTest < ActionController::TestCase
     assert_response :not_found
 
     # Now with a trace that has been deleted
-    get :data, { :display_name => users(:public_user).display_name, :id => 5 }, { :user => users(:public_user).id }
+    deleted_trace_file = create(:trace, :deleted)
+    get :data, { :display_name => users(:public_user).display_name, :id => deleted_trace_file.id }, { :user => users(:public_user).id }
     assert_response :not_found
   end
 
   # Test downloading the picture for a trace
   def test_picture
-    # First with no auth, which should work since the trace is public
-    get :picture, :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id
-    check_trace_picture gpx_files(:public_trace_file)
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+    public_trace_file.stub :large_picture_name, "#{GPX_TRACE_DIR}/1.gif" do
+      Trace.stub :find, public_trace_file do
+        # First with no auth, which should work since the trace is public
+        get :picture, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
+        check_trace_picture public_trace_file
 
-    # Now with some other user, which should work since the trace is public
-    get :picture, { :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id }, { :user => users(:public_user).id }
-    check_trace_picture gpx_files(:public_trace_file)
+        # Now with some other user, which should work since the trace is public
+        get :picture, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:public_user).id }
+        check_trace_picture public_trace_file
 
-    # And finally we should be able to do it with the owner of the trace
-    get :picture, { :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id }, { :user => users(:normal_user).id }
-    check_trace_picture gpx_files(:public_trace_file)
+        # And finally we should be able to do it with the owner of the trace
+        get :picture, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:normal_user).id }
+        check_trace_picture public_trace_file
+      end
+    end
   end
 
   # Check the picture for an anonymous trace can't be downloaded by another user
   def test_picture_anon
-    # First with no auth
-    get :picture, :display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id
-    assert_response :forbidden
+    anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
+    anon_trace_file.stub :large_picture_name, "#{GPX_TRACE_DIR}/2.gif" do
+      # First with no auth
+      get :picture, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
+      assert_response :forbidden
 
-    # Now with some other user, which shouldn't work since the trace is anon
-    get :picture, { :display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id }, { :user => users(:normal_user).id }
-    assert_response :forbidden
+      # Now with some other user, which shouldn't work since the trace is anon
+      get :picture, { :display_name => users(:public_user).display_name, :id => anon_trace_file.id }, { :user => users(:normal_user).id }
+      assert_response :forbidden
 
-    # And finally we should be able to do it with the owner of the trace
-    get :picture, { :display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id }, { :user => users(:public_user).id }
-    check_trace_picture gpx_files(:anon_trace_file)
+      # And finally we should be able to do it with the owner of the trace
+      Trace.stub :find, anon_trace_file do
+        get :picture, { :display_name => users(:public_user).display_name, :id => anon_trace_file.id }, { :user => users(:public_user).id }
+        check_trace_picture anon_trace_file
+      end
+    end
   end
 
   # Test downloading the picture for a trace that doesn't exist
@@ -397,53 +430,65 @@ class TraceControllerTest < ActionController::TestCase
     get :picture, { :display_name => users(:public_user).display_name, :id => 0 }, { :user => users(:public_user).id }
     assert_response :not_found
 
-    # And finally we should be able to do it with the owner of the trace
-    get :picture, { :display_name => users(:public_user).display_name, :id => 5 }, { :user => users(:public_user).id }
+    # And finally we should not be able to do it with a deleted trace
+    deleted_trace_file = create(:trace, :deleted)
+    get :picture, { :display_name => users(:public_user).display_name, :id => deleted_trace_file.id }, { :user => users(:public_user).id }
     assert_response :not_found
   end
 
   # Test downloading the icon for a trace
   def test_icon
-    # First with no auth, which should work since the trace is public
-    get :icon, :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id
-    check_trace_icon gpx_files(:public_trace_file)
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+    public_trace_file.stub :icon_picture_name, "#{GPX_TRACE_DIR}/1_icon.gif" do
+      Trace.stub :find, public_trace_file do
+        # First with no auth, which should work since the trace is public
+        get :icon, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
+        check_trace_icon public_trace_file
 
-    # Now with some other user, which should work since the trace is public
-    get :icon, { :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id }, { :user => users(:public_user).id }
-    check_trace_icon gpx_files(:public_trace_file)
+        # Now with some other user, which should work since the trace is public
+        get :icon, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:public_user).id }
+        check_trace_icon public_trace_file
 
-    # And finally we should be able to do it with the owner of the trace
-    get :icon, { :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id }, { :user => users(:normal_user).id }
-    check_trace_icon gpx_files(:public_trace_file)
+        # And finally we should be able to do it with the owner of the trace
+        get :icon, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:normal_user).id }
+        check_trace_icon public_trace_file
+      end
+    end
   end
 
   # Check the icon for an anonymous trace can't be downloaded by another user
   def test_icon_anon
-    # First with no auth
-    get :icon, :display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id
-    assert_response :forbidden
+    anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
+    anon_trace_file.stub :icon_picture_name, "#{GPX_TRACE_DIR}/2_icon.gif" do
+      # First with no auth
+      get :icon, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
+      assert_response :forbidden
 
-    # Now with some other user, which shouldn't work since the trace is anon
-    get :icon, { :display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id }, { :user => users(:normal_user).id }
-    assert_response :forbidden
+      # Now with some other user, which shouldn't work since the trace is anon
+      get :icon, { :display_name => users(:public_user).display_name, :id => anon_trace_file.id }, { :user => users(:normal_user).id }
+      assert_response :forbidden
 
-    # And finally we should be able to do it with the owner of the trace
-    get :icon, { :display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id }, { :user => users(:public_user).id }
-    check_trace_icon gpx_files(:anon_trace_file)
+      # And finally we should be able to do it with the owner of the trace
+      Trace.stub :find, anon_trace_file do
+        get :icon, { :display_name => users(:public_user).display_name, :id => anon_trace_file.id }, { :user => users(:public_user).id }
+        check_trace_icon anon_trace_file
+      end
+    end
   end
 
   # Test downloading the icon for a trace that doesn't exist
   def test_icon_not_found
-    # First with no auth, which should work since the trace is public
+    # First with no auth
     get :icon, :display_name => users(:public_user).display_name, :id => 0
     assert_response :not_found
 
-    # Now with some other user, which should work since the trace is public
+    # Now with some other user
     get :icon, { :display_name => users(:public_user).display_name, :id => 0 }, { :user => users(:public_user).id }
     assert_response :not_found
 
-    # And finally we should be able to do it with the owner of the trace
-    get :icon, { :display_name => users(:public_user).display_name, :id => 5 }, { :user => users(:public_user).id }
+    # And finally we should not be able to do it with a deleted trace
+    deleted_trace_file = create(:trace, :deleted)
+    get :icon, { :display_name => users(:public_user).display_name, :id => deleted_trace_file.id }, { :user => users(:public_user).id }
     assert_response :not_found
   end
 
@@ -477,40 +522,46 @@ class TraceControllerTest < ActionController::TestCase
 
   # Test creating a trace
   def test_create_post
-    # Get file to use
-    file = Rack::Test::UploadedFile.new(gpx_files(:public_trace_file).trace_name, "application/gpx+xml")
+    public_trace_file = create(:trace, :visibility => "public")
+    public_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/1.gpx" do
+      # Get file to use
+      file = Rack::Test::UploadedFile.new(public_trace_file.trace_name, "application/gpx+xml")
 
-    # First with no auth
-    post :create, :trace => { :gpx_file => file, :description => "New Trace", :tagstring => "new,trace", :visibility => "trackable" }
-    assert_response :forbidden
+      # First with no auth
+      post :create, :trace => { :gpx_file => file, :description => "New Trace", :tagstring => "new,trace", :visibility => "trackable" }
+      assert_response :forbidden
 
-    # Now authenticated
-    create(:user_preference, :user => users(:public_user), :k => "gps.trace.visibility", :v => "identifiable")
-    assert_not_equal "trackable", users(:public_user).preferences.where(:k => "gps.trace.visibility").first.v
-    post :create, { :trace => { :gpx_file => file, :description => "New Trace", :tagstring => "new,trace", :visibility => "trackable" } }, { :user => users(:public_user).id }
-    assert_response :redirect
-    assert_redirected_to :action => :list, :display_name => users(:public_user).display_name
-    assert_match /file has been uploaded/, flash[:notice]
-    trace = Trace.order(:id => :desc).first
-    assert_equal "1.gpx", trace.name
-    assert_equal "New Trace", trace.description
-    assert_equal %w(new trace), trace.tags.order(:tag).collect(&:tag)
-    assert_equal "trackable", trace.visibility
-    assert_equal false, trace.inserted
-    assert_equal File.new(gpx_files(:public_trace_file).trace_name).read, File.new(trace.trace_name).read
-    trace.destroy
-    assert_equal "trackable", users(:public_user).preferences.where(:k => "gps.trace.visibility").first.v
+      # Now authenticated
+      create(:user_preference, :user => users(:public_user), :k => "gps.trace.visibility", :v => "identifiable")
+      assert_not_equal "trackable", users(:public_user).preferences.where(:k => "gps.trace.visibility").first.v
+      post :create, { :trace => { :gpx_file => file, :description => "New Trace", :tagstring => "new,trace", :visibility => "trackable" } }, { :user => users(:public_user).id }
+      assert_response :redirect
+      assert_redirected_to :action => :list, :display_name => users(:public_user).display_name
+      assert_match /file has been uploaded/, flash[:notice]
+      trace = Trace.order(:id => :desc).first
+      assert_equal "1.gpx", trace.name
+      assert_equal "New Trace", trace.description
+      assert_equal %w(new trace), trace.tags.order(:tag).collect(&:tag)
+      assert_equal "trackable", trace.visibility
+      assert_equal false, trace.inserted
+      assert_equal File.new(public_trace_file.trace_name).read, File.new(trace.trace_name).read
+      trace.destroy
+      assert_equal "trackable", users(:public_user).preferences.where(:k => "gps.trace.visibility").first.v
+    end
   end
 
   # Test fetching the edit page for a trace
   def test_edit_get
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+    deleted_trace_file = create(:trace, :deleted, :user => users(:public_user))
+
     # First with no auth
-    get :edit, :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id
+    get :edit, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
     assert_response :redirect
-    assert_redirected_to :controller => :user, :action => :login, :referer => trace_edit_path(:display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id)
+    assert_redirected_to :controller => :user, :action => :login, :referer => trace_edit_path(:display_name => users(:normal_user).display_name, :id => public_trace_file.id)
 
     # Now with some other user, which should fail
-    get :edit, { :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id }, { :user => users(:public_user).id }
+    get :edit, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:public_user).id }
     assert_response :forbidden
 
     # Now with a trace which doesn't exist
@@ -518,25 +569,27 @@ class TraceControllerTest < ActionController::TestCase
     assert_response :not_found
 
     # Now with a trace which has been deleted
-    get :edit, { :display_name => users(:public_user).display_name, :id => gpx_files(:deleted_trace_file).id }, { :user => users(:public_user).id }
+    get :edit, { :display_name => users(:public_user).display_name, :id => deleted_trace_file.id }, { :user => users(:public_user).id }
     assert_response :not_found
 
     # Finally with a trace that we are allowed to edit
-    get :edit, { :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id }, { :user => users(:normal_user).id }
+    get :edit, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:normal_user).id }
     assert_response :success
   end
 
   # Test saving edits to a trace
   def test_edit_post
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+    deleted_trace_file = create(:trace, :deleted, :user => users(:public_user))
     # New details
     new_details = { :description => "Changed description", :tagstring => "new_tag", :visibility => "private" }
 
     # First with no auth
-    post :edit, :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id, :trace => new_details
+    post :edit, :display_name => users(:normal_user).display_name, :id => public_trace_file.id, :trace => new_details
     assert_response :forbidden
 
     # Now with some other user, which should fail
-    post :edit, { :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id, :trace => new_details }, { :user => users(:public_user).id }
+    post :edit, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id, :trace => new_details }, { :user => users(:public_user).id }
     assert_response :forbidden
 
     # Now with a trace which doesn't exist
@@ -544,14 +597,14 @@ class TraceControllerTest < ActionController::TestCase
     assert_response :not_found
 
     # Now with a trace which has been deleted
-    post :edit, { :display_name => users(:public_user).display_name, :id => gpx_files(:deleted_trace_file).id, :trace => new_details }, { :user => users(:public_user).id }
+    post :edit, { :display_name => users(:public_user).display_name, :id => deleted_trace_file.id, :trace => new_details }, { :user => users(:public_user).id }
     assert_response :not_found
 
     # Finally with a trace that we are allowed to edit
-    post :edit, { :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id, :trace => new_details }, { :user => users(:normal_user).id }
+    post :edit, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id, :trace => new_details }, { :user => users(:normal_user).id }
     assert_response :redirect
     assert_redirected_to :action => :view, :display_name => users(:normal_user).display_name
-    trace = Trace.find(gpx_files(:public_trace_file).id)
+    trace = Trace.find(public_trace_file.id)
     assert_equal new_details[:description], trace.description
     assert_equal new_details[:tagstring], trace.tagstring
     assert_equal new_details[:visibility], trace.visibility
@@ -559,12 +612,15 @@ class TraceControllerTest < ActionController::TestCase
 
   # Test deleting a trace
   def test_delete
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+    deleted_trace_file = create(:trace, :deleted, :user => users(:public_user))
+
     # First with no auth
-    post :delete, :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id
+    post :delete, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
     assert_response :forbidden
 
     # Now with some other user, which should fail
-    post :delete, { :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id }, { :user => users(:public_user).id }
+    post :delete, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:public_user).id }
     assert_response :forbidden
 
     # Now with a trace which doesn't exist
@@ -572,54 +628,60 @@ class TraceControllerTest < ActionController::TestCase
     assert_response :not_found
 
     # Now with a trace has already been deleted
-    post :delete, { :display_name => users(:public_user).display_name, :id => gpx_files(:deleted_trace_file).id }, { :user => users(:public_user).id }
+    post :delete, { :display_name => users(:public_user).display_name, :id => deleted_trace_file.id }, { :user => users(:public_user).id }
     assert_response :not_found
 
     # Finally with a trace that we are allowed to delete
-    post :delete, { :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id }, { :user => users(:normal_user).id }
+    post :delete, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:normal_user).id }
     assert_response :redirect
     assert_redirected_to :action => :list, :display_name => users(:normal_user).display_name
-    trace = Trace.find(gpx_files(:public_trace_file).id)
+    trace = Trace.find(public_trace_file.id)
     assert_equal false, trace.visible
   end
 
   # Check getting a specific trace through the api
   def test_api_read
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+
     # First with no auth
-    get :api_read, :id => gpx_files(:public_trace_file).id
+    get :api_read, :id => public_trace_file.id
     assert_response :unauthorized
 
     # Now with some other user, which should work since the trace is public
     basic_authorization(users(:public_user).display_name, "test")
-    get :api_read, :id => gpx_files(:public_trace_file).id
+    get :api_read, :id => public_trace_file.id
     assert_response :success
 
     # And finally we should be able to do it with the owner of the trace
     basic_authorization(users(:normal_user).display_name, "test")
-    get :api_read, :id => gpx_files(:public_trace_file).id
+    get :api_read, :id => public_trace_file.id
     assert_response :success
   end
 
   # Check an anoymous trace can't be specifically fetched by another user
   def test_api_read_anon
-    # Furst with no auth
-    get :api_read, :id => gpx_files(:anon_trace_file).id
+    anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
+
+    # First with no auth
+    get :api_read, :id => anon_trace_file.id
     assert_response :unauthorized
 
     # Now try with another user, which shouldn't work since the trace is anon
     basic_authorization(users(:normal_user).display_name, "test")
-    get :api_read, :id => gpx_files(:anon_trace_file).id
+    get :api_read, :id => anon_trace_file.id
     assert_response :forbidden
 
     # And finally we should be able to get the trace details with the trace owner
     basic_authorization(users(:public_user).display_name, "test")
-    get :api_read, :id => gpx_files(:anon_trace_file).id
+    get :api_read, :id => anon_trace_file.id
     assert_response :success
   end
 
   # Check the api details for a trace that doesn't exist
   def test_api_read_not_found
-    # Try first with no auth, as it should requure it
+    deleted_trace_file = create(:trace, :deleted, :user => users(:public_user))
+
+    # Try first with no auth, as it should require it
     get :api_read, :id => 0
     assert_response :unauthorized
 
@@ -630,60 +692,85 @@ class TraceControllerTest < ActionController::TestCase
 
     # Now try a trace which did exist but has been deleted
     basic_authorization(users(:public_user).display_name, "test")
-    get :api_read, :id => 5
+    get :api_read, :id => deleted_trace_file.id
     assert_response :not_found
   end
 
   # Test downloading a trace through the api
   def test_api_data
-    # First with no auth
-    get :api_data, :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id
-    assert_response :unauthorized
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+    public_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/1.gpx" do
+      visible = MiniTest::Mock.new
+      visible.expect :find, public_trace_file, [String]
+      visible.expect :find, public_trace_file, [String]
+      Trace.stub :visible, visible do
+        # First with no auth
+        get :api_data, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
+        assert_response :unauthorized
 
-    # Now with some other user, which should work since the trace is public
-    basic_authorization(users(:public_user).display_name, "test")
-    get :api_data, :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id
-    check_trace_data gpx_files(:public_trace_file)
+        # Now with some other user, which should work since the trace is public
+        basic_authorization(users(:public_user).display_name, "test")
+        get :api_data, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
+        check_trace_data public_trace_file
 
-    # And finally we should be able to do it with the owner of the trace
-    basic_authorization(users(:normal_user).display_name, "test")
-    get :api_data, :display_name => users(:normal_user).display_name, :id => gpx_files(:public_trace_file).id
-    check_trace_data gpx_files(:public_trace_file)
+        # # And finally we should be able to do it with the owner of the trace
+        basic_authorization(users(:normal_user).display_name, "test")
+        get :api_data, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
+        check_trace_data public_trace_file
+      end
+    end
   end
 
   # Test downloading a compressed trace through the api
   def test_api_data_compressed
-    # Authenticate as the owner of the trace we will be using
-    basic_authorization(users(:public_user).display_name, "test")
+    identifiable_trace_file = create(:trace, :visibility => "identifiable", :user => users(:public_user))
+    identifiable_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/4.gpx" do
+      visible = MiniTest::Mock.new
+      visible.expect :find, identifiable_trace_file, [String]
+      visible.expect :find, identifiable_trace_file, [String]
+      visible.expect :find, identifiable_trace_file, [String]
+      Trace.stub :visible, visible do
+        # Authenticate as the owner of the trace we will be using
+        basic_authorization(users(:public_user).display_name, "test")
 
-    # First get the data as is
-    get :api_data, :display_name => users(:public_user).display_name, :id => gpx_files(:identifiable_trace_file).id
-    check_trace_data gpx_files(:identifiable_trace_file), "application/x-gzip", "gpx.gz"
+        # First get the data as is
+        get :api_data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id
+        check_trace_data identifiable_trace_file, "application/x-gzip", "gpx.gz"
 
-    # Now ask explicitly for XML format
-    get :api_data, :display_name => users(:public_user).display_name, :id => gpx_files(:identifiable_trace_file).id, :format => "xml"
-    check_trace_data gpx_files(:identifiable_trace_file), "application/xml", "xml"
+        # Now ask explicitly for XML format
+        get :api_data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id, :format => "xml"
+        check_trace_data identifiable_trace_file, "application/xml", "xml"
 
-    # Now ask explicitly for GPX format
-    get :api_data, :display_name => users(:public_user).display_name, :id => gpx_files(:identifiable_trace_file).id, :format => "gpx"
-    check_trace_data gpx_files(:identifiable_trace_file)
+        # # Now ask explicitly for GPX format
+        get :api_data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id, :format => "gpx"
+        check_trace_data identifiable_trace_file
+      end
+    end
   end
 
   # Check an anonymous trace can't be downloaded by another user through the api
   def test_api_data_anon
-    # First with no auth
-    get :api_data, :display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id
-    assert_response :unauthorized
+    anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
+    anon_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/2.gpx" do
+      visible = MiniTest::Mock.new
+      visible.expect :find, anon_trace_file, [String]
+      visible.expect :find, anon_trace_file, [String]
+      Trace.stub :visible, visible do
+        # First with no auth
+        get :api_data, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
+        assert_response :unauthorized
 
-    # Now with some other user, which shouldn't work since the trace is anon
-    basic_authorization(users(:normal_user).display_name, "test")
-    get :api_data, :display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id
-    assert_response :forbidden
+        # Now with some other user, which shouldn't work since the trace is anon
+        basic_authorization(users(:normal_user).display_name, "test")
+        get :api_data, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
+        assert_response :forbidden
 
-    # And finally we should be able to do it with the owner of the trace
-    basic_authorization(users(:public_user).display_name, "test")
-    get :api_data, :display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id
-    check_trace_data gpx_files(:anon_trace_file)
+        # And finally we should be able to do it with the owner of the trace
+        basic_authorization(users(:public_user).display_name, "test")
+        get :api_data, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
+        check_trace_data anon_trace_file
+      end
+    end
   end
 
   # Test downloading a trace that doesn't exist through the api
@@ -698,127 +785,139 @@ class TraceControllerTest < ActionController::TestCase
     assert_response :not_found
 
     # Now with a trace that has been deleted
+    deleted_trace_file = create(:trace, :deleted)
     basic_authorization(users(:public_user).display_name, "test")
-    get :api_data, :display_name => users(:public_user).display_name, :id => 5
+    get :api_data, :display_name => users(:public_user).display_name, :id => deleted_trace_file.id
     assert_response :not_found
   end
 
   # Test creating a trace through the api
   def test_api_create
-    # Get file to use
-    file = Rack::Test::UploadedFile.new(gpx_files(:public_trace_file).trace_name, "application/gpx+xml")
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+    public_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/1.gpx" do
+      # Get file to use
+      file = Rack::Test::UploadedFile.new(public_trace_file.trace_name, "application/gpx+xml")
 
-    # First with no auth
-    post :api_create, :file => file, :description => "New Trace", :tags => "new,trace", :visibility => "trackable"
-    assert_response :unauthorized
+      # First with no auth
+      post :api_create, :file => file, :description => "New Trace", :tags => "new,trace", :visibility => "trackable"
+      assert_response :unauthorized
 
-    # Now authenticated
-    create(:user_preference, :user => users(:public_user), :k => "gps.trace.visibility", :v => "identifiable")
-    assert_not_equal "trackable", users(:public_user).preferences.where(:k => "gps.trace.visibility").first.v
-    basic_authorization(users(:public_user).display_name, "test")
-    post :api_create, :file => file, :description => "New Trace", :tags => "new,trace", :visibility => "trackable"
-    assert_response :success
-    trace = Trace.find(response.body.to_i)
-    assert_equal "1.gpx", trace.name
-    assert_equal "New Trace", trace.description
-    assert_equal %w(new trace), trace.tags.order(:tag).collect(&:tag)
-    assert_equal "trackable", trace.visibility
-    assert_equal false, trace.inserted
-    assert_equal File.new(gpx_files(:public_trace_file).trace_name).read, File.new(trace.trace_name).read
-    trace.destroy
-    assert_equal "trackable", users(:public_user).preferences.where(:k => "gps.trace.visibility").first.v
+      # Now authenticated
+      create(:user_preference, :user => users(:public_user), :k => "gps.trace.visibility", :v => "identifiable")
+      assert_not_equal "trackable", users(:public_user).preferences.where(:k => "gps.trace.visibility").first.v
+      basic_authorization(users(:public_user).display_name, "test")
+      post :api_create, :file => file, :description => "New Trace", :tags => "new,trace", :visibility => "trackable"
+      assert_response :success
+      trace = Trace.find(response.body.to_i)
+      assert_equal "1.gpx", trace.name
+      assert_equal "New Trace", trace.description
+      assert_equal %w(new trace), trace.tags.order(:tag).collect(&:tag)
+      assert_equal "trackable", trace.visibility
+      assert_equal false, trace.inserted
+      assert_equal File.new(public_trace_file.trace_name).read, File.new(trace.trace_name).read
+      trace.destroy
+      assert_equal "trackable", users(:public_user).preferences.where(:k => "gps.trace.visibility").first.v
 
-    # Rewind the file
-    file.rewind
+      # Rewind the file
+      file.rewind
 
-    # Now authenticated, with the legacy public flag
-    assert_not_equal "public", users(:public_user).preferences.where(:k => "gps.trace.visibility").first.v
-    basic_authorization(users(:public_user).display_name, "test")
-    post :api_create, :file => file, :description => "New Trace", :tags => "new,trace", :public => 1
-    assert_response :success
-    trace = Trace.find(response.body.to_i)
-    assert_equal "1.gpx", trace.name
-    assert_equal "New Trace", trace.description
-    assert_equal %w(new trace), trace.tags.order(:tag).collect(&:tag)
-    assert_equal "public", trace.visibility
-    assert_equal false, trace.inserted
-    assert_equal File.new(gpx_files(:public_trace_file).trace_name).read, File.new(trace.trace_name).read
-    trace.destroy
-    assert_equal "public", users(:public_user).preferences.where(:k => "gps.trace.visibility").first.v
+      # Now authenticated, with the legacy public flag
+      assert_not_equal "public", users(:public_user).preferences.where(:k => "gps.trace.visibility").first.v
+      basic_authorization(users(:public_user).display_name, "test")
+      post :api_create, :file => file, :description => "New Trace", :tags => "new,trace", :public => 1
+      assert_response :success
+      trace = Trace.find(response.body.to_i)
+      assert_equal "1.gpx", trace.name
+      assert_equal "New Trace", trace.description
+      assert_equal %w(new trace), trace.tags.order(:tag).collect(&:tag)
+      assert_equal "public", trace.visibility
+      assert_equal false, trace.inserted
+      assert_equal File.new(public_trace_file.trace_name).read, File.new(trace.trace_name).read
+      trace.destroy
+      assert_equal "public", users(:public_user).preferences.where(:k => "gps.trace.visibility").first.v
 
-    # Rewind the file
-    file.rewind
+      # Rewind the file
+      file.rewind
 
-    # Now authenticated, with the legacy private flag
-    assert_nil users(:second_public_user).preferences.where(:k => "gps.trace.visibility").first
-    basic_authorization(users(:second_public_user).display_name, "test")
-    post :api_create, :file => file, :description => "New Trace", :tags => "new,trace", :public => 0
-    assert_response :success
-    trace = Trace.find(response.body.to_i)
-    assert_equal "1.gpx", trace.name
-    assert_equal "New Trace", trace.description
-    assert_equal %w(new trace), trace.tags.order(:tag).collect(&:tag)
-    assert_equal "private", trace.visibility
-    assert_equal false, trace.inserted
-    assert_equal File.new(gpx_files(:public_trace_file).trace_name).read, File.new(trace.trace_name).read
-    trace.destroy
-    assert_equal "private", users(:second_public_user).preferences.where(:k => "gps.trace.visibility").first.v
+      # Now authenticated, with the legacy private flag
+      assert_nil users(:second_public_user).preferences.where(:k => "gps.trace.visibility").first
+      basic_authorization(users(:second_public_user).display_name, "test")
+      post :api_create, :file => file, :description => "New Trace", :tags => "new,trace", :public => 0
+      assert_response :success
+      trace = Trace.find(response.body.to_i)
+      assert_equal "1.gpx", trace.name
+      assert_equal "New Trace", trace.description
+      assert_equal %w(new trace), trace.tags.order(:tag).collect(&:tag)
+      assert_equal "private", trace.visibility
+      assert_equal false, trace.inserted
+      assert_equal File.new(public_trace_file.trace_name).read, File.new(trace.trace_name).read
+      trace.destroy
+      assert_equal "private", users(:second_public_user).preferences.where(:k => "gps.trace.visibility").first.v
+    end
   end
 
   # Check updating a trace through the api
   def test_api_update
-    # First with no auth
-    content gpx_files(:public_trace_file).to_xml
-    put :api_update, :id => gpx_files(:public_trace_file).id
-    assert_response :unauthorized
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+    deleted_trace_file = create(:trace, :deleted, :user => users(:public_user))
+    anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
 
-    # Now with some other user, which should fail
-    basic_authorization(users(:public_user).display_name, "test")
-    content gpx_files(:public_trace_file).to_xml
-    put :api_update, :id => gpx_files(:public_trace_file).id
-    assert_response :forbidden
+    public_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/1.gpx" do
+      # First with no auth
+      content public_trace_file.to_xml
+      put :api_update, :id => public_trace_file.id
+      assert_response :unauthorized
 
-    # Now with a trace which doesn't exist
-    basic_authorization(users(:public_user).display_name, "test")
-    content gpx_files(:public_trace_file).to_xml
-    put :api_update, :id => 0
-    assert_response :not_found
+      # Now with some other user, which should fail
+      basic_authorization(users(:public_user).display_name, "test")
+      content public_trace_file.to_xml
+      put :api_update, :id => public_trace_file.id
+      assert_response :forbidden
 
-    # Now with a trace which did exist but has been deleted
-    basic_authorization(users(:public_user).display_name, "test")
-    content gpx_files(:deleted_trace_file).to_xml
-    put :api_update, :id => gpx_files(:deleted_trace_file).id
-    assert_response :not_found
+      # Now with a trace which doesn't exist
+      basic_authorization(users(:public_user).display_name, "test")
+      content public_trace_file.to_xml
+      put :api_update, :id => 0
+      assert_response :not_found
 
-    # Now try an update with the wrong ID
-    basic_authorization(users(:normal_user).display_name, "test")
-    content gpx_files(:anon_trace_file).to_xml
-    put :api_update, :id => gpx_files(:public_trace_file).id
-    assert_response :bad_request,
-                    "should not be able to update a trace with a different ID from the XML"
+      # Now with a trace which did exist but has been deleted
+      basic_authorization(users(:public_user).display_name, "test")
+      content deleted_trace_file.to_xml
+      put :api_update, :id => deleted_trace_file.id
+      assert_response :not_found
 
-    # And finally try an update that should work
-    basic_authorization(users(:normal_user).display_name, "test")
-    t = gpx_files(:public_trace_file)
-    t.description = "Changed description"
-    t.visibility = "private"
-    content t.to_xml
-    put :api_update, :id => t.id
-    assert_response :success
-    nt = Trace.find(t.id)
-    assert_equal nt.description, t.description
-    assert_equal nt.visibility, t.visibility
+      # Now try an update with the wrong ID
+      basic_authorization(users(:normal_user).display_name, "test")
+      content anon_trace_file.to_xml
+      put :api_update, :id => public_trace_file.id
+      assert_response :bad_request,
+                      "should not be able to update a trace with a different ID from the XML"
+
+      # And finally try an update that should work
+      basic_authorization(users(:normal_user).display_name, "test")
+      t = public_trace_file
+      t.description = "Changed description"
+      t.visibility = "private"
+      content t.to_xml
+      put :api_update, :id => t.id
+      assert_response :success
+      nt = Trace.find(t.id)
+      assert_equal nt.description, t.description
+      assert_equal nt.visibility, t.visibility
+    end
   end
 
   # Check deleting a trace through the api
   def test_api_delete
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+
     # First with no auth
-    delete :api_delete, :id => gpx_files(:public_trace_file).id
+    delete :api_delete, :id => public_trace_file.id
     assert_response :unauthorized
 
     # Now with some other user, which should fail
     basic_authorization(users(:public_user).display_name, "test")
-    delete :api_delete, :id => gpx_files(:public_trace_file).id
+    delete :api_delete, :id => public_trace_file.id
     assert_response :forbidden
 
     # Now with a trace which doesn't exist
@@ -828,12 +927,12 @@ class TraceControllerTest < ActionController::TestCase
 
     # And finally we should be able to do it with the owner of the trace
     basic_authorization(users(:normal_user).display_name, "test")
-    delete :api_delete, :id => gpx_files(:public_trace_file).id
+    delete :api_delete, :id => public_trace_file.id
     assert_response :success
 
     # Try it a second time, which should fail
     basic_authorization(users(:normal_user).display_name, "test")
-    delete :api_delete, :id => gpx_files(:public_trace_file).id
+    delete :api_delete, :id => public_trace_file.id
     assert_response :not_found
   end
 
diff --git a/test/controllers/user_controller_test.rb b/test/controllers/user_controller_test.rb
index 409a93b4e..717089b64 100644
--- a/test/controllers/user_controller_test.rb
+++ b/test/controllers/user_controller_test.rb
@@ -1085,6 +1085,12 @@ class UserControllerTest < ActionController::TestCase
   end
 
   def test_api_gpx_files
+    trace1 = create(:trace, :user => users(:normal_user)) do |trace|
+      create(:tracetag, :trace => trace, :tag => "London")
+    end
+    trace2 = create(:trace, :user => users(:normal_user)) do |trace|
+      create(:tracetag, :trace => trace, :tag => "Birmingham")
+    end
     # check that nothing is returned when not logged in
     get :api_gpx_files
     assert_response :unauthorized
@@ -1096,10 +1102,10 @@ class UserControllerTest < ActionController::TestCase
     assert_equal "text/xml", response.content_type
 
     # check the data that is returned
-    assert_select "gpx_file[id='1']", 1 do
+    assert_select "gpx_file[id='#{trace1.id}']", 1 do
       assert_select "tag", "London"
     end
-    assert_select "gpx_file[id='4']", 1 do
+    assert_select "gpx_file[id='#{trace2.id}']", 1 do
       assert_select "tag", "Birmingham"
     end
   end
diff --git a/test/factories/tracepoints.rb b/test/factories/tracepoints.rb
new file mode 100644
index 000000000..c47b7932d
--- /dev/null
+++ b/test/factories/tracepoints.rb
@@ -0,0 +1,11 @@
+FactoryGirl.define do
+  factory :tracepoint do
+    trackid 1
+    latitude 1 * GeoRecord::SCALE
+    longitude 1 * GeoRecord::SCALE
+    # tile QuadTile.tile_for_point(1,1)
+    timestamp Time.now
+
+    trace
+  end
+end
diff --git a/test/factories/traces.rb b/test/factories/traces.rb
new file mode 100644
index 000000000..3f409b367
--- /dev/null
+++ b/test/factories/traces.rb
@@ -0,0 +1,16 @@
+FactoryGirl.define do
+  factory :trace do
+    sequence(:name) { |n| "Trace #{n}.gpx" }
+    sequence(:description) { |n| "This is trace #{n}" }
+
+    # Fixme requires User Factory
+    user_id 1
+
+    timestamp Time.now
+    inserted true
+
+    trait :deleted do
+      visible false
+    end
+  end
+end
diff --git a/test/factories/tracetags.rb b/test/factories/tracetags.rb
new file mode 100644
index 000000000..3dbeeb877
--- /dev/null
+++ b/test/factories/tracetags.rb
@@ -0,0 +1,7 @@
+FactoryGirl.define do
+  factory :tracetag do
+    sequence(:tag) { |n| "Tag #{n}" }
+
+    trace
+  end
+end
diff --git a/test/fixtures/gps_points.yml b/test/fixtures/gps_points.yml
deleted file mode 100644
index b3171ac45..000000000
--- a/test/fixtures/gps_points.yml
+++ /dev/null
@@ -1,35 +0,0 @@
-<% SCALE = 10000000 unless defined?(SCALE) %>
-
-first_trace_1:
-  altitude: 134
-  trackid: 1
-  latitude: <%= 1 * SCALE %>
-  longitude: <%= 1 * SCALE %>
-  gpx_id: 1
-  timestamp: "2008-10-01 10:10:10"
-  tile: <%= QuadTile.tile_for_point(1, 1) %>
-  
-trackable_trace_1:
-  trackid: 1
-  latitude: <%= (51.510 * SCALE).to_i %>
-  longitude: <%= (-0.140 * SCALE).to_i %>
-  gpx_id: 3
-  timestamp: "2009-07-30 17:46:00"
-  tile: <%= QuadTile.tile_for_point(51.510, -0.140) %>
-
-trackable_trace_2:
-  trackid: 2
-  latitude: <%= (51.511 * SCALE).to_i %>
-  longitude: <%= (-0.141 * SCALE).to_i %>
-  gpx_id: 3
-  timestamp: "2009-07-30 17:47:00"
-  tile: <%= QuadTile.tile_for_point(51.511, -0.141) %>
-
-identifiable_trace_1:
-  trackid: 1
-  latitude: <%= (51.512 * SCALE).to_i %>
-  longitude: <%= (0.142 * SCALE).to_i %>
-  gpx_id: 4
-  timestamp: "2009-07-30 17:46:00"
-  tile: <%= QuadTile.tile_for_point(51.512, 0.142) %>
-
diff --git a/test/fixtures/gpx_file_tags.yml b/test/fixtures/gpx_file_tags.yml
deleted file mode 100644
index 1dac0f26d..000000000
--- a/test/fixtures/gpx_file_tags.yml
+++ /dev/null
@@ -1,19 +0,0 @@
-first_trace_1:
-  gpx_id: 1
-  tag: London
-  id: 1
-
-second_trace_1:
-  gpx_id: 2
-  tag: London
-  id: 2
-
-second_trace_2:
-  gpx_id: 2
-  tag: Birmingham
-  id: 3
-
-third_trace_1:
-  gpx_id: 4
-  tag: Birmingham
-  id: 4
diff --git a/test/fixtures/gpx_files.yml b/test/fixtures/gpx_files.yml
deleted file mode 100644
index 4af266cf2..000000000
--- a/test/fixtures/gpx_files.yml
+++ /dev/null
@@ -1,129 +0,0 @@
-public_trace_file:
-  id: 1
-  user_id: 1
-  visible: true
-  name: Fist Trace.gpx
-  size:
-  latitude: 1
-  longitude: 1
-  timestamp: "2008-10-29 10:10:10"
-  visibility: "public"
-  description: This is a trace
-  inserted: true
-
-anon_trace_file:
-  id: 2
-  user_id: 2
-  visible: true
-  name: Private Trace.gpx
-  size: 123
-  latitude: 51.3
-  longitude: -0.56
-  timestamp: "2009-05-06 13:34:34"
-  visibility: "private"
-  description: This is an anonymous trace
-  inserted: true
-
-trackable_trace_file:
-  id: 3
-  user_id: 2
-  visible: true
-  name: Trackable Trace.gpx
-  size: 123
-  latitude: 51.51
-  longitude: -0.14
-  timestamp: "2009-07-30 17:48:34"
-  visibility: "trackable"
-  description: This trace shows trksegs and timestamps, but no user details.
-  inserted: true
-
-identifiable_trace_file:
-  id: 4
-  user_id: 1
-  visible: true
-  name: Identifiable Trace.gpx
-  size: 123
-  latitude: 51.512
-  longitude: 0.142
-  timestamp: "2009-07-30 17:48:34"
-  visibility: "identifiable"
-  description: This trace shows trksegs, timestamps and user details.
-  inserted: true
-
-deleted_trace_file:
-  id: 5
-  user_id: 2
-  visible: false
-  name: Deleted Trace.gpx
-  size: 123
-  latitude: 51.512
-  longitude: 0.142
-  timestamp: "2009-07-30 17:48:34"
-  visibility: "public"
-  description: This is a trace that has been deleted.
-  inserted: true
-
-zipped_trace_file:
-  id: 6
-  user_id: 4
-  visible: true
-  name: Zipped Trace.gpx
-  size:
-  latitude: 1
-  longitude: 1
-  timestamp: "2008-10-29 10:10:10"
-  visibility: "private"
-  description: This is a zipped trace
-  inserted: true
-
-tar_trace_file:
-  id: 7
-  user_id: 4
-  visible: true
-  name: Tarred Trace.gpx
-  size:
-  latitude: 1
-  longitude: 1
-  timestamp: "2008-10-29 10:10:10"
-  visibility: "private"
-  description: This is a tarred trace
-  inserted: true
-
-tar_gzip_trace_file:
-  id: 8
-  user_id: 4
-  visible: true
-  name: Gzipped Tarred Trace.gpx
-  size:
-  latitude: 1
-  longitude: 1
-  timestamp: "2008-10-29 10:10:10"
-  visibility: "private"
-  description: This is a gzipped tarred trace
-  inserted: true
-
-tar_bzip_trace_file:
-  id: 9
-  user_id: 4
-  visible: true
-  name: Bzipped Tarred Trace.gpx
-  size:
-  latitude: 1
-  longitude: 1
-  timestamp: "2008-10-29 10:10:10"
-  visibility: "private"
-  description: This is a bzipped tarred trace
-  inserted: true
-
-pending_trace_file:
-  id: 10
-  user_id: 2
-  visible: true
-  name: Pending Trace.gpx
-  size: 123
-  latitude: 51.3
-  longitude: -0.56
-  timestamp: "2009-05-06 13:34:34"
-  visibility: "public"
-  description: This is a pending trace
-  inserted: false
diff --git a/test/integration/oauth_test.rb b/test/integration/oauth_test.rb
index 00a9ae06d..ad3d5383d 100644
--- a/test/integration/oauth_test.rb
+++ b/test/integration/oauth_test.rb
@@ -1,8 +1,7 @@
 require "test_helper"
 
 class OAuthTest < ActionDispatch::IntegrationTest
-  fixtures :users, :client_applications, :gpx_files
-  set_fixture_class :gpx_files => Trace
+  fixtures :users, :client_applications
 
   include OAuth::Helper
 
@@ -164,7 +163,8 @@ class OAuthTest < ActionDispatch::IntegrationTest
     assert_nil token.invalidated_at
     assert_allowed token, [:allow_write_api, :allow_read_gpx]
 
-    signed_get "/api/0.6/gpx/2", :consumer => client, :token => token
+    trace = create(:trace, :user => users(:public_user))
+    signed_get "/api/0.6/gpx/#{trace.id}", :consumer => client, :token => token
     assert_response :success
 
     signed_get "/api/0.6/user/details", :consumer => client, :token => token
@@ -226,7 +226,8 @@ class OAuthTest < ActionDispatch::IntegrationTest
     signed_get "/api/0.6/user/preferences", :consumer => client, :token => token
     assert_response :success
 
-    signed_get "/api/0.6/gpx/2", :consumer => client, :token => token
+    trace = create(:trace, :user => users(:public_user))
+    signed_get "/api/0.6/gpx/#{trace.id}", :consumer => client, :token => token
     assert_response :forbidden
 
     post "/oauth/revoke", :token => token.token
@@ -274,7 +275,8 @@ class OAuthTest < ActionDispatch::IntegrationTest
     assert_nil token.invalidated_at
     assert_allowed token, [:allow_write_api, :allow_read_gpx]
 
-    signed_get "/api/0.6/gpx/2", :consumer => client, :token => token
+    trace = create(:trace, :user => users(:public_user))
+    signed_get "/api/0.6/gpx/#{trace.id}", :consumer => client, :token => token
     assert_response :success
 
     signed_get "/api/0.6/user/details", :consumer => client, :token => token
diff --git a/test/models/trace_test.rb b/test/models/trace_test.rb
index 32d98c9eb..1cb94aeef 100644
--- a/test/models/trace_test.rb
+++ b/test/models/trace_test.rb
@@ -1,8 +1,9 @@
 require "test_helper"
 require "digest"
+require "minitest/mock"
 
 class TraceTest < ActiveSupport::TestCase
-  api_fixtures
+  fixtures :users
 
   def setup
     @gpx_trace_dir = Object.send("remove_const", "GPX_TRACE_DIR")
@@ -20,41 +21,55 @@ class TraceTest < ActiveSupport::TestCase
     Object.const_set("GPX_IMAGE_DIR", @gpx_image_dir)
   end
 
-  def test_trace_count
-    assert_equal 10, Trace.count
-  end
-
   def test_visible
-    check_query(Trace.visible, [
-                  :public_trace_file, :anon_trace_file, :trackable_trace_file,
-                  :identifiable_trace_file, :zipped_trace_file, :tar_trace_file,
-                  :tar_gzip_trace_file, :tar_bzip_trace_file, :pending_trace_file
-                ])
+    public_trace_file = create(:trace)
+    create(:trace, :deleted)
+    check_query(Trace.visible, [public_trace_file])
   end
 
   def test_visible_to
-    check_query(Trace.visible_to(1), [
-                  :public_trace_file, :identifiable_trace_file, :pending_trace_file
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+    anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
+    identifiable_trace_file = create(:trace, :visibility => "identifiable", :user => users(:normal_user))
+    pending_trace_file = create(:trace, :visibility => "public", :user => users(:public_user), :inserted => false)
+    trackable_trace_file = create(:trace, :visibility => "trackable", :user => users(:public_user))
+    _other_trace_file = create(:trace, :visibility => "private", :user => users(:second_public_user))
+
+    check_query(Trace.visible_to(users(:normal_user).id), [
+                  public_trace_file, identifiable_trace_file, pending_trace_file
                 ])
-    check_query(Trace.visible_to(2), [
-                  :public_trace_file, :anon_trace_file, :trackable_trace_file,
-                  :identifiable_trace_file, :pending_trace_file
+    check_query(Trace.visible_to(users(:public_user)), [
+                  public_trace_file, anon_trace_file, trackable_trace_file,
+                  identifiable_trace_file, pending_trace_file
                 ])
-    check_query(Trace.visible_to(3), [
-                  :public_trace_file, :identifiable_trace_file, :pending_trace_file
+    check_query(Trace.visible_to(users(:inactive_user)), [
+                  public_trace_file, identifiable_trace_file, pending_trace_file
                 ])
   end
 
   def test_visible_to_all
+    public_trace_file = create(:trace, :visibility => "public")
+    _private_trace_file = create(:trace, :visibility => "private")
+    identifiable_trace_file = create(:trace, :visibility => "identifiable")
+    _trackable_trace_file = create(:trace, :visibility => "trackable")
+    deleted_trace_file = create(:trace, :deleted, :visibility => "public")
+    pending_trace_file = create(:trace, :visibility => "public", :inserted => false)
+
     check_query(Trace.visible_to_all, [
-                  :public_trace_file, :identifiable_trace_file,
-                  :deleted_trace_file, :pending_trace_file
+                  public_trace_file, identifiable_trace_file,
+                  deleted_trace_file, pending_trace_file
                 ])
   end
 
   def test_tagged
-    check_query(Trace.tagged("London"), [:public_trace_file, :anon_trace_file])
-    check_query(Trace.tagged("Birmingham"), [:anon_trace_file, :identifiable_trace_file])
+    london_trace_file = create(:trace) do |trace|
+      create(:tracetag, :trace => trace, :tag => "London")
+    end
+    birmingham_trace_file = create(:trace) do |trace|
+      create(:tracetag, :trace => trace, :tag => "Birmingham")
+    end
+    check_query(Trace.tagged("London"), [london_trace_file])
+    check_query(Trace.tagged("Birmingham"), [birmingham_trace_file])
     check_query(Trace.tagged("Unknown"), [])
   end
 
@@ -74,7 +89,7 @@ class TraceTest < ActiveSupport::TestCase
   end
 
   def test_tagstring
-    trace = Trace.new(gpx_files(:public_trace_file).attributes)
+    trace = build(:trace)
     trace.tagstring = "foo bar baz"
     assert trace.valid?
     assert_equal 3, trace.tags.length
@@ -92,66 +107,71 @@ class TraceTest < ActiveSupport::TestCase
   end
 
   def test_public?
-    assert_equal true, gpx_files(:public_trace_file).public?
-    assert_equal false, gpx_files(:anon_trace_file).public?
-    assert_equal false, gpx_files(:trackable_trace_file).public?
-    assert_equal true, gpx_files(:identifiable_trace_file).public?
-    assert_equal true, gpx_files(:deleted_trace_file).public?
+    assert_equal true, build(:trace, :visibility => "public").public?
+    assert_equal false, build(:trace, :visibility => "private").public?
+    assert_equal false, build(:trace, :visibility => "trackable").public?
+    assert_equal true, build(:trace, :visibility => "identifiable").public?
+    assert_equal true, build(:trace, :deleted, :visibility => "public").public?
   end
 
   def test_trackable?
-    assert_equal false, gpx_files(:public_trace_file).trackable?
-    assert_equal false, gpx_files(:anon_trace_file).trackable?
-    assert_equal true, gpx_files(:trackable_trace_file).trackable?
-    assert_equal true, gpx_files(:identifiable_trace_file).trackable?
-    assert_equal false, gpx_files(:deleted_trace_file).trackable?
+    assert_equal false, build(:trace, :visibility => "public").trackable?
+    assert_equal false, build(:trace, :visibility => "private").trackable?
+    assert_equal true, build(:trace, :visibility => "trackable").trackable?
+    assert_equal true, build(:trace, :visibility => "identifiable").trackable?
+    assert_equal false, build(:trace, :deleted, :visibility => "public").trackable?
   end
 
   def test_identifiable?
-    assert_equal false, gpx_files(:public_trace_file).identifiable?
-    assert_equal false, gpx_files(:anon_trace_file).identifiable?
-    assert_equal false, gpx_files(:trackable_trace_file).identifiable?
-    assert_equal true, gpx_files(:identifiable_trace_file).identifiable?
-    assert_equal false, gpx_files(:deleted_trace_file).identifiable?
+    assert_equal false, build(:trace, :visibility => "public").identifiable?
+    assert_equal false, build(:trace, :visibility => "private").identifiable?
+    assert_equal false, build(:trace, :visibility => "trackable").identifiable?
+    assert_equal true, build(:trace, :visibility => "identifiable").identifiable?
+    assert_equal false, build(:trace, :deleted, :visibility => "public").identifiable?
   end
 
   def test_mime_type
-    assert_equal "application/gpx+xml", gpx_files(:public_trace_file).mime_type
-    assert_equal "application/gpx+xml", gpx_files(:anon_trace_file).mime_type
-    assert_equal "application/x-bzip2", gpx_files(:trackable_trace_file).mime_type
-    assert_equal "application/x-gzip", gpx_files(:identifiable_trace_file).mime_type
-    assert_equal "application/x-zip", gpx_files(:zipped_trace_file).mime_type
-    assert_equal "application/x-tar", gpx_files(:tar_trace_file).mime_type
-    assert_equal "application/x-gzip", gpx_files(:tar_gzip_trace_file).mime_type
-    assert_equal "application/x-bzip2", gpx_files(:tar_bzip_trace_file).mime_type
+    # The ids refer to the .gpx fixtures in test/traces
+    check_mime_type(1, "application/gpx+xml")
+    check_mime_type(2, "application/gpx+xml")
+    check_mime_type(3, "application/x-bzip2")
+    check_mime_type(4, "application/x-gzip")
+    check_mime_type(6, "application/x-zip")
+    check_mime_type(7, "application/x-tar")
+    check_mime_type(8, "application/x-gzip")
+    check_mime_type(9, "application/x-bzip2")
   end
 
   def test_extension_name
-    assert_equal ".gpx", gpx_files(:public_trace_file).extension_name
-    assert_equal ".gpx", gpx_files(:anon_trace_file).extension_name
-    assert_equal ".gpx.bz2", gpx_files(:trackable_trace_file).extension_name
-    assert_equal ".gpx.gz", gpx_files(:identifiable_trace_file).extension_name
-    assert_equal ".zip", gpx_files(:zipped_trace_file).extension_name
-    assert_equal ".tar", gpx_files(:tar_trace_file).extension_name
-    assert_equal ".tar.gz", gpx_files(:tar_gzip_trace_file).extension_name
-    assert_equal ".tar.bz2", gpx_files(:tar_bzip_trace_file).extension_name
+    # The ids refer to the .gpx fixtures in test/traces
+    check_extension_name(1, ".gpx")
+    check_extension_name(2, ".gpx")
+    check_extension_name(3, ".gpx.bz2")
+    check_extension_name(4, ".gpx.gz")
+    check_extension_name(6, ".zip")
+    check_extension_name(7, ".tar")
+    check_extension_name(8, ".tar.gz")
+    check_extension_name(9, ".tar.bz2")
   end
 
   def test_xml_file
-    assert_equal "848caa72f2f456d1bd6a0fdf228aa1b9", md5sum(gpx_files(:public_trace_file).xml_file)
-    assert_equal "66179ca44f1e93d8df62e2b88cbea732", md5sum(gpx_files(:anon_trace_file).xml_file)
-    assert_equal "848caa72f2f456d1bd6a0fdf228aa1b9", md5sum(gpx_files(:trackable_trace_file).xml_file)
-    assert_equal "abd6675fdf3024a84fc0a1deac147c0d", md5sum(gpx_files(:identifiable_trace_file).xml_file)
-    assert_equal "848caa72f2f456d1bd6a0fdf228aa1b9", md5sum(gpx_files(:zipped_trace_file).xml_file)
-    assert_equal "848caa72f2f456d1bd6a0fdf228aa1b9", md5sum(gpx_files(:tar_trace_file).xml_file)
-    assert_equal "848caa72f2f456d1bd6a0fdf228aa1b9", md5sum(gpx_files(:tar_gzip_trace_file).xml_file)
-    assert_equal "848caa72f2f456d1bd6a0fdf228aa1b9", md5sum(gpx_files(:tar_bzip_trace_file).xml_file)
+    check_xml_file(1, "848caa72f2f456d1bd6a0fdf228aa1b9")
+    check_xml_file(2, "66179ca44f1e93d8df62e2b88cbea732")
+    check_xml_file(3, "848caa72f2f456d1bd6a0fdf228aa1b9")
+    check_xml_file(4, "abd6675fdf3024a84fc0a1deac147c0d")
+    check_xml_file(6, "848caa72f2f456d1bd6a0fdf228aa1b9")
+    check_xml_file(7, "848caa72f2f456d1bd6a0fdf228aa1b9")
+    check_xml_file(8, "848caa72f2f456d1bd6a0fdf228aa1b9")
+    check_xml_file(9, "848caa72f2f456d1bd6a0fdf228aa1b9")
   end
 
   def test_large_picture
-    picture = gpx_files(:public_trace_file).large_picture
-    trace = Trace.create
+    trace = create(:trace)
+    picture = trace.stub :large_picture_name, "#{GPX_IMAGE_DIR}/1.gif" do
+      trace.large_picture
+    end
 
+    trace = Trace.create
     trace.large_picture = picture
     assert_equal "7c841749e084ee4a5d13f12cd3bef456", md5sum(File.new(trace.large_picture_name))
     assert_equal picture, trace.large_picture
@@ -160,9 +180,12 @@ class TraceTest < ActiveSupport::TestCase
   end
 
   def test_icon_picture
-    picture = gpx_files(:public_trace_file).icon_picture
-    trace = Trace.create
+    trace = create(:trace)
+    picture = trace.stub :icon_picture_name, "#{GPX_IMAGE_DIR}/1_icon.gif" do
+      trace.icon_picture
+    end
 
+    trace = Trace.create
     trace.icon_picture = picture
     assert_equal "b47baf22ed0e85d77e808694fad0ee27", md5sum(File.new(trace.icon_picture_name))
     assert_equal picture, trace.icon_picture
@@ -173,12 +196,33 @@ class TraceTest < ActiveSupport::TestCase
   private
 
   def check_query(query, traces)
-    traces = traces.map { |t| gpx_files(t).id }.sort
+    traces = traces.map { |t| t.id }.sort
     assert_equal traces, query.order(:id).ids
   end
 
+  def check_mime_type(id, mime_type)
+    trace = create(:trace)
+    trace.stub :trace_name, "#{GPX_TRACE_DIR}/#{id}.gpx" do
+      assert_equal mime_type, trace.mime_type
+    end
+  end
+
+  def check_extension_name(id, extension_name)
+    trace = create(:trace)
+    trace.stub :trace_name, "#{GPX_TRACE_DIR}/#{id}.gpx" do
+      assert_equal extension_name, trace.extension_name
+    end
+  end
+
+  def check_xml_file(id, md5sum)
+    trace = create(:trace)
+    trace.stub :trace_name, "#{GPX_TRACE_DIR}/#{id}.gpx" do
+      assert_equal md5sum, md5sum(trace.xml_file)
+    end
+  end
+
   def trace_valid(attrs, result = true)
-    entry = Trace.new(gpx_files(:public_trace_file).attributes)
+    entry = build(:trace)
     entry.assign_attributes(attrs)
     assert_equal result, entry.valid?, "Expected #{attrs.inspect} to be #{result}"
   end
diff --git a/test/models/tracepoint_test.rb b/test/models/tracepoint_test.rb
index e24a833cf..45dd7496f 100644
--- a/test/models/tracepoint_test.rb
+++ b/test/models/tracepoint_test.rb
@@ -1,9 +1,10 @@
 require "test_helper"
 
 class TracepointTest < ActiveSupport::TestCase
-  api_fixtures
-
-  def test_tracepoint_count
-    assert_equal 4, Tracepoint.count
+  def test_timestamp_required
+    tracepoint = create(:tracepoint)
+    assert tracepoint.valid?
+    tracepoint.timestamp = nil
+    assert !tracepoint.valid?
   end
 end
diff --git a/test/models/tracetag_test.rb b/test/models/tracetag_test.rb
index a13d41e4a..04263dbb7 100644
--- a/test/models/tracetag_test.rb
+++ b/test/models/tracetag_test.rb
@@ -1,12 +1,6 @@
 require "test_helper"
 
 class TracetagTest < ActiveSupport::TestCase
-  api_fixtures
-
-  def test_tracetag_count
-    assert_equal 4, Tracetag.count
-  end
-
   def test_validations
     tracetag_valid({})
     tracetag_valid({ :tag => nil }, false)
@@ -24,7 +18,7 @@ class TracetagTest < ActiveSupport::TestCase
   private
 
   def tracetag_valid(attrs, result = true)
-    entry = Tracetag.new(gpx_file_tags(:first_trace_1).attributes)
+    entry = build(:tracetag)
     entry.assign_attributes(attrs)
     assert_equal result, entry.valid?, "Expected #{attrs.inspect} to be #{result}"
   end
diff --git a/test/test_helper.rb b/test/test_helper.rb
index 0cf7c4afb..77674147f 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -52,11 +52,6 @@ module ActiveSupport
       set_fixture_class :relation_members => OldRelationMember
       set_fixture_class :relation_tags => OldRelationTag
 
-      fixtures :gpx_files, :gps_points, :gpx_file_tags
-      set_fixture_class :gpx_files => Trace
-      set_fixture_class :gps_points => Tracepoint
-      set_fixture_class :gpx_file_tags => Tracetag
-
       fixtures :client_applications
 
       fixtures :redactions

From ea502ac9df868bef953b3595d81aa59c14da07fc Mon Sep 17 00:00:00 2001
From: Andy Allan <git@gravitystorm.co.uk>
Date: Sat, 29 Oct 2016 17:32:50 +0200
Subject: [PATCH 2/7] Rubocop fix.

---
 test/models/trace_test.rb | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/test/models/trace_test.rb b/test/models/trace_test.rb
index 1cb94aeef..bdc187c99 100644
--- a/test/models/trace_test.rb
+++ b/test/models/trace_test.rb
@@ -196,7 +196,7 @@ class TraceTest < ActiveSupport::TestCase
   private
 
   def check_query(query, traces)
-    traces = traces.map { |t| t.id }.sort
+    traces = traces.map(&:id).sort
     assert_equal traces, query.order(:id).ids
   end
 

From a32333ba12bb1fed2ca217e7994cbad0a3127dd2 Mon Sep 17 00:00:00 2001
From: Andy Allan <git@gravitystorm.co.uk>
Date: Sun, 30 Oct 2016 10:22:10 +0100
Subject: [PATCH 3/7] Use stub_any_instance in order to stub find results.

This is a much cleaner approach than before.
---
 Gemfile                                   |   1 +
 Gemfile.lock                              |   2 +
 test/controllers/trace_controller_test.rb | 202 +++++++++-------------
 3 files changed, 88 insertions(+), 117 deletions(-)

diff --git a/Gemfile b/Gemfile
index 2cbb1cbef..039216f93 100644
--- a/Gemfile
+++ b/Gemfile
@@ -101,6 +101,7 @@ group :test do
   gem "rubocop"
   gem "timecop"
   gem "minitest", "~> 5.1", :platforms => [:ruby_19, :ruby_20]
+  gem "minitest-stub_any_instance"
 end
 
 # Needed in development as well so rake can see konacha tasks
diff --git a/Gemfile.lock b/Gemfile.lock
index d03cd5ad2..f4573658b 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -150,6 +150,7 @@ GEM
     mimemagic (0.3.0)
     mini_portile2 (2.1.0)
     minitest (5.9.1)
+    minitest-stub_any_instance (1.0.1)
     multi_json (1.12.1)
     multi_xml (0.5.5)
     multipart-post (2.0.0)
@@ -334,6 +335,7 @@ DEPENDENCIES
   libxml-ruby (>= 2.0.5)
   logstasher
   minitest (~> 5.1)
+  minitest-stub_any_instance
   oauth-plugin (>= 0.5.1)
   omniauth
   omniauth-facebook
diff --git a/test/controllers/trace_controller_test.rb b/test/controllers/trace_controller_test.rb
index 9170d6e2e..3888b1294 100644
--- a/test/controllers/trace_controller_test.rb
+++ b/test/controllers/trace_controller_test.rb
@@ -305,49 +305,43 @@ class TraceControllerTest < ActionController::TestCase
   # Test downloading a trace
   def test_data
     public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
-    # We need to stub both the trace_name, to control which file from test/traces is used,
-    # and also the Trace.find method so that our stubbed object is used by the controller.
-    public_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/1.gpx" do
-      Trace.stub :find, public_trace_file do
-        # First with no auth, which should work since the trace is public
-        get :data, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
-        check_trace_data public_trace_file
+    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/1.gpx" do
+      # First with no auth, which should work since the trace is public
+      get :data, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
+      check_trace_data public_trace_file
 
-        # Now with some other user, which should work since the trace is public
-        get :data, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:public_user).id }
-        check_trace_data public_trace_file
+      # Now with some other user, which should work since the trace is public
+      get :data, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:public_user).id }
+      check_trace_data public_trace_file
 
-        # And finally we should be able to do it with the owner of the trace
-        get :data, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:normal_user).id }
-        check_trace_data public_trace_file
-      end
+      # And finally we should be able to do it with the owner of the trace
+      get :data, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:normal_user).id }
+      check_trace_data public_trace_file
     end
   end
 
   # Test downloading a compressed trace
   def test_data_compressed
     identifiable_trace_file = create(:trace, :visibility => "identifiable")
-    identifiable_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/4.gpx" do
-      Trace.stub :find, identifiable_trace_file do
-        # First get the data as is
-        get :data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id
-        check_trace_data identifiable_trace_file, "application/x-gzip", "gpx.gz"
+    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/4.gpx" do
+      # First get the data as is
+      get :data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id
+      check_trace_data identifiable_trace_file, "application/x-gzip", "gpx.gz"
 
-        # Now ask explicitly for XML format
-        get :data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id, :format => "xml"
-        check_trace_data identifiable_trace_file, "application/xml", "xml"
+      # Now ask explicitly for XML format
+      get :data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id, :format => "xml"
+      check_trace_data identifiable_trace_file, "application/xml", "xml"
 
-        # Now ask explicitly for GPX format
-        get :data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id, :format => "gpx"
-        check_trace_data identifiable_trace_file
-      end
+      # Now ask explicitly for GPX format
+      get :data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id, :format => "gpx"
+      check_trace_data identifiable_trace_file
     end
   end
 
   # Check an anonymous trace can't be downloaded by another user
   def test_data_anon
     anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
-    anon_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/2.gpx" do
+    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/2.gpx" do
       # First with no auth
       get :data, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
       assert_response :not_found
@@ -357,10 +351,8 @@ class TraceControllerTest < ActionController::TestCase
       assert_response :not_found
 
       # And finally we should be able to do it with the owner of the trace
-      Trace.stub :find, anon_trace_file do
-        get :data, { :display_name => users(:public_user).display_name, :id => anon_trace_file.id }, { :user => users(:public_user).id }
-        check_trace_data anon_trace_file
-      end
+      get :data, { :display_name => users(:public_user).display_name, :id => anon_trace_file.id }, { :user => users(:public_user).id }
+      check_trace_data anon_trace_file
     end
   end
 
@@ -383,27 +375,25 @@ class TraceControllerTest < ActionController::TestCase
   # Test downloading the picture for a trace
   def test_picture
     public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
-    public_trace_file.stub :large_picture_name, "#{GPX_TRACE_DIR}/1.gif" do
-      Trace.stub :find, public_trace_file do
-        # First with no auth, which should work since the trace is public
-        get :picture, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
-        check_trace_picture public_trace_file
+    Trace.stub_any_instance :large_picture_name, "#{GPX_TRACE_DIR}/1.gif" do
+      # First with no auth, which should work since the trace is public
+      get :picture, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
+      check_trace_picture public_trace_file
 
-        # Now with some other user, which should work since the trace is public
-        get :picture, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:public_user).id }
-        check_trace_picture public_trace_file
+      # Now with some other user, which should work since the trace is public
+      get :picture, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:public_user).id }
+      check_trace_picture public_trace_file
 
-        # And finally we should be able to do it with the owner of the trace
-        get :picture, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:normal_user).id }
-        check_trace_picture public_trace_file
-      end
+      # And finally we should be able to do it with the owner of the trace
+      get :picture, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:normal_user).id }
+      check_trace_picture public_trace_file
     end
   end
 
   # Check the picture for an anonymous trace can't be downloaded by another user
   def test_picture_anon
     anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
-    anon_trace_file.stub :large_picture_name, "#{GPX_TRACE_DIR}/2.gif" do
+    Trace.stub_any_instance :large_picture_name, "#{GPX_TRACE_DIR}/2.gif" do
       # First with no auth
       get :picture, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
       assert_response :forbidden
@@ -413,10 +403,8 @@ class TraceControllerTest < ActionController::TestCase
       assert_response :forbidden
 
       # And finally we should be able to do it with the owner of the trace
-      Trace.stub :find, anon_trace_file do
-        get :picture, { :display_name => users(:public_user).display_name, :id => anon_trace_file.id }, { :user => users(:public_user).id }
-        check_trace_picture anon_trace_file
-      end
+      get :picture, { :display_name => users(:public_user).display_name, :id => anon_trace_file.id }, { :user => users(:public_user).id }
+      check_trace_picture anon_trace_file
     end
   end
 
@@ -439,27 +427,25 @@ class TraceControllerTest < ActionController::TestCase
   # Test downloading the icon for a trace
   def test_icon
     public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
-    public_trace_file.stub :icon_picture_name, "#{GPX_TRACE_DIR}/1_icon.gif" do
-      Trace.stub :find, public_trace_file do
-        # First with no auth, which should work since the trace is public
-        get :icon, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
-        check_trace_icon public_trace_file
+    Trace.stub_any_instance :icon_picture_name, "#{GPX_TRACE_DIR}/1_icon.gif" do
+      # First with no auth, which should work since the trace is public
+      get :icon, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
+      check_trace_icon public_trace_file
 
-        # Now with some other user, which should work since the trace is public
-        get :icon, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:public_user).id }
-        check_trace_icon public_trace_file
+      # Now with some other user, which should work since the trace is public
+      get :icon, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:public_user).id }
+      check_trace_icon public_trace_file
 
-        # And finally we should be able to do it with the owner of the trace
-        get :icon, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:normal_user).id }
-        check_trace_icon public_trace_file
-      end
+      # And finally we should be able to do it with the owner of the trace
+      get :icon, { :display_name => users(:normal_user).display_name, :id => public_trace_file.id }, { :user => users(:normal_user).id }
+      check_trace_icon public_trace_file
     end
   end
 
   # Check the icon for an anonymous trace can't be downloaded by another user
   def test_icon_anon
     anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
-    anon_trace_file.stub :icon_picture_name, "#{GPX_TRACE_DIR}/2_icon.gif" do
+    Trace.stub_any_instance :icon_picture_name, "#{GPX_TRACE_DIR}/2_icon.gif" do
       # First with no auth
       get :icon, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
       assert_response :forbidden
@@ -469,10 +455,8 @@ class TraceControllerTest < ActionController::TestCase
       assert_response :forbidden
 
       # And finally we should be able to do it with the owner of the trace
-      Trace.stub :find, anon_trace_file do
-        get :icon, { :display_name => users(:public_user).display_name, :id => anon_trace_file.id }, { :user => users(:public_user).id }
-        check_trace_icon anon_trace_file
-      end
+      get :icon, { :display_name => users(:public_user).display_name, :id => anon_trace_file.id }, { :user => users(:public_user).id }
+      check_trace_icon anon_trace_file
     end
   end
 
@@ -699,77 +683,61 @@ class TraceControllerTest < ActionController::TestCase
   # Test downloading a trace through the api
   def test_api_data
     public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
-    public_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/1.gpx" do
-      visible = MiniTest::Mock.new
-      visible.expect :find, public_trace_file, [String]
-      visible.expect :find, public_trace_file, [String]
-      Trace.stub :visible, visible do
-        # First with no auth
-        get :api_data, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
-        assert_response :unauthorized
+    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/1.gpx" do
+      # First with no auth
+      get :api_data, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
+      assert_response :unauthorized
 
-        # Now with some other user, which should work since the trace is public
-        basic_authorization(users(:public_user).display_name, "test")
-        get :api_data, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
-        check_trace_data public_trace_file
+      # Now with some other user, which should work since the trace is public
+      basic_authorization(users(:public_user).display_name, "test")
+      get :api_data, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
+      check_trace_data public_trace_file
 
-        # # And finally we should be able to do it with the owner of the trace
-        basic_authorization(users(:normal_user).display_name, "test")
-        get :api_data, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
-        check_trace_data public_trace_file
-      end
+      # # And finally we should be able to do it with the owner of the trace
+      basic_authorization(users(:normal_user).display_name, "test")
+      get :api_data, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
+      check_trace_data public_trace_file
     end
   end
 
   # Test downloading a compressed trace through the api
   def test_api_data_compressed
     identifiable_trace_file = create(:trace, :visibility => "identifiable", :user => users(:public_user))
-    identifiable_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/4.gpx" do
-      visible = MiniTest::Mock.new
-      visible.expect :find, identifiable_trace_file, [String]
-      visible.expect :find, identifiable_trace_file, [String]
-      visible.expect :find, identifiable_trace_file, [String]
-      Trace.stub :visible, visible do
-        # Authenticate as the owner of the trace we will be using
-        basic_authorization(users(:public_user).display_name, "test")
+    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/4.gpx" do
+      # Authenticate as the owner of the trace we will be using
+      basic_authorization(users(:public_user).display_name, "test")
 
-        # First get the data as is
-        get :api_data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id
-        check_trace_data identifiable_trace_file, "application/x-gzip", "gpx.gz"
+      # First get the data as is
+      get :api_data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id
+      check_trace_data identifiable_trace_file, "application/x-gzip", "gpx.gz"
 
-        # Now ask explicitly for XML format
-        get :api_data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id, :format => "xml"
-        check_trace_data identifiable_trace_file, "application/xml", "xml"
+      # Now ask explicitly for XML format
+      get :api_data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id, :format => "xml"
+      check_trace_data identifiable_trace_file, "application/xml", "xml"
 
-        # # Now ask explicitly for GPX format
-        get :api_data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id, :format => "gpx"
-        check_trace_data identifiable_trace_file
-      end
+      # # Now ask explicitly for GPX format
+      get :api_data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id, :format => "gpx"
+      check_trace_data identifiable_trace_file
     end
   end
 
   # Check an anonymous trace can't be downloaded by another user through the api
   def test_api_data_anon
     anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
-    anon_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/2.gpx" do
-      visible = MiniTest::Mock.new
-      visible.expect :find, anon_trace_file, [String]
-      visible.expect :find, anon_trace_file, [String]
-      Trace.stub :visible, visible do
-        # First with no auth
-        get :api_data, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
-        assert_response :unauthorized
+    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/2.gpx" do
+      # First with no auth
+      get :api_data, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
+      assert_response :unauthorized
 
-        # Now with some other user, which shouldn't work since the trace is anon
-        basic_authorization(users(:normal_user).display_name, "test")
-        get :api_data, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
-        assert_response :forbidden
+      # Now with some other user, which shouldn't work since the trace is anon
+      basic_authorization(users(:normal_user).display_name, "test")
+      get :api_data, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
+      assert_response :forbidden
 
-        # And finally we should be able to do it with the owner of the trace
-        basic_authorization(users(:public_user).display_name, "test")
-        get :api_data, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
-        check_trace_data anon_trace_file
-      end
+      # And finally we should be able to do it with the owner of the trace
+      basic_authorization(users(:public_user).display_name, "test")
+      get :api_data, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
+      check_trace_data anon_trace_file
     end
   end
 

From d80dee498926d13ec86417ded215ffd63d5cb4fb Mon Sep 17 00:00:00 2001
From: Andy Allan <git@gravitystorm.co.uk>
Date: Tue, 1 Nov 2016 10:43:38 +0000
Subject: [PATCH 4/7] Move instance creation to the top of the test methods.

---
 test/controllers/trace_controller_test.rb | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/test/controllers/trace_controller_test.rb b/test/controllers/trace_controller_test.rb
index 3888b1294..d91ab3e09 100644
--- a/test/controllers/trace_controller_test.rb
+++ b/test/controllers/trace_controller_test.rb
@@ -285,6 +285,8 @@ class TraceControllerTest < ActionController::TestCase
 
   # Test viewing a trace that doesn't exist
   def test_view_not_found
+    deleted_trace_file = create(:trace, :deleted)
+
     # First with no auth
     get :view, :display_name => users(:public_user).display_name, :id => 0
     assert_response :redirect
@@ -296,7 +298,6 @@ class TraceControllerTest < ActionController::TestCase
     assert_redirected_to :action => :list
 
     # And finally we should not be able to view a deleted trace
-    deleted_trace_file = create(:trace, :deleted)
     get :view, { :display_name => users(:public_user).display_name, :id => deleted_trace_file.id }, { :user => users(:public_user).id }
     assert_response :redirect
     assert_redirected_to :action => :list
@@ -358,6 +359,8 @@ class TraceControllerTest < ActionController::TestCase
 
   # Test downloading a trace that doesn't exist
   def test_data_not_found
+    deleted_trace_file = create(:trace, :deleted)
+
     # First with no auth and a trace that has never existed
     get :data, :display_name => users(:public_user).display_name, :id => 0
     assert_response :not_found
@@ -367,7 +370,6 @@ class TraceControllerTest < ActionController::TestCase
     assert_response :not_found
 
     # Now with a trace that has been deleted
-    deleted_trace_file = create(:trace, :deleted)
     get :data, { :display_name => users(:public_user).display_name, :id => deleted_trace_file.id }, { :user => users(:public_user).id }
     assert_response :not_found
   end

From a66f0f32e977cb2d6ae7231e7014861c119cc657 Mon Sep 17 00:00:00 2001
From: Andy Allan <git@gravitystorm.co.uk>
Date: Wed, 16 Nov 2016 17:45:26 +0000
Subject: [PATCH 5/7] Rename test trace fixtures to use letters instead of
 numbers.

This prevents them from being deleted by mistake, if trace.delete is
called on a factory-generated trace with a coincidental id.
---
 test/controllers/trace_controller_test.rb |  34 +++++++-------
 test/controllers/user_controller_test.rb  |   2 +-
 test/models/trace_test.rb                 |  52 +++++++++++-----------
 test/traces/{1.gif => a.gif}              | Bin
 test/traces/{1.gpx => a.gpx}              |   0
 test/traces/{1_icon.gif => a_icon.gif}    | Bin
 test/traces/{2.gif => b.gif}              | Bin
 test/traces/{10.gpx => b.gpx}             |   0
 test/traces/{2_icon.gif => b_icon.gif}    | Bin
 test/traces/{3.gif => c.gif}              | Bin
 test/traces/{3.gpx => c.gpx}              | Bin
 test/traces/{3_icon.gif => c_icon.gif}    | Bin
 test/traces/{4.gif => d.gif}              | Bin
 test/traces/{4.gpx => d.gpx}              | Bin
 test/traces/{4_icon.gif => d_icon.gif}    | Bin
 test/traces/{6.gif => f.gif}              | Bin
 test/traces/{6.gpx => f.gpx}              | Bin
 test/traces/{6_icon.gif => f_icon.gif}    | Bin
 test/traces/{7.gif => g.gif}              | Bin
 test/traces/{7.gpx => g.gpx}              | Bin
 test/traces/{7_icon.gif => g_icon.gif}    | Bin
 test/traces/{8.gif => h.gif}              | Bin
 test/traces/{8.gpx => h.gpx}              | Bin
 test/traces/{8_icon.gif => h_icon.gif}    | Bin
 test/traces/{9.gif => i.gif}              | Bin
 test/traces/{9.gpx => i.gpx}              | Bin
 test/traces/{9_icon.gif => i_icon.gif}    | Bin
 test/traces/{2.gpx => j.gpx}              |   0
 28 files changed, 44 insertions(+), 44 deletions(-)
 rename test/traces/{1.gif => a.gif} (100%)
 rename test/traces/{1.gpx => a.gpx} (100%)
 rename test/traces/{1_icon.gif => a_icon.gif} (100%)
 rename test/traces/{2.gif => b.gif} (100%)
 rename test/traces/{10.gpx => b.gpx} (100%)
 rename test/traces/{2_icon.gif => b_icon.gif} (100%)
 rename test/traces/{3.gif => c.gif} (100%)
 rename test/traces/{3.gpx => c.gpx} (100%)
 rename test/traces/{3_icon.gif => c_icon.gif} (100%)
 rename test/traces/{4.gif => d.gif} (100%)
 rename test/traces/{4.gpx => d.gpx} (100%)
 rename test/traces/{4_icon.gif => d_icon.gif} (100%)
 rename test/traces/{6.gif => f.gif} (100%)
 rename test/traces/{6.gpx => f.gpx} (100%)
 rename test/traces/{6_icon.gif => f_icon.gif} (100%)
 rename test/traces/{7.gif => g.gif} (100%)
 rename test/traces/{7.gpx => g.gpx} (100%)
 rename test/traces/{7_icon.gif => g_icon.gif} (100%)
 rename test/traces/{8.gif => h.gif} (100%)
 rename test/traces/{8.gpx => h.gpx} (100%)
 rename test/traces/{8_icon.gif => h_icon.gif} (100%)
 rename test/traces/{9.gif => i.gif} (100%)
 rename test/traces/{9.gpx => i.gpx} (100%)
 rename test/traces/{9_icon.gif => i_icon.gif} (100%)
 rename test/traces/{2.gpx => j.gpx} (100%)

diff --git a/test/controllers/trace_controller_test.rb b/test/controllers/trace_controller_test.rb
index d91ab3e09..d6159ae40 100644
--- a/test/controllers/trace_controller_test.rb
+++ b/test/controllers/trace_controller_test.rb
@@ -306,7 +306,7 @@ class TraceControllerTest < ActionController::TestCase
   # Test downloading a trace
   def test_data
     public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
-    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/1.gpx" do
+    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/a.gpx" do
       # First with no auth, which should work since the trace is public
       get :data, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
       check_trace_data public_trace_file
@@ -324,7 +324,7 @@ class TraceControllerTest < ActionController::TestCase
   # Test downloading a compressed trace
   def test_data_compressed
     identifiable_trace_file = create(:trace, :visibility => "identifiable")
-    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/4.gpx" do
+    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/d.gpx" do
       # First get the data as is
       get :data, :display_name => users(:public_user).display_name, :id => identifiable_trace_file.id
       check_trace_data identifiable_trace_file, "application/x-gzip", "gpx.gz"
@@ -342,7 +342,7 @@ class TraceControllerTest < ActionController::TestCase
   # Check an anonymous trace can't be downloaded by another user
   def test_data_anon
     anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
-    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/2.gpx" do
+    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/b.gpx" do
       # First with no auth
       get :data, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
       assert_response :not_found
@@ -377,7 +377,7 @@ class TraceControllerTest < ActionController::TestCase
   # Test downloading the picture for a trace
   def test_picture
     public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
-    Trace.stub_any_instance :large_picture_name, "#{GPX_TRACE_DIR}/1.gif" do
+    Trace.stub_any_instance :large_picture_name, "#{GPX_TRACE_DIR}/a.gif" do
       # First with no auth, which should work since the trace is public
       get :picture, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
       check_trace_picture public_trace_file
@@ -395,7 +395,7 @@ class TraceControllerTest < ActionController::TestCase
   # Check the picture for an anonymous trace can't be downloaded by another user
   def test_picture_anon
     anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
-    Trace.stub_any_instance :large_picture_name, "#{GPX_TRACE_DIR}/2.gif" do
+    Trace.stub_any_instance :large_picture_name, "#{GPX_TRACE_DIR}/b.gif" do
       # First with no auth
       get :picture, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
       assert_response :forbidden
@@ -429,7 +429,7 @@ class TraceControllerTest < ActionController::TestCase
   # Test downloading the icon for a trace
   def test_icon
     public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
-    Trace.stub_any_instance :icon_picture_name, "#{GPX_TRACE_DIR}/1_icon.gif" do
+    Trace.stub_any_instance :icon_picture_name, "#{GPX_TRACE_DIR}/a_icon.gif" do
       # First with no auth, which should work since the trace is public
       get :icon, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
       check_trace_icon public_trace_file
@@ -447,7 +447,7 @@ class TraceControllerTest < ActionController::TestCase
   # Check the icon for an anonymous trace can't be downloaded by another user
   def test_icon_anon
     anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
-    Trace.stub_any_instance :icon_picture_name, "#{GPX_TRACE_DIR}/2_icon.gif" do
+    Trace.stub_any_instance :icon_picture_name, "#{GPX_TRACE_DIR}/b_icon.gif" do
       # First with no auth
       get :icon, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
       assert_response :forbidden
@@ -509,7 +509,7 @@ class TraceControllerTest < ActionController::TestCase
   # Test creating a trace
   def test_create_post
     public_trace_file = create(:trace, :visibility => "public")
-    public_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/1.gpx" do
+    public_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/a.gpx" do
       # Get file to use
       file = Rack::Test::UploadedFile.new(public_trace_file.trace_name, "application/gpx+xml")
 
@@ -525,7 +525,7 @@ class TraceControllerTest < ActionController::TestCase
       assert_redirected_to :action => :list, :display_name => users(:public_user).display_name
       assert_match /file has been uploaded/, flash[:notice]
       trace = Trace.order(:id => :desc).first
-      assert_equal "1.gpx", trace.name
+      assert_equal "a.gpx", trace.name
       assert_equal "New Trace", trace.description
       assert_equal %w(new trace), trace.tags.order(:tag).collect(&:tag)
       assert_equal "trackable", trace.visibility
@@ -685,7 +685,7 @@ class TraceControllerTest < ActionController::TestCase
   # Test downloading a trace through the api
   def test_api_data
     public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
-    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/1.gpx" do
+    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/a.gpx" do
       # First with no auth
       get :api_data, :display_name => users(:normal_user).display_name, :id => public_trace_file.id
       assert_response :unauthorized
@@ -705,7 +705,7 @@ class TraceControllerTest < ActionController::TestCase
   # Test downloading a compressed trace through the api
   def test_api_data_compressed
     identifiable_trace_file = create(:trace, :visibility => "identifiable", :user => users(:public_user))
-    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/4.gpx" do
+    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/d.gpx" do
       # Authenticate as the owner of the trace we will be using
       basic_authorization(users(:public_user).display_name, "test")
 
@@ -726,7 +726,7 @@ class TraceControllerTest < ActionController::TestCase
   # Check an anonymous trace can't be downloaded by another user through the api
   def test_api_data_anon
     anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
-    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/2.gpx" do
+    Trace.stub_any_instance :trace_name, "#{GPX_TRACE_DIR}/b.gpx" do
       # First with no auth
       get :api_data, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
       assert_response :unauthorized
@@ -764,7 +764,7 @@ class TraceControllerTest < ActionController::TestCase
   # Test creating a trace through the api
   def test_api_create
     public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
-    public_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/1.gpx" do
+    public_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/a.gpx" do
       # Get file to use
       file = Rack::Test::UploadedFile.new(public_trace_file.trace_name, "application/gpx+xml")
 
@@ -779,7 +779,7 @@ class TraceControllerTest < ActionController::TestCase
       post :api_create, :file => file, :description => "New Trace", :tags => "new,trace", :visibility => "trackable"
       assert_response :success
       trace = Trace.find(response.body.to_i)
-      assert_equal "1.gpx", trace.name
+      assert_equal "a.gpx", trace.name
       assert_equal "New Trace", trace.description
       assert_equal %w(new trace), trace.tags.order(:tag).collect(&:tag)
       assert_equal "trackable", trace.visibility
@@ -797,7 +797,7 @@ class TraceControllerTest < ActionController::TestCase
       post :api_create, :file => file, :description => "New Trace", :tags => "new,trace", :public => 1
       assert_response :success
       trace = Trace.find(response.body.to_i)
-      assert_equal "1.gpx", trace.name
+      assert_equal "a.gpx", trace.name
       assert_equal "New Trace", trace.description
       assert_equal %w(new trace), trace.tags.order(:tag).collect(&:tag)
       assert_equal "public", trace.visibility
@@ -815,7 +815,7 @@ class TraceControllerTest < ActionController::TestCase
       post :api_create, :file => file, :description => "New Trace", :tags => "new,trace", :public => 0
       assert_response :success
       trace = Trace.find(response.body.to_i)
-      assert_equal "1.gpx", trace.name
+      assert_equal "a.gpx", trace.name
       assert_equal "New Trace", trace.description
       assert_equal %w(new trace), trace.tags.order(:tag).collect(&:tag)
       assert_equal "private", trace.visibility
@@ -832,7 +832,7 @@ class TraceControllerTest < ActionController::TestCase
     deleted_trace_file = create(:trace, :deleted, :user => users(:public_user))
     anon_trace_file = create(:trace, :visibility => "private", :user => users(:public_user))
 
-    public_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/1.gpx" do
+    public_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/a.gpx" do
       # First with no auth
       content public_trace_file.to_xml
       put :api_update, :id => public_trace_file.id
diff --git a/test/controllers/user_controller_test.rb b/test/controllers/user_controller_test.rb
index 717089b64..9e499b605 100644
--- a/test/controllers/user_controller_test.rb
+++ b/test/controllers/user_controller_test.rb
@@ -804,7 +804,7 @@ class UserControllerTest < ActionController::TestCase
     assert_select "form#accountForm > fieldset > div.form-row > select#user_preferred_editor > option[selected]", false
 
     # Changing to an uploaded image should work
-    image = Rack::Test::UploadedFile.new("test/traces/1.gif", "image/gif")
+    image = Rack::Test::UploadedFile.new("test/traces/a.gif", "image/gif")
     post :account, { :display_name => user.display_name, :image_action => "new", :user => user.attributes.merge(:image => image) }, { :user => user.id }
     assert_response :success
     assert_template :account
diff --git a/test/models/trace_test.rb b/test/models/trace_test.rb
index bdc187c99..c9530b067 100644
--- a/test/models/trace_test.rb
+++ b/test/models/trace_test.rb
@@ -132,42 +132,42 @@ class TraceTest < ActiveSupport::TestCase
 
   def test_mime_type
     # The ids refer to the .gpx fixtures in test/traces
-    check_mime_type(1, "application/gpx+xml")
-    check_mime_type(2, "application/gpx+xml")
-    check_mime_type(3, "application/x-bzip2")
-    check_mime_type(4, "application/x-gzip")
-    check_mime_type(6, "application/x-zip")
-    check_mime_type(7, "application/x-tar")
-    check_mime_type(8, "application/x-gzip")
-    check_mime_type(9, "application/x-bzip2")
+    check_mime_type("a", "application/gpx+xml")
+    check_mime_type("b", "application/gpx+xml")
+    check_mime_type("c", "application/x-bzip2")
+    check_mime_type("d", "application/x-gzip")
+    check_mime_type("f", "application/x-zip")
+    check_mime_type("g", "application/x-tar")
+    check_mime_type("h", "application/x-gzip")
+    check_mime_type("i", "application/x-bzip2")
   end
 
   def test_extension_name
     # The ids refer to the .gpx fixtures in test/traces
-    check_extension_name(1, ".gpx")
-    check_extension_name(2, ".gpx")
-    check_extension_name(3, ".gpx.bz2")
-    check_extension_name(4, ".gpx.gz")
-    check_extension_name(6, ".zip")
-    check_extension_name(7, ".tar")
-    check_extension_name(8, ".tar.gz")
-    check_extension_name(9, ".tar.bz2")
+    check_extension_name("a", ".gpx")
+    check_extension_name("b", ".gpx")
+    check_extension_name("c", ".gpx.bz2")
+    check_extension_name("d", ".gpx.gz")
+    check_extension_name("f", ".zip")
+    check_extension_name("g", ".tar")
+    check_extension_name("h", ".tar.gz")
+    check_extension_name("i", ".tar.bz2")
   end
 
   def test_xml_file
-    check_xml_file(1, "848caa72f2f456d1bd6a0fdf228aa1b9")
-    check_xml_file(2, "66179ca44f1e93d8df62e2b88cbea732")
-    check_xml_file(3, "848caa72f2f456d1bd6a0fdf228aa1b9")
-    check_xml_file(4, "abd6675fdf3024a84fc0a1deac147c0d")
-    check_xml_file(6, "848caa72f2f456d1bd6a0fdf228aa1b9")
-    check_xml_file(7, "848caa72f2f456d1bd6a0fdf228aa1b9")
-    check_xml_file(8, "848caa72f2f456d1bd6a0fdf228aa1b9")
-    check_xml_file(9, "848caa72f2f456d1bd6a0fdf228aa1b9")
+    check_xml_file("a", "848caa72f2f456d1bd6a0fdf228aa1b9")
+    check_xml_file("b", "66179ca44f1e93d8df62e2b88cbea732")
+    check_xml_file("c", "848caa72f2f456d1bd6a0fdf228aa1b9")
+    check_xml_file("d", "abd6675fdf3024a84fc0a1deac147c0d")
+    check_xml_file("f", "848caa72f2f456d1bd6a0fdf228aa1b9")
+    check_xml_file("g", "848caa72f2f456d1bd6a0fdf228aa1b9")
+    check_xml_file("h", "848caa72f2f456d1bd6a0fdf228aa1b9")
+    check_xml_file("i", "848caa72f2f456d1bd6a0fdf228aa1b9")
   end
 
   def test_large_picture
     trace = create(:trace)
-    picture = trace.stub :large_picture_name, "#{GPX_IMAGE_DIR}/1.gif" do
+    picture = trace.stub :large_picture_name, "#{GPX_IMAGE_DIR}/a.gif" do
       trace.large_picture
     end
 
@@ -181,7 +181,7 @@ class TraceTest < ActiveSupport::TestCase
 
   def test_icon_picture
     trace = create(:trace)
-    picture = trace.stub :icon_picture_name, "#{GPX_IMAGE_DIR}/1_icon.gif" do
+    picture = trace.stub :icon_picture_name, "#{GPX_IMAGE_DIR}/a_icon.gif" do
       trace.icon_picture
     end
 
diff --git a/test/traces/1.gif b/test/traces/a.gif
similarity index 100%
rename from test/traces/1.gif
rename to test/traces/a.gif
diff --git a/test/traces/1.gpx b/test/traces/a.gpx
similarity index 100%
rename from test/traces/1.gpx
rename to test/traces/a.gpx
diff --git a/test/traces/1_icon.gif b/test/traces/a_icon.gif
similarity index 100%
rename from test/traces/1_icon.gif
rename to test/traces/a_icon.gif
diff --git a/test/traces/2.gif b/test/traces/b.gif
similarity index 100%
rename from test/traces/2.gif
rename to test/traces/b.gif
diff --git a/test/traces/10.gpx b/test/traces/b.gpx
similarity index 100%
rename from test/traces/10.gpx
rename to test/traces/b.gpx
diff --git a/test/traces/2_icon.gif b/test/traces/b_icon.gif
similarity index 100%
rename from test/traces/2_icon.gif
rename to test/traces/b_icon.gif
diff --git a/test/traces/3.gif b/test/traces/c.gif
similarity index 100%
rename from test/traces/3.gif
rename to test/traces/c.gif
diff --git a/test/traces/3.gpx b/test/traces/c.gpx
similarity index 100%
rename from test/traces/3.gpx
rename to test/traces/c.gpx
diff --git a/test/traces/3_icon.gif b/test/traces/c_icon.gif
similarity index 100%
rename from test/traces/3_icon.gif
rename to test/traces/c_icon.gif
diff --git a/test/traces/4.gif b/test/traces/d.gif
similarity index 100%
rename from test/traces/4.gif
rename to test/traces/d.gif
diff --git a/test/traces/4.gpx b/test/traces/d.gpx
similarity index 100%
rename from test/traces/4.gpx
rename to test/traces/d.gpx
diff --git a/test/traces/4_icon.gif b/test/traces/d_icon.gif
similarity index 100%
rename from test/traces/4_icon.gif
rename to test/traces/d_icon.gif
diff --git a/test/traces/6.gif b/test/traces/f.gif
similarity index 100%
rename from test/traces/6.gif
rename to test/traces/f.gif
diff --git a/test/traces/6.gpx b/test/traces/f.gpx
similarity index 100%
rename from test/traces/6.gpx
rename to test/traces/f.gpx
diff --git a/test/traces/6_icon.gif b/test/traces/f_icon.gif
similarity index 100%
rename from test/traces/6_icon.gif
rename to test/traces/f_icon.gif
diff --git a/test/traces/7.gif b/test/traces/g.gif
similarity index 100%
rename from test/traces/7.gif
rename to test/traces/g.gif
diff --git a/test/traces/7.gpx b/test/traces/g.gpx
similarity index 100%
rename from test/traces/7.gpx
rename to test/traces/g.gpx
diff --git a/test/traces/7_icon.gif b/test/traces/g_icon.gif
similarity index 100%
rename from test/traces/7_icon.gif
rename to test/traces/g_icon.gif
diff --git a/test/traces/8.gif b/test/traces/h.gif
similarity index 100%
rename from test/traces/8.gif
rename to test/traces/h.gif
diff --git a/test/traces/8.gpx b/test/traces/h.gpx
similarity index 100%
rename from test/traces/8.gpx
rename to test/traces/h.gpx
diff --git a/test/traces/8_icon.gif b/test/traces/h_icon.gif
similarity index 100%
rename from test/traces/8_icon.gif
rename to test/traces/h_icon.gif
diff --git a/test/traces/9.gif b/test/traces/i.gif
similarity index 100%
rename from test/traces/9.gif
rename to test/traces/i.gif
diff --git a/test/traces/9.gpx b/test/traces/i.gpx
similarity index 100%
rename from test/traces/9.gpx
rename to test/traces/i.gpx
diff --git a/test/traces/9_icon.gif b/test/traces/i_icon.gif
similarity index 100%
rename from test/traces/9_icon.gif
rename to test/traces/i_icon.gif
diff --git a/test/traces/2.gpx b/test/traces/j.gpx
similarity index 100%
rename from test/traces/2.gpx
rename to test/traces/j.gpx

From 717f4e1b7b765d12dd5cc13bb8ae193d07a1637f Mon Sep 17 00:00:00 2001
From: Andy Allan <git@gravitystorm.co.uk>
Date: Wed, 16 Nov 2016 20:08:16 +0000
Subject: [PATCH 6/7] Create object for list tests, and assert the responses
 have the required number of results.

Previously, the tests would pass regardless of whether anything was
in the database.
---
 test/controllers/trace_controller_test.rb | 56 +++++++++++++++++------
 1 file changed, 42 insertions(+), 14 deletions(-)

diff --git a/test/controllers/trace_controller_test.rb b/test/controllers/trace_controller_test.rb
index d6159ae40..2c47b42ec 100644
--- a/test/controllers/trace_controller_test.rb
+++ b/test/controllers/trace_controller_test.rb
@@ -166,27 +166,48 @@ class TraceControllerTest < ActionController::TestCase
     )
   end
 
-  # Check that the list of changesets is displayed
+  # Check that the list of traces is displayed
   def test_list
+    # The fourth test below is surpisingly sensitive to timestamp ordering when the timestamps are equal.
+    create(:trace, :visibility => "public", :timestamp => 4.seconds.ago) do |trace|
+      create(:tracetag, :trace => trace, :tag => "London")
+    end
+    create(:trace, :visibility => "public", :timestamp => 3.seconds.ago) do |trace|
+      create(:tracetag, :trace => trace, :tag => "Birmingham")
+    end
+    create(:trace, :visibility => "private", :user => users(:public_user), :timestamp => 2.seconds.ago) do |trace|
+      create(:tracetag, :trace => trace, :tag => "London")
+    end
+    create(:trace, :visibility => "private", :user => users(:public_user), :timestamp => 1.second.ago) do |trace|
+      create(:tracetag, :trace => trace, :tag => "Birmingham")
+    end
+
     # First with the public list
     get :list
-    check_trace_list Trace.visible_to_all
+    check_trace_list Trace.visible_to_all, 2
 
     # Restrict traces to those with a given tag
     get :list, :tag => "London"
-    check_trace_list Trace.tagged("London").visible_to_all
+    check_trace_list Trace.tagged("London").visible_to_all, 1
 
     # Should see more when we are logged in
     get :list, {}, { :user => users(:public_user).id }
-    check_trace_list Trace.visible_to(users(:public_user).id)
+    check_trace_list Trace.visible_to(users(:public_user).id), 4
 
     # Again, we should see more when we are logged in
     get :list, { :tag => "London" }, { :user => users(:public_user).id }
-    check_trace_list Trace.tagged("London").visible_to(users(:public_user).id)
+    check_trace_list Trace.visible_to(users(:public_user).id).tagged("London"), 2
   end
 
   # Check that I can get mine
   def test_list_mine
+    create(:trace, :visibility => "public") do |trace|
+      create(:tracetag, :trace => trace, :tag => "Birmingham")
+    end
+    create(:trace, :visibility => "private", :user => users(:public_user)) do |trace|
+      create(:tracetag, :trace => trace, :tag => "London")
+    end
+
     # First try to get it when not logged in
     get :mine
     assert_redirected_to :controller => "user", :action => "login", :referer => "/traces/mine"
@@ -197,30 +218,36 @@ class TraceControllerTest < ActionController::TestCase
 
     # Fetch the actual list
     get :list, { :display_name => users(:public_user).display_name }, { :user => users(:public_user).id }
-    check_trace_list users(:public_user).traces
+    check_trace_list users(:public_user).traces, 1
   end
 
-  # Check the list of changesets for a specific user
+  # Check the list of traces for a specific user
   def test_list_user
+    create(:trace)
+    create(:trace, :visibility => "public", :user => users(:public_user))
+    create(:trace, :visibility => "private", :user => users(:public_user)) do |trace|
+      create(:tracetag, :trace => trace, :tag => "London")
+    end
+
     # Test a user with no traces
     get :list, :display_name => users(:second_public_user).display_name
-    check_trace_list users(:second_public_user).traces.visible_to_all
+    check_trace_list users(:second_public_user).traces.visible_to_all, 0
 
     # Test a user with some traces - should see only public ones
     get :list, :display_name => users(:public_user).display_name
-    check_trace_list users(:public_user).traces.visible_to_all
+    check_trace_list users(:public_user).traces.visible_to_all, 1
 
     # Should still see only public ones when authenticated as another user
     get :list, { :display_name => users(:public_user).display_name }, { :user => users(:normal_user).id }
-    check_trace_list users(:public_user).traces.visible_to_all
+    check_trace_list users(:public_user).traces.visible_to_all, 1
 
     # Should see all traces when authenticated as the target user
     get :list, { :display_name => users(:public_user).display_name }, { :user => users(:public_user).id }
-    check_trace_list users(:public_user).traces
+    check_trace_list users(:public_user).traces, 2
 
     # Should only see traces with the correct tag when a tag is specified
     get :list, { :display_name => users(:public_user).display_name, :tag => "London" }, { :user => users(:public_user).id }
-    check_trace_list users(:public_user).traces.tagged("London")
+    check_trace_list users(:public_user).traces.tagged("London"), 1
 
     # Should get an error if the user does not exist
     get :list, :display_name => "UnknownUser"
@@ -932,11 +959,12 @@ class TraceControllerTest < ActionController::TestCase
     end
   end
 
-  def check_trace_list(traces)
+  def check_trace_list(traces, count)
     assert_response :success
     assert_template "list"
+    assert_equal traces.count, count
 
-    if traces.count > 0
+    if count > 0
       assert_select "table#trace_list tbody", :count => 1 do
         assert_select "tr", :count => traces.visible.count do |rows|
           traces.visible.order("timestamp DESC").zip(rows).each do |trace, row|

From 62346c7e88901df3ca21afc142cab00f6fb6ae38 Mon Sep 17 00:00:00 2001
From: Andy Allan <git@gravitystorm.co.uk>
Date: Thu, 17 Nov 2016 10:26:54 +0000
Subject: [PATCH 7/7] Be explicit in which traces are expected.

I think this is an improvement compared to running db queries to
fetch the expected results.
---
 test/controllers/trace_controller_test.rb | 45 +++++++++++------------
 1 file changed, 22 insertions(+), 23 deletions(-)

diff --git a/test/controllers/trace_controller_test.rb b/test/controllers/trace_controller_test.rb
index 2c47b42ec..85644b663 100644
--- a/test/controllers/trace_controller_test.rb
+++ b/test/controllers/trace_controller_test.rb
@@ -169,34 +169,34 @@ class TraceControllerTest < ActionController::TestCase
   # Check that the list of traces is displayed
   def test_list
     # The fourth test below is surpisingly sensitive to timestamp ordering when the timestamps are equal.
-    create(:trace, :visibility => "public", :timestamp => 4.seconds.ago) do |trace|
+    trace_a = create(:trace, :visibility => "public", :timestamp => 4.seconds.ago) do |trace|
       create(:tracetag, :trace => trace, :tag => "London")
     end
-    create(:trace, :visibility => "public", :timestamp => 3.seconds.ago) do |trace|
+    trace_b = create(:trace, :visibility => "public", :timestamp => 3.seconds.ago) do |trace|
       create(:tracetag, :trace => trace, :tag => "Birmingham")
     end
-    create(:trace, :visibility => "private", :user => users(:public_user), :timestamp => 2.seconds.ago) do |trace|
+    trace_c = create(:trace, :visibility => "private", :user => users(:public_user), :timestamp => 2.seconds.ago) do |trace|
       create(:tracetag, :trace => trace, :tag => "London")
     end
-    create(:trace, :visibility => "private", :user => users(:public_user), :timestamp => 1.second.ago) do |trace|
+    trace_d = create(:trace, :visibility => "private", :user => users(:public_user), :timestamp => 1.second.ago) do |trace|
       create(:tracetag, :trace => trace, :tag => "Birmingham")
     end
 
     # First with the public list
     get :list
-    check_trace_list Trace.visible_to_all, 2
+    check_trace_list [trace_b, trace_a]
 
     # Restrict traces to those with a given tag
     get :list, :tag => "London"
-    check_trace_list Trace.tagged("London").visible_to_all, 1
+    check_trace_list [trace_a]
 
     # Should see more when we are logged in
     get :list, {}, { :user => users(:public_user).id }
-    check_trace_list Trace.visible_to(users(:public_user).id), 4
+    check_trace_list [trace_d, trace_c, trace_b, trace_a]
 
     # Again, we should see more when we are logged in
     get :list, { :tag => "London" }, { :user => users(:public_user).id }
-    check_trace_list Trace.visible_to(users(:public_user).id).tagged("London"), 2
+    check_trace_list [trace_c, trace_a]
   end
 
   # Check that I can get mine
@@ -204,7 +204,7 @@ class TraceControllerTest < ActionController::TestCase
     create(:trace, :visibility => "public") do |trace|
       create(:tracetag, :trace => trace, :tag => "Birmingham")
     end
-    create(:trace, :visibility => "private", :user => users(:public_user)) do |trace|
+    trace_b = create(:trace, :visibility => "private", :user => users(:public_user)) do |trace|
       create(:tracetag, :trace => trace, :tag => "London")
     end
 
@@ -218,36 +218,36 @@ class TraceControllerTest < ActionController::TestCase
 
     # Fetch the actual list
     get :list, { :display_name => users(:public_user).display_name }, { :user => users(:public_user).id }
-    check_trace_list users(:public_user).traces, 1
+    check_trace_list [trace_b]
   end
 
   # Check the list of traces for a specific user
   def test_list_user
     create(:trace)
-    create(:trace, :visibility => "public", :user => users(:public_user))
-    create(:trace, :visibility => "private", :user => users(:public_user)) do |trace|
+    trace_b = create(:trace, :visibility => "public", :user => users(:public_user))
+    trace_c = create(:trace, :visibility => "private", :user => users(:public_user)) do |trace|
       create(:tracetag, :trace => trace, :tag => "London")
     end
 
     # Test a user with no traces
     get :list, :display_name => users(:second_public_user).display_name
-    check_trace_list users(:second_public_user).traces.visible_to_all, 0
+    check_trace_list []
 
     # Test a user with some traces - should see only public ones
     get :list, :display_name => users(:public_user).display_name
-    check_trace_list users(:public_user).traces.visible_to_all, 1
+    check_trace_list [trace_b]
 
     # Should still see only public ones when authenticated as another user
     get :list, { :display_name => users(:public_user).display_name }, { :user => users(:normal_user).id }
-    check_trace_list users(:public_user).traces.visible_to_all, 1
+    check_trace_list [trace_b]
 
     # Should see all traces when authenticated as the target user
     get :list, { :display_name => users(:public_user).display_name }, { :user => users(:public_user).id }
-    check_trace_list users(:public_user).traces, 2
+    check_trace_list [trace_c, trace_b]
 
-    # Should only see traces with the correct tag when a tag is specified
+    # # Should only see traces with the correct tag when a tag is specified
     get :list, { :display_name => users(:public_user).display_name, :tag => "London" }, { :user => users(:public_user).id }
-    check_trace_list users(:public_user).traces.tagged("London"), 1
+    check_trace_list [trace_c]
 
     # Should get an error if the user does not exist
     get :list, :display_name => "UnknownUser"
@@ -959,15 +959,14 @@ class TraceControllerTest < ActionController::TestCase
     end
   end
 
-  def check_trace_list(traces, count)
+  def check_trace_list(traces)
     assert_response :success
     assert_template "list"
-    assert_equal traces.count, count
 
-    if count > 0
+    if !traces.empty?
       assert_select "table#trace_list tbody", :count => 1 do
-        assert_select "tr", :count => traces.visible.count do |rows|
-          traces.visible.order("timestamp DESC").zip(rows).each do |trace, row|
+        assert_select "tr", :count => traces.length do |rows|
+          traces.zip(rows).each do |trace, row|
             assert_select row, "a", Regexp.new(Regexp.escape(trace.name))
             assert_select row, "span.trace_summary", Regexp.new(Regexp.escape("(#{trace.size} points)")) if trace.inserted?
             assert_select row, "td", Regexp.new(Regexp.escape(trace.description))