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
Reenable block editing UI tests #1147
Conversation
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.
Nice, finally getting the last tests back and running.
I did find a few potential problems though.
76ebe39
to
a35567f
Compare
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.
The last commit I didn't review in detail, as thats mostly moving around a bunch of code. I found a couple things and resolved most of my previous comments. I will have another go at this PR once that other one is merged, and this is rebased (as u susgested)
This comment was marked as resolved.
This comment was marked as resolved.
This replaces the `nth()` locator with more specific ones. Using `nth()` is apparently discouraged and indeed lead to some flaky behaviour in the block editing tests. The buttons are now located by using specific unique locators. Blocks containing only text content like title and text blocks can be located by using a regex filter, while the video and series blocks can be located by combining a filter that checks the presence of specific text with a filter that checks for the absence of other specific text (that is contained in another block). However, we do need to make sure that the test steps are performed in a set order: If the text whose presence is checked is removed in a test step, that step needs to be the very last one of that specific block. Otherwise the locator will fail. This also incidentally inlines all the block editing instead of having one large function for it. I do maintain that the function was nicer, but with these new locators it has gotten somewhat pointless.
Since all functions and structures were either moved to more specific files or removed completely, the only thing this file did was reimport/export the test function for convenience. That can now be imported directly in the test files.
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.
Sorry for the massive delay in reviewing this! I finally looked over everything again and looks good to me! Good work. Lets finally get this merged!
These were previously skipped because they were unreliable and failed often. With the changes from #1093, all tests got a lot more stable, but the old block tests needed adjustments.