From bb8fd0868582af8b40661a67450da9e55696c6fd Mon Sep 17 00:00:00 2001 From: Matt Rogers Date: Wed, 17 Oct 2018 21:22:13 -0500 Subject: [PATCH 1/4] Remove the `assert_value_changed` helper It's only used in one place and it's easy enough to rewrite this into something more straightforward and less clever. --- test/models/user_test.rb | 6 +++--- test/test_helper.rb | 6 ------ 2 files changed, 3 insertions(+), 9 deletions(-) diff --git a/test/models/user_test.rb b/test/models/user_test.rb index 20188647..be5b3320 100644 --- a/test/models/user_test.rb +++ b/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 + assert_not_equal old_token, @admin_user.token end def test_find_admin diff --git a/test/test_helper.rb b/test/test_helper.rb index 1bcfcdb4..d0e82d8f 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -33,12 +33,6 @@ 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 From dea3b1b58e8d23d70a074470c13fc8c649fc7c65 Mon Sep 17 00:00:00 2001 From: Matt Rogers Date: Fri, 19 Oct 2018 09:25:15 -0500 Subject: [PATCH 2/4] Update to a more performant random string implementation Since `SecureRandom.alphanumeric` is Ruby 2.5 only, we can't use that for now. Implement a new version until we can get Tracks updated to that version. --- test/test_helper.rb | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/test/test_helper.rb b/test/test_helper.rb index d0e82d8f..9c5aa0ac 100644 --- a/test/test_helper.rb +++ b/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. # @@ -38,13 +40,8 @@ class ActiveSupport::TestCase # 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) From 71c95c0d014dd7431eaa5dca7028bc35d539a1fa Mon Sep 17 00:00:00 2001 From: Matt Rogers Date: Fri, 19 Oct 2018 11:14:14 -0500 Subject: [PATCH 3/4] Switch `assert_not_equal` to `refute_equal` Preferring to use minitest methods here instead of test-unit. --- test/controllers/notes_controller_test.rb | 2 +- test/controllers/todos_controller_test.rb | 6 +++--- test/integration/recurring_todos_test.rb | 4 ++-- test/models/user_test.rb | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/test/controllers/notes_controller_test.rb b/test/controllers/notes_controller_test.rb index 9c14d236..828d4e7a 100644 --- a/test/controllers/notes_controller_test.rb +++ b/test/controllers/notes_controller_test.rb @@ -25,7 +25,7 @@ class NotesControllerTest < ActionController::TestCase note = users(:admin_user).notes.first - assert_not_equal "test", note.body + refute_equal "test", note.body post :update, id: note.id, note: {body: "test"}, format: :js assert_equal "test", note.reload.body end diff --git a/test/controllers/todos_controller_test.rb b/test/controllers/todos_controller_test.rb index ec203975..1f23616c 100644 --- a/test/controllers/todos_controller_test.rb +++ b/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 diff --git a/test/integration/recurring_todos_test.rb b/test/integration/recurring_todos_test.rb index bad00387..2aa57143 100644 --- a/test/integration/recurring_todos_test.rb +++ b/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 \ No newline at end of file +end diff --git a/test/models/user_test.rb b/test/models/user_test.rb index be5b3320..6e20d6a4 100644 --- a/test/models/user_test.rb +++ b/test/models/user_test.rb @@ -162,7 +162,7 @@ class UserTest < ActiveSupport::TestCase def test_generate_token_updates_token old_token = @admin_user.token @admin_user.generate_token - assert_not_equal old_token, @admin_user.token + refute_equal old_token, @admin_user.token end def test_find_admin From edd0559da1a1cd9d9a167477be8e4b7bf4349778 Mon Sep 17 00:00:00 2001 From: Matt Rogers Date: Fri, 19 Oct 2018 11:32:22 -0500 Subject: [PATCH 4/4] Fix the mixed-indent in NotesControllerTest --- test/controllers/notes_controller_test.rb | 25 +++++++++++------------ 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/test/controllers/notes_controller_test.rb b/test/controllers/notes_controller_test.rb index 828d4e7a..88fafa14 100644 --- a/test/controllers/notes_controller_test.rb +++ b/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 - refute_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