Merge remote-tracking branch 'openstreetmap/pull/1347' into master

Gemfile

@@ -103,6 +103,7 @@ end
 # Gems needed for running tests
 group :test do
   gem "minitest", "~> 5.1", :platforms => [:ruby_19, :ruby_20]
+  gem "minitest-stub_any_instance"
   gem "rubocop"
   gem "timecop"
   gem "webmock"

Gemfile.lock

@@ -157,6 +157,7 @@ GEM
     mimemagic (0.3.0)
     mini_portile2 (2.1.0)
     minitest (5.10.1)
+    minitest-stub_any_instance (1.0.1)
     multi_json (1.12.1)
     multi_xml (0.6.0)
     multipart-post (2.0.0)
@@ -354,6 +355,7 @@ DEPENDENCIES
   libxml-ruby (>= 2.0.5)
   logstasher
   minitest (~> 5.1)
+  minitest-stub_any_instance
   oauth-plugin (>= 0.5.1)
   omniauth
   omniauth-facebook

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

test/controllers/api_controller_test.rb

@@ -134,7 +134,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
@@ -150,7 +152,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
@@ -172,7 +177,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

test/controllers/site_controller_test.rb

@@ -282,7 +282,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

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

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")
@@ -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.
+    trace_a = create(:trace, :visibility => "public", :timestamp => 4.seconds.ago) do |trace|
+      create(:tracetag, :trace => trace, :tag => "London")
+    end
+    trace_b = create(:trace, :visibility => "public", :timestamp => 3.seconds.ago) do |trace|
+      create(:tracetag, :trace => trace, :tag => "Birmingham")
+    end
+    trace_c = create(:trace, :visibility => "private", :user => users(:public_user), :timestamp => 2.seconds.ago) do |trace|
+      create(:tracetag, :trace => trace, :tag => "London")
+    end
+    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
+    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
+    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)
+    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.tagged("London").visible_to(users(:public_user).id)
+    check_trace_list [trace_c, trace_a]
   end
 
   # Check that I can get mine
   def test_list_mine
+    create(:trace, :visibility => "public") do |trace|
+      create(:tracetag, :trace => trace, :tag => "Birmingham")
+    end
+    trace_b = 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 [trace_b]
   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)
+    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
+    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
+    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
+    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
+    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")
+    check_trace_list [trace_c]
 
     # Should get an error if the user does not exist
     get :list, :display_name => "UnknownUser"
@@ -249,101 +276,118 @@ 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
+    deleted_trace_file = create(:trace, :deleted)
+
+    # 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
+    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
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+    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 => gpx_files(:public_trace_file).id
-    check_trace_data gpx_files(:public_trace_file)
+      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)
+      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)
+      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")
+    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 => gpx_files(:identifiable_trace_file).id
-    check_trace_data gpx_files(:identifiable_trace_file), "application/x-gzip", "gpx.gz"
+      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"
+      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)
+      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))
+    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 => gpx_files(:anon_trace_file).id
+      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 }
+      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)
+      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
 
   # 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
@@ -353,38 +397,44 @@ 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 }
+    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
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+    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 => gpx_files(:public_trace_file).id
-    check_trace_picture gpx_files(:public_trace_file)
+      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)
+      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)
+      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))
+    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 => gpx_files(:anon_trace_file).id
+      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 }
+      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)
+      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
 
   # Test downloading the picture for a trace that doesn't exist
@@ -397,53 +447,61 @@ 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
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+    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 => gpx_files(:public_trace_file).id
-    check_trace_icon gpx_files(:public_trace_file)
+      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)
+      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)
+      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))
+    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 => gpx_files(:anon_trace_file).id
+      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 }
+      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)
+      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
 
   # 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,8 +535,10 @@ 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}/a.gpx" do
       # Get file to use
-    file = Rack::Test::UploadedFile.new(gpx_files(:public_trace_file).trace_name, "application/gpx+xml")
+      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" }
@@ -492,25 +552,29 @@ 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
       assert_equal false, trace.inserted
-    assert_equal File.new(gpx_files(:public_trace_file).trace_name).read, File.new(trace.trace_name).read
+      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 +582,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 +610,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 +625,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 +641,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 +705,69 @@ 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
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+    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 => gpx_files(:public_trace_file).id
+      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)
+      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
+      # # 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)
+      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))
+    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")
 
       # 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"
+      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"
+      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
 
   # 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}/b.gpx" do
       # First with no auth
-    get :api_data, :display_name => users(:public_user).display_name, :id => gpx_files(:anon_trace_file).id
+      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
+      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)
+      get :api_data, :display_name => users(:public_user).display_name, :id => anon_trace_file.id
+      check_trace_data anon_trace_file
+    end
   end
 
   # Test downloading a trace that doesn't exist through the api
@@ -698,15 +782,18 @@ 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
+    public_trace_file = create(:trace, :visibility => "public", :user => users(:normal_user))
+    public_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/a.gpx" do
       # Get file to use
-    file = Rack::Test::UploadedFile.new(gpx_files(:public_trace_file).trace_name, "application/gpx+xml")
+      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"
@@ -719,12 +806,12 @@ 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
       assert_equal false, trace.inserted
-    assert_equal File.new(gpx_files(:public_trace_file).trace_name).read, File.new(trace.trace_name).read
+      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
 
@@ -737,12 +824,12 @@ 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
       assert_equal false, trace.inserted
-    assert_equal File.new(gpx_files(:public_trace_file).trace_name).read, File.new(trace.trace_name).read
+      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
 
@@ -755,51 +842,57 @@ 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
       assert_equal false, trace.inserted
-    assert_equal File.new(gpx_files(:public_trace_file).trace_name).read, File.new(trace.trace_name).read
+      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
+    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))
+
+    public_trace_file.stub :trace_name, "#{GPX_TRACE_DIR}/a.gpx" do
       # First with no auth
-    content gpx_files(:public_trace_file).to_xml
-    put :api_update, :id => gpx_files(:public_trace_file).id
+      content public_trace_file.to_xml
+      put :api_update, :id => public_trace_file.id
       assert_response :unauthorized
 
       # 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
+      content public_trace_file.to_xml
+      put :api_update, :id => public_trace_file.id
       assert_response :forbidden
 
       # Now with a trace which doesn't exist
       basic_authorization(users(:public_user).display_name, "test")
-    content gpx_files(:public_trace_file).to_xml
+      content public_trace_file.to_xml
       put :api_update, :id => 0
       assert_response :not_found
 
       # 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
+      content deleted_trace_file.to_xml
+      put :api_update, :id => deleted_trace_file.id
       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
+      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 = gpx_files(:public_trace_file)
+      t = public_trace_file
       t.description = "Changed description"
       t.visibility = "private"
       content t.to_xml
@@ -809,16 +902,19 @@ class TraceControllerTest < ActionController::TestCase
       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 +924,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
 
@@ -867,10 +963,10 @@ class TraceControllerTest < ActionController::TestCase
     assert_response :success
     assert_template "list"
 
-    if traces.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))

test/controllers/user_controller_test.rb

@@ -813,7 +813,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
@@ -1094,6 +1094,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
@@ -1105,10 +1111,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

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

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

test/factories/tracetags.rb

@@ -0,0 +1,7 @@
+FactoryGirl.define do
+  factory :tracetag do
+    sequence(:tag) { |n| "Tag #{n}" }
+
+    trace
+  end
+end

test/fixtures/gps_points.yml

@@ -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) %>
-

test/fixtures/gpx_file_tags.yml

@@ -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

test/fixtures/gpx_files.yml

@@ -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

test/integration/oauth_test.rb

@@ -1,8 +1,7 @@
 require "test_helper"
 
 class OAuthTest < ActionDispatch::IntegrationTest
-  fixtures :users, :gpx_files
-  set_fixture_class :gpx_files => Trace
+  fixtures :users
 
   include OAuth::Helper
 
@@ -168,7 +167,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
@@ -230,7 +230,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
@@ -278,7 +279,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

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("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
-    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("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
-    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("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
-    picture = gpx_files(:public_trace_file).large_picture
-    trace = Trace.create
+    trace = create(:trace)
+    picture = trace.stub :large_picture_name, "#{GPX_IMAGE_DIR}/a.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}/a_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(&: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

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

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

test/test_helper.rb

@@ -45,11 +45,6 @@ module ActiveSupport
       fixtures :relation_members
       set_fixture_class :relation_members => OldRelationMember
 
-      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 :redactions
     end