-
Notifications
You must be signed in to change notification settings - Fork 13.4k
fix(timeouts): clear timeouts created in componentDidLoad #30851
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
base: main
Are you sure you want to change the base?
fix(timeouts): clear timeouts created in componentDidLoad #30851
Conversation
|
@KillerCodeMonkey is attempting to deploy a commit to the Ionic Team on Vercel. A member of the Team first needs to authorize it. |
|
lint + build work locally. but one test is failing: src/components/datetime/test/disabled/datetime.spec.tsx
● ion-datetime disabled › picker should be disabled in prefer wheel mode
expect(received).toEqual(expected) // deep equality
Expected: 4
Received: 3
31 | const columns = datetime.shadowRoot!.querySelectorAll('ion-picker-column');
32 |
> 33 | await expect(columns.length).toEqual(4);
| ^
34 |
35 | columns.forEach((column) => {
36 | expect(column.disabled).toBe(true);
at Object.<anonymous> (src/components/datetime/test/disabled/datetime.spec.tsx:33:34)but it fails with clean master as well. |
|
@KillerCodeMonkey Thank you for the pull request! In order for us to move forward, we need this PR to have a linked issue. Please submit one with a minimal repro, thanks! |
|
How to create a repro repository on possible flaky tests and not cleaned up timeouts? I mean this is just how to work with timeouts, intervals? Clean them up when not needed anymore to avoid side effects. And prevent memory leaks. So how should i be able to create a repro repo? There are already some cleanups in the code in other components... Greets |
|
@KillerCodeMonkey I agree that we should clean up the timeouts and we appreciate your work on this! However, if you can provide a reproduction of what you have even if it's flaky. By providing an official issue, we can verify that we haven't missed anything else. We might end up finding more issues related to timeouts during triage that we can keep tabs of. |
|
@KillerCodeMonkey don't worry too much on providing a repro. The most important thing is to create the issue, thanks! |
|
i just created a clean issue and not a bug to avoid to set the required repo url field. i hope this is okay #30860 |
|
That should be fine! Thank you so much!! |
Issue number: resolves #30860
What is the current behavior?
We have flaky tests in an ionic angular project that root cause are not cleaned up timeouts.
I commented out the timeout in the searchbar componentWillLoad method. and after several runs no flaky tests at all.
My guess -> test runs faster than the 300ms it takes til the timeout runs. Everything is cleaned up, but not the ionic timeouts (i think i saw something similar in other components)
What is the new behavior?
Timeouts are cleaned on disconnect.
Does this introduce a breaking change?
Other information
Testrunner is vitest + angular 20 and latest ionic version 8.x