-
-
Notifications
You must be signed in to change notification settings - Fork 19
Refactor UI tests for tm_t_events* modules #1443
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
Conversation
|
|
||
| testthat::expect_equal( | ||
| app_driver$get_text("#teal-teal_modules-active_tab .active > a"), | ||
| app_driver$get_text("#teal-teal_modules-active_module_id * .teal-modules-tree * .nav-link"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@llrs-roche
What do you think about this change that I made on other PR in similar place?
https://github.com/insightsengineering/teal.modules.clinical/pull/1441/files#diff-eea16ad04c22c49466b528d79edebfcb683d3f6f2a255581e007bcc8534c9e9cR65
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only realized of this later and used it in a different PR but I wanted to ask for your feedback.
I like that this one uses the id but yours is shorter which is also a plus.
Maybe the one with id is more tightly coupled and works better as a test if the UI changes but I let you decide
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@llrs-roche my preference is the shorter the better : P
once you change this. you can merge to my branch
Unit Tests Summary 1 files ± 0 71 suites ±0 2h 20m 32s ⏱️ + 2h 19m 10s For more details on these failures and errors, see this check. Results for commit 06cfc20. ± Comparison against base commit d200e5f. ♻️ This comment has been updated with latest results. |
Unit Test Performance DifferenceTest suite performance difference
Results for commit 9f6e8f7 ♻️ This comment has been updated with latest results. |
Pull Request
Part of #1440
Companion of #1441