Test cases for module browser

[ahmed-mgd]

Fixes #4971

Added test cases for the mod browser's core functionality, as well as a 'TOGGLE_WAIT' constant which will be used more thoroughly for #4929

[Bubballoo3]

Instead of sleeping explicitly here, we should prefer searching for an element with assert_selector and passing the maximum wait time. That way if the actual wait time is less than TOGGLE_WAIT, the test continues as quickly as possible

[ahmed-mgd]

The way the search is implemented, it would be a bit tricky to use assert_selector here. The cards are hidden/shown dynamically and the only reason we have to sleep is because of the 300ms debounce.

ondemand/apps/dashboard/app/javascript/module_browser.js

Line 161 in f57cdcc

moduleSearch.addEventListener('input', debounce(filterModules, 300));

I'm thinking it would be easier to either shorten the sleep to 0.3, or completely get rid of the debouncing since it's a bit redundant. Let me know what you think.

[Bubballoo3]

That makes sense. As a rule, I think we should absolutely try to make the wait as short as we can while still having the tests pass.

IIRC another approach I have seen around here is to have filterModules produce a hidden element when it finishes, specifically so that we have something to set an assert_selector looking for. Of course that would require us to put off merging until 4.1 is released, but could be the best long term approach.

I wouldn't be opposed to removing the debounce, but would be a bit concerned about possible performance implications. If you have a massive number of modules to search through, and execute a search on every letter inputted, I could see how that could back up and run more slowly than if you debounce and get most/all of the search query after only two or three passes. I'd also be interested to hear @johrstrom's thoughts before we try removing it.

[Bubballoo3]

I was looking into this today, and realized that we can use the button classes to detect the opening/closing, and generated the following diff (which I can push if you like, instead of manually doing it). But this shaved off around 5s from testing the whole file (before your last commit).

+  MODULE_BTN_SELECT = '.module-card button[data-bs-toggle="collapse"]'
+
   setup do
     stub_sys_apps
   end
@@ -58,18 +60,18 @@ class ModuleBrowserTest < ApplicationSystemTestCase
   test 'module button expands and collapses the module details' do
     with_modified_env({ OOD_MODULE_FILE_DIR: fixture_dir }) do
       visit module_browser_url
-      
-      first_module = find('.module-card button[data-bs-toggle="collapse"]', match: :first)
+
+      first_module = find(MODULE_BTN_SELECT, match: :first)
       collapse_id = first_module['data-bs-target']
       
       assert_selector("#{collapse_id}", visible: :hidden)
       
       first_module.click
-      sleep TOGGLE_WAIT
+      assert_selector("#{MODULE_BTN_SELECT}.active")
       assert_selector("#{collapse_id}", visible: :visible)
       
       first_module.click
-      sleep TOGGLE_WAIT
+      assert_selector("#{MODULE_BTN_SELECT}.collapsed")
       assert_selector("#{collapse_id}", visible: :hidden)
     end
   end
@@ -78,10 +80,10 @@ class ModuleBrowserTest < ApplicationSystemTestCase
     with_modified_env({ OOD_MODULE_FILE_DIR: fixture_dir }) do
       visit module_browser_url
       
-      first_module = find('.module-card button[data-bs-toggle="collapse"]', match: :first)
+      first_module = find(MODULE_BTN_SELECT, match: :first)
       first_module.click
-      sleep TOGGLE_WAIT
-      
+
+      assert_selector("#{MODULE_BTN_SELECT}.active")
       version_button = find('button[data-role="selectable-version"]', match: :first)
       module_name = version_button['data-module']
       version = version_button['data-version']

I will look to do something similar for the searching, though as I mentioned before I may have to modify the javascript to produce trigger elements. Of course you are welcome to take that if you prefer, but I am happy to finish this up since I have a good idea of what I want them to look like.

[ahmed-mgd]

I will look to do something similar for the searching, though as I mentioned before I may have to modify the javascript to produce trigger elements. Of course you are welcome to take that if you prefer, but I am happy to finish this up since I have a good idea of what I want them to look like.

Sounds good. Feel free to finish it up.