Improve Units tests and remove test parallelization

Parallelization produces errors (#92)

test/application_system_test_case.rb

@@ -17,11 +17,13 @@ class ApplicationSystemTestCase < ActionDispatch::SystemTestCase
     options.add_preference('browser.download.dir', "#{Rails.root}/tmp/")
   end
 
-  def sign_in(user: users.select(&:confirmed?).sample, password: randomize_user_password!(user))
+  def sign_in(user: users.select(&:confirmed?).sample,
+              password: randomize_user_password!(user))
     visit new_user_session_url
     fill_in User.human_attribute_name(:email), with: user.email
     fill_in User.human_attribute_name(:password), with: password
     click_on t(:sign_in)
+    yield if block_given?
     user
   end
 

test/system/units_test.rb

@@ -1,33 +1,36 @@
 require "application_system_test_case"
 
 # Fixture prerequisites:
-#  * user with multiple units + subunits
+#  * user with multiple units (at least 1 w/o subunit) + subunits,
-#  * user with single unit
+#  * user with single unit,
-#  * user with no units
+# FIXME: add confirmed user without units
+#  * user with no units.
+# Users need to be active and confirmed.
 
 class UnitsTest < ApplicationSystemTestCase
-  LINK_LABELS = {}
+  def sign_in(...)
+    @link_labels = nil
+    @user = super { click_on t('units.navigation') }
+  end
 
-  setup do
+  def link_labels
-    @user = sign_in
+    @link_labels ||= {
-
+      new_unit:     t('units.index.new_unit'),
-    LINK_LABELS.clear
+      new_subunit:  t('units.unit.new_subunit'),
-    LINK_LABELS[:new_unit] = t('units.index.new_unit')
+      edit:         Regexp.union(@user.units.map(&:symbol))
-    LINK_LABELS[:new_subunit] = t('units.unit.new_subunit')
+    }
-    LINK_LABELS[:edit] = Regexp.union(@user.units.map(&:symbol))
-
-    visit units_path
   end
 
   test "index" do
-    # Wait for the table to appear first, only then check row count
+    sign_in
+    # Wait for the table to appear first, only then check row count.
     within 'tbody' do
       assert_selector 'tr', count: @user.units.count
     end
 
-    # Cannot #destroy_all due to {dependent: :restrict*} on Unit.subunits association
+    # Cannot #destroy_all due to {dependent: :restrict*} on Unit.subunits association.
     @user.units.delete_all
-    visit units_path
+    refresh
     within 'tbody' do
       assert_selector 'tr', count: 1
       assert_text t('units.no_items')
@@ -35,7 +38,9 @@ class UnitsTest < ApplicationSystemTestCase
   end
 
   test "new and create" do
-    type, label = LINK_LABELS.assoc([:new_unit, :new_subunit].sample)
+    sign_in
+    link_labels.slice!(:new_unit, :new_subunit)
+    type, label = link_labels.to_a.sample
     new_link = all(:link, exact_text: label).sample
     new_link.click
     assert_equal 'disabled', new_link[:disabled]
@@ -49,7 +54,7 @@ class UnitsTest < ApplicationSystemTestCase
       end
       values = {
         symbol: random_string(deep_rand(1..3, 4..maxlength['unit[symbol]']),
-                              except: units.map(&:symbol), allow_blank: false),
+                              except: @user.units.map(&:symbol), allow_blank: false),
         description: random_string(rand(0..maxlength['unit[description]']))
       }.with_indifferent_access
       within :field, 'unit[multiplier]' do |field|
@@ -70,7 +75,7 @@ class UnitsTest < ApplicationSystemTestCase
       assert_selector 'tr', count: @user.units.count
     end
     assert_no_selector :element, :a, 'disabled': 'disabled',
-      exact_text: Regexp.union(LINK_LABELS.fetch_values(:new_unit, :new_subunit))
+      exact_text: Regexp.union(link_labels.values)
     assert_equal values, Unit.last.attributes.slice(*values.keys)
   end
 
@@ -99,23 +104,24 @@ class UnitsTest < ApplicationSystemTestCase
   end
 
   test "new and edit on validation error" do
-    # It's not possible to cause validation error on :edit with single unit
+    sign_in
-    LINK_LABELS.delete(:edit) unless @user.units.count > 1
+    # It's impossible to cause validation error on :edit with single unit.
-    type, label = LINK_LABELS.to_a.sample
+    link_labels.delete(:edit) unless @user.units.many?
+    type, label = link_labels.to_a.sample
     link = all(:link, exact_text: label).sample
     link.click
 
     get_values = -> { all(:field).map { |f| [f[:name], f[:value]] }.to_h }
     values = nil
     within 'tbody > tr:has(input[type=text])' do
-      # Provide duplicate :symbol as input invalidatable server side
+      # Provide duplicate :symbol as input invalidatable server side.
       fill_in 'unit[symbol]',
         with: (@user.units.map(&:symbol) - [find_field('unit[symbol]').value]).sample
       values = get_values[]
       send_keys :enter
     end
 
-    # Wait for flash before checking link :disabled status
+    # Wait for flash before checking link :disabled status.
     assert_selector '.flash.alert'
     if type == :edit
       assert_no_selector :link, exact_text: link[:text]
@@ -129,15 +135,18 @@ class UnitsTest < ApplicationSystemTestCase
   end
 
   test "new and edit allow opening multiple forms" do
-    # Requires at least 1 unit to be able to open 2 forms
+    # Require at least 1 unit to be able to open 2 forms.
-    links = LINK_LABELS.transform_values { |labels| all(:link, exact_text: labels).to_a }
+    sign_in(user: users.select { |u| u.confirmed? && u.units.any? }.sample)
+    links = link_labels.transform_values do |labels|
+      all(:link, exact_text: labels).to_a
+    end
     random_link = ->{ links.transform_values(&:sample).compact.to_a.sample }
-    # Define tr count change depending on link clicked
+    # Define <tr> count change depending on link clicked.
     tr_diff = {new_unit: 1, new_subunit: 1, edit: 0}
 
     type, link = random_link[].tap { |t, l| links[t].delete(l) }
     subunit_link = link.ancestor('tr')
-      .first(:link, LINK_LABELS[:new_subunit], between: 0..1) if type == :edit
+      .first(:link, link_labels[:new_subunit], between: 0..1) if type == :edit
     assert_difference ->{ all('tbody tr').count }, tr_diff[type] do
       assert_difference ->{ all('tbody tr:has(input[type=text])').count }, 1 do
         link.click
@@ -162,12 +171,14 @@ class UnitsTest < ApplicationSystemTestCase
     assert_not_equal form, find('tbody tr:has(input:focus)')
   end
 
-  #test "edit" do
+  test "edit" do
     # NOTE: Check if displayed attributes match record
-  #end
+    assert true
+  end
 
   # NOTE: extend with any add/edit link
-  test "close new unit form with escape key" do
+  test "new form closes on escape key" do
+    sign_in
     click_on t('units.index.new_unit')
     first('tbody > tr').all(:field).sample.send_keys :escape
     within 'tbody' do
@@ -176,7 +187,8 @@ class UnitsTest < ApplicationSystemTestCase
   end
 
   # NOTE: extend with any add/edit link
-  test "close and reopen new unit form" do
+  test "new form can be reopened after close" do
+    sign_in
     click_on t('units.index.new_unit')
     within 'tbody' do
       find(:link_or_button, exact_text: t(:cancel)).click
@@ -186,13 +198,19 @@ class UnitsTest < ApplicationSystemTestCase
     assert_selector 'tbody > tr:has(input, textarea)'
   end
 
+  test "rebase" do
+    # TODO
+    assert true
+  end
+
   test "destroy" do
+    sign_in(user: users.select { |u| u.confirmed? && u.units.any? }.sample)
     link = all(:link_or_button, exact_text: t('units.unit.destroy')).sample
-    label = link.ancestor('tr').first(:link)[:text]
+    symbol = link.ancestor('tr').first(:link).text
     assert_difference ->{ @user.units.count }, -1 do
       link.click
     end
     assert_selector 'tbody tr', count: [@user.units.count, 1].max
-    assert_selector '.flash.notice', text: t('units.destroy.success', unit: label)
+    assert_selector '.flash.notice', text: t('units.destroy.success', unit: symbol)
   end
 end

test/system/users_test.rb

@@ -1,5 +1,6 @@
 require "application_system_test_case"
 
+# TODO: review #visit usage and replace with #click_on where applicable
 class UsersTest < ApplicationSystemTestCase
   setup do
     @admin = users(:admin)

test/test_helper.rb

@@ -3,9 +3,6 @@ require_relative "../config/environment"
 require "rails/test_help"
 
 class ActiveSupport::TestCase
-  # Run tests in parallel with specified workers
-  parallelize(workers: :number_of_processors)
-
   # Setup all fixtures in test/fixtures/*.yml for all tests in alphabetical order.
   fixtures :all