roadmap icon indicating copy to clipboard operation
roadmap copied to clipboard

Refactor Conditional Questions Tests

Open aaronskiba opened this issue 8 months ago • 4 comments

Changes proposed in this PR:

  • https://github.com/DMPRoadmap/roadmap/pull/3516 adds a lot of test coverage within spec/features/questions/conditions_questions_spec.rb and spec/controllers/answers_controller_with_conditional_questions_spec.rb. This PR attempts to refactor those files by applying DRY principles.
  • spec/support/helpers/conditional_questions_helper.rb has been created to facilitate the shared logic between the two aforementioned files.
  • The only actual change is the addition of @non_conditional_questions[:radiobutton][index].id to some remove_data: values. (I am not sure why these :radiobutton options were absent in the first place and could add them back if needed.)

aaronskiba avatar May 06 '25 19:05 aaronskiba

* The only actual change is the addition of `@non_conditional_questions[:radiobutton][index].id` to some `remove_data: ` values. (I am not sure why these `:radiobutton` options were absent in the first place and could add them back if needed.)

Please add. That was a slip up on my part.

johnpinto1 avatar May 07 '25 08:05 johnpinto1

1 Warning
:warning: This PR is too big! Consider breaking it down into smaller PRs.

Generated by :no_entry_sign: Danger

github-actions[bot] avatar May 23 '25 22:05 github-actions[bot]

The tests are getting noticeably faster, but I will try to improve things a bit more.

BEFORE

https://github.com/DMPRoadmap/roadmap/actions/runs/14705500339/job/41264851295

Top 10 slowest example groups:
  AnswersController
    3.95 seconds average (43.5 seconds / 11 examples) ./spec/controllers/answers_controller_with_conditional_questions_spec.rb:5

Top 10 slowest example groups:
  Question::Conditions questions
    10.24 seconds average (92.2 seconds / 9 examples) ./spec/features/questions/conditions_questions_spec.rb:5

AFTER

https://github.com/DMPRoadmap/roadmap/actions/runs/15404832747/job/43345375835?pr=3522 (up to commit 8893f32)

Top 10 slowest example groups:
  AnswersController
    1.98 seconds average (21.78 seconds / 11 examples) ./spec/controllers/answers_controller_with_conditional_questions_spec.rb:5

Top 10 slowest example groups:
  Templates::UpgradeCustomisations
    9.57 seconds average (9.57 seconds / 1 example) ./spec/features/templates/templates_upgrade_customisations_spec.rb:5
  Question::Conditions questions
    9.49 seconds average (85.44 seconds / 9 examples) ./spec/features/questions/conditions_questions_spec.rb:5

aaronskiba avatar Jun 02 '25 23:06 aaronskiba

Commit e951b4d greatly reduces the test time overhead. ./spec/features/questions/conditions_questions_spec.rb is now being executed 3x faster:

Top 10 slowest example groups:
  Question::Conditions questions
    9.36 seconds average (28.09 seconds / 3 examples) ./spec/features/questions/conditions_questions_spec.rb:5

However, the testing approach is a bit different with this change. Here is a copy of the commit description which describes the change:

  • Previously, spec/features/questions/conditions_questions_spec.rb tested remove_data and add_webhook functionality separately for each question type: checkbox, radiobutton, and dropdown. While thorough, this approach added a lot of overhead to test execution time. This change reduces that overhead by randomly choosing only one of checkbox, radiobutton, or dropdown for each of the tests. Over time, repeated executions via the GitHub Actions should ensure that all types continue to be exercised, maintaining coverage while also improving overall test time.

aaronskiba avatar Jun 04 '25 16:06 aaronskiba