-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
It looks like memory leaks tests are broken again #1731
Comments
To mitigate the issue for CI it make sense to temporarily use Chrome 73.x until this issue is resolved. Meanwhile this issue should be fixed asap. Once done we'll restore the latest Chrome version on our CI. |
https://docs.travis-ci.com/user/chrome
So we need to find an image (archive), download it on CI and install Chrome from it. And this change must be done in all repositories. |
All right so ignoring the test sounds like a simpler idea, thanks for checking that. |
My findings so far:
I have a weird feeling that either Chrome messed something because I couldn't find any obvious leak (like detached event listeners). Also some retainers looks like webpack-related stuff - but again I didn't find anything that I'd be 100% sure about. |
…74.*`. See [ckeditor/ckeditor5#1731](ckeditor/ckeditor5#1731) for more details.
Ok, for the time being the test were ignored with ckeditor/ckeditor5-core#172. |
@jodator for the time being please prioritize #1490 over this issue for the time being. We'd like to have this feature as soon as possible, while issue with Chrome's gc API already took some time and we would be able to resume to it once #1490 is ready for the review. It's ok to push it to iteration 25th if we can't do it within this iteration. |
I was investigating this issue for a while and I have mixed feelings. I've started with Chrome Anyway my observation are:
I'm holding this issue to the end of iteration and will get to this later. |
@Reinmar, unfortunately, it looks like the memory tests are still unreliable. I'm not sure if we're going to be able to have something stable enough for automatic testing. |
I've revived the tests on |
It would be nice to use |
The first occurence of
master-revision
CI started to fail today: https://travis-ci.org/ckeditor/ckeditor5-editor-classic/jobs/524842961. The previous one is OK: https://travis-ci.org/ckeditor/ckeditor5/builds/523466495.It looks like Chrome introduces some changes as previos Jobs were executed on Chrome
73.x
and the failing one is on Chrome74.x
.I've tested locally and all previous builds on
master-revisions
branch fails locally.The text was updated successfully, but these errors were encountered: