Table flaky/failing tests fix #4458
Conversation
|
8e326b6 to
2a835c9
Compare
| @@ -12,7 +12,7 @@ setConfig(conf); | |||
| const config = getConfig(); | |||
|
|
|||
| describe('table and tablemetadata', () => { | |||
| beforeEach(() => { | |||
| before(() => { | |||
There was a problem hiding this comment.
To me this looks like it builds up test-state and is an anti-pattern. If we depend on whatever one test has done - it's likely wrong.
Maybe we should just wipe the innerHTML and recreate the markup on each run - to properly isolate each test?
There was a problem hiding this comment.
I think it should stay a beforeEach and l7 should be moved into the beforeEach block
There was a problem hiding this comment.
PS: If the tests already been written in a way, that they depend on whatever the test before has done - Good luck 😆
(Might not be worth the hassle in this case to do a refactoring of the whole file, that can be pretty time consuming)
There was a problem hiding this comment.
The problem with beforeEach here is that it initializes every table before each test and if we don't have a check in the table.js if the events have been added then it will add them again and cause bad tests not because of the tests but because of table.js attaching a listener again, so either we add the check in table.js to not attach listeners if already attached or change how we do this specific test like i did in this commit.
PS: moving l7 to beforeEach won't help 😅
There was a problem hiding this comment.
What if there are multiple table blocks on a page - wouldn't that also lead to the table block attaching multiple listeners? Maybe the tests hint at a (real) issue here actually 😁
Might be good to check, otherwise I think it's not worth spending too much time on the unit tests unless you have a lot of free time on your hand
There was a problem hiding this comment.
Tried adding 2 tables on 1 page and added a console log but it only attached 2 times https://table-flaky-tests-fix--milo--adobecom.aem.page/drafts/dusan/check-multiple-tables.
|
This pull request is not passing all required checks. Please see this discussion for information on how to get all checks passing. Inconsistent checks can be manually retried. If a test absolutely can not pass for a good reason, please add a comment with an explanation to the PR. |
e72dd19 to
08490a9
Compare
Fixes flaky/failing table tests introduced in #4374
Test URLs: