Merge pull request #2120 from TracksApp/remove-testing-cruft

Update some testing practices to use more relevant methods

test/controllers/notes_controller_test.rb

@@ -1,7 +1,7 @@
 require 'test_helper'
 
 class NotesControllerTest < ActionController::TestCase
-  
+
   def test_get_notes_page
     login_as :admin_user
     get :index
@@ -13,27 +13,27 @@ class NotesControllerTest < ActionController::TestCase
     project = users(:admin_user).projects.first
     count = users(:admin_user).notes.count
 
-  	post :create, note: {body: "test note", project_id: project.id}, format: :js
+    post :create, note: {body: "test note", project_id: project.id}, format: :js
 
-  	assert_response 200
-  	assert assigns['saved'], "@saved should be true"
-  	assert count+1, users(:admin_user).notes.reload.count
+    assert_response 200
+    assert assigns['saved'], "@saved should be true"
+    assert count+1, users(:admin_user).notes.reload.count
   end
 
   def test_update_note
-  	login_as :admin_user
+    login_as :admin_user
 
-  	note = users(:admin_user).notes.first
+    note = users(:admin_user).notes.first
 
-  	assert_not_equal "test", note.body
-  	post :update, id: note.id, note: {body: "test"}, format: :js
-  	assert_equal "test", note.reload.body
+    refute_equal "test", note.body
+    post :update, id: note.id, note: {body: "test"}, format: :js
+    assert_equal "test", note.reload.body
   end
 
   def test_destroy_note
-  	login_as :admin_user
+    login_as :admin_user
 
-  	note = users(:admin_user).notes.first
+    note = users(:admin_user).notes.first
     count = users(:admin_user).notes.count
 
     post :destroy, id: note.id, format: :js
@@ -42,5 +42,4 @@ class NotesControllerTest < ActionController::TestCase
     assert_nil old_note
     assert count-1, users(:admin_user).notes.reload.count
   end
-  
 end

test/controllers/todos_controller_test.rb

@@ -397,7 +397,7 @@ class TodosControllerTest < ActionController::TestCase
     todo = users(:admin_user).todos.active.first
     context = users(:admin_user).contexts.first
 
-    assert_not_equal todo.context.id, context.id
+    refute_equal todo.context.id, context.id
 
     xhr :post, :change_context, :id => todo.id, :todo=>{:context_id => context.id}, :_source_view=>"todo"
     assert assigns['context_changed'], "context should have changed"
@@ -820,7 +820,7 @@ class TodosControllerTest < ActionController::TestCase
     next_todo = Todo.where(:recurring_todo_id => recurring_todo_1.id, :state => 'active').first
     assert_equal "Call Bill Gates every day", next_todo.description
     # check that the new todo is not the same as todo_1
-    assert_not_equal todo_1.id, next_todo.id
+    refute_equal todo_1.id, next_todo.id
 
     # change recurrence pattern to monthly and set show_from 2 days before due
     # date this forces the next todo to be put in the tickler
@@ -891,7 +891,7 @@ class TodosControllerTest < ActionController::TestCase
       assert !new_todo.nil?, "the todo should be in the tickler"
 
       assert_equal "Call Bill Gates every day", new_todo.description
-      assert_not_equal todo_1.id, new_todo.id, "check that the new todo is not the same as todo_1"
+      refute_equal todo_1.id, new_todo.id, "check that the new todo is not the same as todo_1"
       assert !new_todo.show_from.nil?, "check that the new_todo is in the tickler to show next month"
 
       assert_equal today + 1.month, new_todo.show_from

test/integration/recurring_todos_test.rb

@@ -29,7 +29,7 @@ class RecurringTodosTest < ActionDispatch::IntegrationTest
     rt.reload                   # then there should be two todos referencing
     assert_equal 2, rt.todos.size
     todo2 = Todo.where(:recurring_todo_id => rt.id, :state => 'active').first
-    assert_not_equal todo2.id, todo.id # and the todos should be different
+    refute_equal todo2.id, todo.id # and the todos should be different
 
     # when I delete the recurring todo
     delete_via_redirect "/recurring_todos/#{rt.id}", :_source_view => 'todo'
@@ -40,4 +40,4 @@ class RecurringTodosTest < ActionDispatch::IntegrationTest
     assert todo.recurring_todo_id.nil?
     assert todo2.recurring_todo_id.nil?
   end
-end
+end

test/models/user_test.rb

@@ -160,9 +160,9 @@ class UserTest < ActiveSupport::TestCase
   end
 
   def test_generate_token_updates_token
-    assert_value_changed @admin_user, :token do
-      @admin_user.send :generate_token
-    end
+    old_token = @admin_user.token
+    @admin_user.generate_token
+    refute_equal old_token, @admin_user.token
   end
 
   def test_find_admin

test/test_helper.rb

@@ -1,6 +1,7 @@
 ENV['RAILS_ENV'] ||= 'test'
 require File.expand_path('../../config/environment', __FILE__)
 require 'rails/test_help'
+require 'securerandom'
 
 # set config for tests. Overwrite those read from config/site.yml. Use inject to avoid warning about changing CONSTANT
 {
@@ -10,6 +11,7 @@ require 'rails/test_help'
   "time_zone" => "Amsterdam"  # force UTC+1 so Travis triggers time zone failures
 }.inject( SITE_CONFIG ) { |h, elem| h[elem[0]] = elem[1]; h }
 
+
 class ActiveSupport::TestCase
   # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order.
   #
@@ -33,24 +35,13 @@ class ActiveSupport::TestCase
     @thursday = Time.zone.local(2008,6,12)
   end
 
-  # Add more helper methods to be used by all tests here...
-  def assert_value_changed(object, method = nil)
-    initial_value = object.send(method)
-    yield
-    assert_not_equal initial_value, object.send(method), "#{object}##{method}"
-  end
   # Generates a random string of ascii characters (a-z, "1 0")
   # of a given length for testing assignment to fields
   # for validation purposes
   #
   def generate_random_string(length)
-    string = ""
-    characters = %w(a b c d e f g h i j k l m n o p q r s t u v w z y z 1\ 0)
-    length.times do
-      pick = characters[rand(26)]
-      string << pick
-    end
-    return string
+    o = [('a'..'z'), ('A'..'Z'), (0..9)].flat_map(&:to_a)
+    (0...length).map { o[rand(o.length)] }.join
   end
 
   def assert_equal_dmy(date1, date2)