Tests are failing on local master and CI

[mremiszewski]

📝 Provide detailed reproduction steps (if any)

Some tests are failing

1. packages\ckeditor5-image\tests\integration.js
   
2. packages\ckeditor5-table\tests\ui\inserttableview.js
   
3. rect-not-in-dom (kind of not specific?) - it may be just a result of previous failures
   

✔️ Expected result

Tests should complete without failure

❌ Actual result

CI is failing because of these errors.

❓ Possible solution

Reproduction steps in comments.

📃 Other details

• Browser: Chrome 109
• OS: Win 11, iOS
• First affected CKEditor version: tested on 35.4 and 36.0

---

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

[mremiszewski]

Reproduction steps:

Image integration. It happens when the browser is not focused.

1. git checkout master && git pull
2. yarn run test --files="image/integration" --watch
3. Open http://localhost:9876/debug.html or similar if your Karma server is running on a different port.
4. Open developer tools (F12) and focus on the console
5. Refresh the page and observe failed test

TableInsert. It also happens when the browser is not focused.

1. git checkout master && git pull
2. yarn run test --files="table/ui/inserttableview" --watch
3. Open http://localhost:9876/debug.html or similar if your Karma server is running on a different port.
4. Open developer tools (F12) and focus on the console
5. Refresh the page and observe failed test
    

[niegowski]

1. yarn run test --files="image/integration" --watch

This can be fixed by adding:

newEditor.ui.focusTracker.isFocused = true;

At the beginning of that test.

[niegowski]

1. yarn run test --files="table/ui/inserttableview" --watch

This one is tricky because it relies on DOM element.focus() and focus event that seems not to get fired if the page is not focused. It could be sth related to the browser change.

[niegowski]

The 2 above tests fixed/disabled in commit 9496e5e.

Seems that the rect-source-not-in-dom is still failing. I'm trying to find where and why.

[niegowski]

Finally, I found what was causing the rect-source-not-in-dom warning. This was happening in the TooltipManager that was trying to pin BalloonPanelView to the BlockToolbar#buttonView and it's handled in the debounced method without checking whether the button is still visible. 

Draft branch (without updated tests for CC): https://github.com/ckeditor/ckeditor5/compare/ck/13347

This all problems are caused by the fact that for an unknown reason, the integration CI is running tests in a blurred Chrome window. Maybe we should try to handle this problem on the CI side by providing --browsers=ChromeHeadless an option to auto tests (this is fixing tests locally).

Apart from the above, I noticed plenty of misc and random errors like this:

ERROR in ./packages/ckeditor5-basic-styles/src/bold/boldui.ts
Module build failed (from ./node_modules/ts-loader/index.js):
Error: TypeScript emitted no output for /Users/kuba/WebstormProjects/ckeditor5/packages/ckeditor5-basic-styles/src/bold/boldui.ts.

[Reinmar]

This all problems are caused by the fact that for an unknown reason, the integration CI is running tests in a blurred Chrome window. Maybe we should try to handle this problem on the CI side by providing --browsers=ChromeHeadless an option to auto tests (this is fixing tests locally).

What do you mean by "option to auto tests"? How does it help locally?

[niegowski]

What do you mean by "option to auto tests"? How does it help locally?

Headless Chrome seems to be focused all the time.

[Reinmar]

Apart from the above, I noticed plenty of misc and random errors like this:

Whenever seeing that error please investigate a bit more into the error that caused this one. As discussed on Monday, @pomek noticed that the original error is also logged. Knowing the root cause of these TS problems would help us understand whether it's related to module augmentation (of which we're aware already) or someting new.

[Reinmar]

What do you mean by "option to auto tests"? How does it help locally?

Headless Chrome seems to be focused all the time.

Interesting. Are we aware of any other differences between real Chrome and Headless Chrome? Perhaps HC would be faster too, making CI jobs shorter (and more stable). But at the same time, if it changes something more and would make tests unrealistic, switching to Headless Chrome would make no sense ;|

[niegowski]

Whenever seeing that error please investigate a bit more into the error that caused this one. As discussed on Monday, @pomek noticed that the original error is also logged. Knowing the root cause of these TS problems would help us understand whether it's related to module augmentation (of which we're aware already) or someting new.

By random, I was talking about happening in different places on different files and after triggering tests multiple times it happened in different places almost every time without any changes in the code).

[niegowski]

Interesting. Are we aware of any other differences between real Chrome and Headless Chrome? Perhaps HC would be faster too, making CI jobs shorter (and more stable). But at the same time, if it changes something more and would make tests unrealistic, switching to Headless Chrome would make no sense ;|

I'm using headless chrome locally to run tests for a few years now and did not notice any changes except its just a focused tab (and this annoying window is not appearing when starting tests).

[pomek]

In the past, we measured the time needed for executing tests using Headless Chrome and Chrome. It's more or less the same. But if we gain more stability, I am all for using HC instead of a standard browser.

[niegowski]

I was trying to fix the "rect" problem but finally, it won't work because TooltipManager depends on ResizeObserver and it does not fire if the tab is in the background. 

I noticed that it's not about the page being focused or blurred but whether it is visible or in the background. Found this comment: karma-runner/karma-chrome-launcher#228 (comment) and tested it on our karma config and the tests started working. Without switching to ChromeHeadless. So this is another option, probably safer.

customLaunchers: {
    CHROME_TRAVIS_CI: {
        base: 'Chrome',
        flags: [ '--no-sandbox', '--disable-background-timer-throttling', '--js-flags="--expose-gc"' ]
    },
    CHROME_LOCAL: {
        base: 'Chrome',
        flags: [ '--disable-background-timer-throttling', '--js-flags="--expose-gc"', '--remote-debugging-port=9222', 
            '--disable-renderer-backgrounding', '--disable-backgrounding-occluded-windows' ]
    }
},

[niegowski]

I also found this: https://github.com/karma-runner/karma-chrome-launcher/blob/master/index.js#L45-L47

Looks like by default, when running tests in Chrome, this flag should be enabled. Maybe we are overriding it? I'm not sure how customLaunchers work. I'm not sure if both flags are needed.

[niegowski]

The 2 above tests fixed/disabled in commit 9496e5e.

Reverted in 2b6a3f2.

[niegowski]

@pomek can we close this issue? Or we should keep it open to keep info about '--disable-renderer-backgrounding', '--disable-backgrounding-occluded-windows' until we add it the karma config?

[pomek]

☝️ it deserves to be extracted into a new issue. This issue is about failing tests. The new case could be about enabling additional flags in tests.

[niegowski]

Extracted: #13407.