feat(tests): Add sample tests #191
Conversation
…nality - move extractObject function to tests/utils
…ion page and sample class with type annotations
GuiLeme
left a comment
GuiLeme
left a comment
There was a problem hiding this comment.
Besides those comments I left, one thing I notice was that some tests were failing here because it wasn't supporting interaction with multiple plugins (For example: the actions-button-dropdown - It hasn't got the expected structure, because there were multiple extensible areas from plugins there)
One suggestion I would make is (This could be done in another PR, just let me know):
- In the
beforeAllfunction, you could insert another callback that verifies if there are other plugins within the test meeting (just need to check the log for loaded plugins) - If it's not possible to add more callbacks to thebeforeAllfunction, you can create abeforeAllWrapperthat will run thecheckPluginAvailabilityand the newverifyOnlyPluginInMeetingfunctions in it - That way, the tester will know fast enough what to do (just remove the current installed plugin frombbb-web.properties).
| expect(urlsJson?.text, 'should log the correct TEXT URL for the second slide').toBeDefined(); | ||
| expect(urlsJson?.text, 'TEXT URL should be a valid URL').toMatch(/^https?:\/\/.+/); | ||
|
|
||
| await sampleTest.modPage.page.locator(e.skipSlide).selectOption({ label: 'Slide 15' }); // last slide |
There was a problem hiding this comment.
This line hardcodes the slide number assuming that the default PDF coming to the whiteboard is the same as the one that comes by default when installing BBB. In my case, that was not true, in fact, I had a 4 slides PDF.
I suggest to change this to the number of pages from default.pdf accessible via https://<bbb-base-domain>/default.pdf (Then you are going to have to use the pdfjs-dist or pdf-lib to get it), or if you have access to the graphql via browser console, you can get it via subscription.
There was a problem hiding this comment.
BBB automated tests start from the premise that "the testing environment is set with default settings". So it's expected to have the default.pdf loaded in the session where the test is running. Using any lib to check a file or something like that will add unnecessary complexity that we do not aim for in tests: they must be reliable and strictly context-scoped.
to fit the changes you suggested, I propose:
- instead of going to "slide 15", go to the second slide
- in case the "next slide" button is disabled, we assume the current presentation has a single page (a presentation with more pages is required to validate the plugin's feature) and fail or skip the test with a descriptive log.
There was a problem hiding this comment.
Hm, I see. So the idea of going for the 15th slide to test if the button doesn't appear is pretty good and covers all scenarios - I wouldn't mind letting it there, knowing the premise you mentioned. However, it would be interesting for the tester, or developer that is testing the code, to know why the fail happened, so for that I would suggest, if possible:
- Check if the 15th slide is there, if so, proceed with the test, if not, then log an error saying that the presentation is likely not the one that comes in default installations of BBB (And ask the dev to use it so the test runs properly);
If that's not a valid idea, we can:
- Change the error log (when the test fails in that specific selector) to warn the developer that the presentation they have is likely not that of the default installation - That could possibly be done with a try/catch (?);
There was a problem hiding this comment.
now I got what you are saying. 86b947e addresses your suggestions. it dynamically gets the last slide (also checking before if the presentation has more than 1 slide - required for testing the sample plugin)
… newline at end of files - also enable formatOnSave
…quired slides and dynamically set last one for testing
- applied on actionButtonDropdown sample
the actual issue was 2 plugins injecting separator elements, which previously had the same Note: It makes a lot of sense for these types of failures to be addressed, as we expect all plugin tests to run properly, regardless of how many others are configured in the session. |
GuiLeme
left a comment
GuiLeme
left a comment
There was a problem hiding this comment.
Changes look solid to me! Let's wait for the merge and then bump SDK's version and tag (so we update the CORE)
3 participants
What does this PR do?
Adds test for the following samples:
sample-audio-settings-dropdown-pluginsample-camera-settings-dropdown-pluginsample-custom-subscription-hooksample-data-channel-pluginMore:
Fixes Typescript errors and enhances types