Add shinytest2 tests for bslib

[llrs-roche]

Pull Request

Fixes #651

[averissimo]

Tests seem ok! Great work finding the selectors.

Check out the comments, some of them can be transposed to MAE

[llrs-roche]

The selectors was thanks to Vedha guidance.
I have addressed most if not all of the minor comments but I'll ping you after testing for non empty elements and adding a new filter. Thanks for the review!

[github-actions]

Unit Tests Summary

  1 files   31 suites   28s ⏱️
384 tests 369 ✅ 15 💤 0 ❌
836 runs  821 ✅ 15 💤 0 ❌

Results for commit e6898e8.

♻️ This comment has been updated with latest results.

[github-actions]

Unit Test Performance Difference

Test Suite	$Status$	Time on main	$±Time$	$±Tests$	$±Skipped$	$±Failures$	$±Errors$
shinytest2_DataframeFilteredDataset	👶		$+0.25$	$+14$	$+14$	$0$	$0$
shinytest2_MAEFilteredDataset	👶		$+0.02$	$+1$	$+1$	$0$	$0$

Additional test case details

Test Suite	$Status$	Time on main	$±Time$	Test Case
shinytest2_DataframeFilteredDataset	👶		$+0.01$	Clicking_add_and_selecting_a_variable_adds_the_card_for_a_given_variable
shinytest2_DataframeFilteredDataset	👶		$+0.01$	Clicking_add_button_on_the_datasets_shows_add_filter_panel
shinytest2_DataframeFilteredDataset	👶		$+0.01$	Expanding_a_card_shows_filter_choices.
shinytest2_DataframeFilteredDataset	👶		$+0.01$	Remove_all_filters_button_removes_all_cards_for_all_datasets.
shinytest2_DataframeFilteredDataset	👶		$+0.01$	Remove_datasets_filters_removes_all_cards.
shinytest2_DataframeFilteredDataset	👶		$+0.01$	Remove_filter_button_removes_a_specific_filter_card
shinytest2_DataframeFilteredDataset	👶		$+0.03$	Toggle_button_shows_and_hide_Active_Filter_Summary_panel
shinytest2_DataframeFilteredDataset	👶		$+0.04$	Toggle_button_shows_and_hide_Filter_Data_panel
shinytest2_DataframeFilteredDataset	👶		$+0.03$	datasets_passed_to_filter_data_creates_Active_Filter_Summary_panel_for_all_of_them
shinytest2_DataframeFilteredDataset	👶		$+0.01$	datasets_passed_to_filter_data_creates_Filter_Data_panel_for_all_of_them
shinytest2_DataframeFilteredDataset	👶		$+0.01$	exclude_varnames_limits_choices_in_add_dropdown
shinytest2_DataframeFilteredDataset	👶		$+0.03$	include_varnames_limits_choices_in_add_dropdown
shinytest2_DataframeFilteredDataset	👶		$+0.01$	teal_slice_objects_pass_to_filter_data_displays_filter_unfiltered_counts_in_Active_Filter_Summary
shinytest2_DataframeFilteredDataset	👶		$+0.01$	teal_slice_objects_pass_to_filter_data_initializes_filter_cards_for_each_teal_slice
shinytest2_MAEFilteredDataset	👶		$+0.02$	Active_filter_summary_has_all_the_experiments.

Results for commit d8b4dbd

♻️ This comment has been updated with latest results.

[llrs-roche]

This PR is close to 100% as it:

• Tests removing filters
• Tests expand/hide filters:
  • Dataset specific filters
  • All datasets
  • Data summary
• Tests adding filters (these are the last tests on the test-shinytest2 files):
  Test are implemented to 90% but when adding a new filter it only lets you select the variable of the dataset. But once it is selected the filter isn't actually added the UI and there are no options to configure the filter. This can be also seen on the teal.gallery: https://genentech.shinyapps.io/nest_basic-teal_stable/
  Running git bisect didn't help to identify when it was broken (but that could be faulty and it could be a false negative). I think adding a new filter is handled by srv_add on https://github.com/insightsengineering/teal.slice/blob/main/R/FilterStates.R#L441 which doesn't explicitly require to configure the filter added, or actually updates the state of the filter if I read the code correctly. Related changes that could affect how the reactivity is handled was merged recently on Fix reactivity #645 or it might be due to a dependency, probably teal.widgets (or the latest shiny update?).

[llrs-roche]

Thanks for the feedback . I've redacted again the descriptions I hope they are descriptive enough .

Adding screenshot tests increased the package size a lot so I decided against it. But I see your point about regression tests and UI test. When similar concerns on teal.transform were raised Vedha suggested only testing the interactivity/elements: insightsengineering/teal.widgets#298 (comment)

[gogonzo]

Hard to say what tests should cover. There is no description in the issue nor in the PR initial comment. I can't approve PR if I don't know what is the expected coverage.

Besides, tests description are misleading or ambiguous. Test are very selective and seem to lack a concrete objective.

[llrs-roche]

As per our discussions the tests were rewritten from scratch following the TOC we elaborated:

General comments:

• I modified local_app_driver on the data.frame case so that filters are accepted and make it granular
• Rename files according to the classes used.
• Bug discovered [Bug]: Slices cannot be added #655

DF:

• 0. Toggle button shows and hide
  • 0.1: Active Filter Summary panel: check if it is visible or not
  • 0.2: Filter Data panel: check if it is visible or not
• 1. datasets passed to filter data (with the set_filter_state)
  • 1.1: creates Active Filter Summary panel for all of them: check that iris and mtcars is on the Data Name column (1 expect)
  • 1.2: creates Filter Data panel for all of them: check that iris and mtcars have a panel.
• 2. teal_slice objects pass to filter data (with the set_filter_state)
  • 2.1: displays filter/unfiltered counts in Active Filter Summary
  • 2.2: initializes filter_cards for each teal_slice: parse filter data panel for species, cyl and mpg (just name)
• 3. Clicking add button on the datasets shows add filter panel
• 4. Clicking add and selecting a variable adds the card for a given variable
• 5. include_varnames limits choices in add dropdown module for specified datasets minus variables that are already active: iris dropdown only has Species and Sepal.Length choices
  NEW: exclude_varnames limits choices in add dropdown module for specified datasets minus variables that are already active
• 6. Remove filter button removes a specific filter card
• 7. Remove datasets filters removes all cards except the locked one.
• 8. Remove all filters button removes all cards for all datasets.
• 9. Expanding a card shows filter choices except the locked one.

MAE:

• 0. Active filter summary has all the experiments.

[llrs-roche]

Updated per feedback on comments. I removed all the keep_na and keep_inf of teal_slice.

[gogonzo]

👍 Please merge once CICD is ready