Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cds/reduce assertions per test #2563

Merged
merged 14 commits into from
Mar 6, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .rubocop.yml
Original file line number Diff line number Diff line change
Expand Up @@ -73,9 +73,11 @@ Metrics/ParameterLists:
- app/models/jupiter_core/search.rb
- app/models/jupiter_core/solr_services/deferred_faceted_solr_query.rb

# TODO: our tests are quite smelly. We should be working to get this lower!
Minitest/MultipleAssertions:
Max: 15 # default 3
Exclude:
- test/system/admin_users_show_test.rb
- test/system/search_test.rb

Naming/FileName:
Exclude:
Expand Down
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ New entries in this file should aim to provide a meaningful amount of informatio

## Unreleased

### Changed
- Refactored tests into smaller tests [PR#2563](https://github.com/ualbertalib/jupiter/pull/2563)

## [2.4.4] - 2023-02-17
### Added
- Push collections and communities to preservation on save along with a rake task to do so [#255](https://github.com/ualbertalib/pushmi_pullyu/issues/255)
Expand Down
536 changes: 0 additions & 536 deletions test/integration/oaisys_list_identifiers_test.rb

This file was deleted.

110 changes: 0 additions & 110 deletions test/integration/oaisys_list_sets_test.rb

This file was deleted.

4 changes: 3 additions & 1 deletion test/integration/site_for_bots_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,7 @@ class SiteForBotsTest < ActionDispatch::IntegrationTest
assert_select "a[rel='nofollow']", text: @item.subject.first
end

test 'structured data for google scholar' do
test 'structured thesis data for google scholar' do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

# TODO: can be a lot more complicated see https://scholar.google.com/intl/en/scholar/inclusion.html#indexing
get item_path @thesis
assert_select "meta[name='citation_title'][content='#{@thesis.title}']"
Expand All @@ -125,7 +125,9 @@ class SiteForBotsTest < ActionDispatch::IntegrationTest
))
assert_select "meta[name='dc.identifier'][content='#{@item.doi}']"
assert_select "meta[name='citation_doi'][content='#{@item.doi}']"
end

test 'structured item data for google scholar' do
get item_path @item
assert_select "meta[name='citation_title'][content='#{@item.title}']"
@item.creators.each do |author|
Expand Down
20 changes: 20 additions & 0 deletions test/jobs/update_user_activity_job_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
require 'test_helper'

class UpdateUserActivityJobTest < ActiveSupport::TestCase

test 'should update the user activity (last seen at and IP) through the job' do
user = users(:user_admin)
ip = '4.73.73.73'
now = Time.now.utc.to_s
UpdateUserActivityJob.perform_now(user.id, now, ip)
user.reload
assert_equal user.last_seen_at.to_s, now
assert_equal user.last_seen_ip, ip
# Still does not change sign-in information
assert_predicate user.last_sign_in_at, :blank?
assert_predicate user.last_sign_in_ip, :blank?
assert_predicate user.previous_sign_in_at, :blank?
assert_predicate user.previous_sign_in_ip, :blank?
end

end
72 changes: 18 additions & 54 deletions test/models/user_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,83 +23,47 @@ class UserTest < ActiveSupport::TestCase
assert_equal "User:#{user.id}", user.flipper_id
end

# rubocop:disable Minitest/MultipleAssertions
# TODO: our tests are quite smelly. This one needs work!
test 'should update the activity columns when not signing-in' do
user = users(:user_regular)
test 'should update the activity columns when not signing-in for new user without activity update' do
user = users(:user_admin)
assert_predicate user.last_seen_at, :blank?
assert_predicate user.last_sign_in_at, :blank?
assert_predicate user.previous_sign_in_at, :blank?
end

test 'should update the activity columns when not signing-in' do
pgwillia marked this conversation as resolved.
Show resolved Hide resolved
user = users(:user_regular)

ip1 = '4.26.50.50'
now1 = Time.now.utc.to_s
ip = '4.26.50.50'
now = Time.now.utc.to_s

user.update_activity!(now1, ip1)
user.update_activity!(now, ip)
user.reload

assert_predicate user.last_seen_at, :present?
assert_equal user.last_seen_at.to_s, now1
assert_equal user.last_seen_ip, ip1
assert_equal user.last_seen_at.to_s, now
assert_equal user.last_seen_ip, ip
# Does not change sign-in information
assert_predicate user.last_sign_in_at, :blank?
assert_predicate user.last_sign_in_ip, :blank?
assert_predicate user.previous_sign_in_at, :blank?
assert_predicate user.previous_sign_in_ip, :blank?

travel 1.hour do
ip2 = '4.73.73.73'
now2 = Time.now.utc.to_s
assert_not_equal now2, now1
UpdateUserActivityJob.perform_now(user.id, now2, ip2)
user.reload
assert_equal user.last_seen_at.to_s, now2
assert_equal user.last_seen_ip, ip2
# Still does not change sign-in information
assert_predicate user.last_sign_in_at, :blank?
assert_predicate user.last_sign_in_ip, :blank?
assert_predicate user.previous_sign_in_at, :blank?
assert_predicate user.previous_sign_in_ip, :blank?
end
end
# rubocop:enable Minitest/MultipleAssertions

# rubocop:disable Minitest/MultipleAssertions
# TODO: our tests are quite smelly. This one needs work!
test 'should update the activity columns when signing-in' do
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to above this could be split into three tests

  1. new user, never signed in (but don't need to write a duplicate test)
  2. updates last seen and last signed in update_activity
  3. do we need the time travel? That's just the same as the second test, right?

user = users(:user_regular)
assert_predicate user.last_seen_at, :blank?
assert_predicate user.last_sign_in_at, :blank?
assert_predicate user.previous_sign_in_at, :blank?

ip1 = '4.26.50.50'
now1 = Time.now.utc.to_s
ip = '4.26.50.50'
now = Time.now.utc.to_s

user.update_activity!(now1, ip1, sign_in: true)
user.update_activity!(now, ip, sign_in: true)
user.reload
assert_predicate user.last_seen_at, :present?
assert_equal user.last_seen_at.to_s, now1
assert_equal user.last_seen_ip, ip1
assert_equal user.last_sign_in_at.to_s, now1
assert_equal user.last_sign_in_ip, ip1
assert_equal user.last_seen_at.to_s, now
assert_equal user.last_seen_ip, ip
assert_equal user.last_sign_in_at.to_s, now
assert_equal user.last_sign_in_ip, ip
assert_predicate user.previous_sign_in_at, :blank?
assert_predicate user.previous_sign_in_ip, :blank?

travel 1.hour do
ip2 = '4.73.73.73'
now2 = Time.now.utc.to_s
assert_not_equal now2, now1

user.update_activity!(now2, ip2, sign_in: true)
user.reload
assert_equal user.last_seen_at.to_s, now2
assert_equal user.last_seen_ip, ip2
assert_equal user.last_sign_in_at.to_s, now2
assert_equal user.last_sign_in_ip, ip2
assert_equal user.previous_sign_in_at, now1
assert_equal user.previous_sign_in_ip, ip1
end
end
# rubocop:enable Minitest/MultipleAssertions

test 'should validate if it does not have an api key and it is not a system account' do
user = User.new(
Expand Down
Loading