Skip to content
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.

Sign up for GitHub

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

Jump to bottom

AutoCleanedWeakCache causes failure when running tests in fakeAsync zone #11790

Closed
vz-tl opened this issue Apr 18, 2024 · 5 comments · Fixed by #11792 or #11810 · May be fixed by shonore/Fickle#1381, humanbot/fullstack-tutorial#1 or humanbot/graphql-apollo#1
Closed

AutoCleanedWeakCache causes failure when running tests in fakeAsync zone #11790

vz-tl opened this issue Apr 18, 2024 · 5 comments · Fixed by #11792 or #11810 · May be fixed by shonore/Fickle#1381, humanbot/fullstack-tutorial#1 or humanbot/graphql-apollo#1
Labels
⁉️ question

Comments

@vz-tl
Copy link

vz-tl commented Apr 18, 2024

Issue Description

Issue occurs in Angular 17 project while executing a Jest test running in fakeAsync zone, imported from @angular/core/testing (pseudo-code):

it('executes ApolloClient in a fakeAsync zone', fakeAsync(() => {
  TestBed.inject(ApolloTestingController).expectOne(...).flush(...);
}));

Result: test fails with Error: 2 timer(s) still in the queue.

Packages:

"@angular/core": "17.3.4",
"@apollo/client": "3.9.0"
"apollo-angular": "6.0.0",
"jest": "29.7.0"

Offending code is located in utilities.cjs :

var scheduledCleanup = new WeakSet();
function schedule(cache) {
    if (!scheduledCleanup.has(cache)) {
        scheduledCleanup.add(cache);
        setTimeout(function () {
            cache.clean();
            scheduledCleanup.delete(cache);
        }, 100); <-- this timeout causes the error in async tests
    }
}
var AutoCleanedWeakCache = function (max, dispose) {
    var cache = new caches.WeakCache(max, dispose);
    cache.set = function (key, value) {
        schedule(this);
        return caches.WeakCache.prototype.set.call(this, key, value);
    };
    return cache;
};

The issue is unrelated to apollo-angular, this package is just being used as a wrapper around ApolloClient (ApolloTestingController used in the test is part of it).

Downgrading @apollo/client to v3.8.10, solves the issue!

Link to Reproduction

Reproduction Steps

No response

@apollo/client version

3.9.0+
@alessbell
Copy link
Member

alessbell commented Apr 18, 2024 •
edited
Loading

Hi @vz-tl 👋 Thanks for the report here.

I was able to reproduce this by wrapping this test in the apollo-angular repo with fakeAsync.

The setTimeout in schedule auto-schedules cleanup of a particular cache (an AutoCleanedWeakCache) when a new item is added, and is throttled to once per 100ms. Clearly this additional microtask is incompatible with your test, but I hope you'll agree it's doing important work :) I'll cc: @phryneas there for any more context he'd like to add.

To unblock your upgrade to 3.9, you can call flush with flush(1) (or flush(2) - not being able to see your test it's hard to say what's causing both) at the end of your test so that only a single microtask is drained from the queue (if there are any more remaining your test will still fail).

I hope that explanation is helpful - I'll leave this open for now in case anyone else on the team has something to add.

phryneas added a commit that referenced this issue Apr 19, 2024
@phryneas
AutoCleanedCache: only schedule batched cache cleanup if the cache is…
c70a4c5 
… full

Fixes #11790

This prevents timeouts from being set when they are not necessary.
@phryneas phryneas mentioned this issue Apr 19, 2024
Merged
@phryneas
Copy link
Member

For background - the purpose of this timeout is to batch cleanups of "memoized cache" cleans.
If your cache is full and you add 20 more items, you don't want to call .clean() 20 times, but only once, with a little bit of a delay.

That said, until now we scheduled these always, not only when the cache was full. I've opened #11792. Since your tests are unlikely to fill these caches, that should fix it for your usecase.

Could you please test the PR release and report back if it fixes your problem?

npm i @apollo/[email protected]

@vz-tl
Copy link
Author

vz-tl commented Apr 19, 2024

@phryneas , with your PR release tests are now working flawlessly!

Thx a lot, @phryneas, @alessbell! You are awesome! Looking forward to pull in the fix release!

phryneas added a commit that referenced this issue Apr 24, 2024
@phryneas
AutoCleanedCache: only schedule batched cache cleanup if the cache is…
5876c35 
… full (#11792)

* AutoCleanedCache: only schedule batched cache cleanup if the cache is full

Fixes #11790

This prevents timeouts from being set when they are not necessary.

* update size limit

* trigger CI

* Clean up Prettier, Size-limit, and Api-Extractor

---------

Co-authored-by: phryneas <[email protected]>
@github-actions GitHub Actions
Copy link
Contributor

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Client usage and allow us to serve you better.

@github-actions github-actions bot mentioned this issue Apr 24, 2024
Merged
@shonore shonore mentioned this issue May 19, 2024
Open
This was referenced May 19, 2024
Open
Open
This was referenced May 20, 2024
Open
Open
Open
@arddluma arddluma mentioned this issue May 22, 2024
Closed
@shonore shonore mentioned this issue May 23, 2024
Open
@xumical xumical mentioned this issue May 24, 2024
Open
@lwih lwih mentioned this issue May 24, 2024
Closed
@nejidevelops nejidevelops mentioned this issue May 24, 2024
Open
@GregBrimble GregBrimble mentioned this issue May 24, 2024
Open
@peterleiva peterleiva mentioned this issue May 24, 2024
Open
@denysmiand denysmiand mentioned this issue May 24, 2024
Open
@X-oss-byte X-oss-byte mentioned this issue May 24, 2024
Open
@basilisSam basilisSam mentioned this issue May 24, 2024
Open
@github-actions GitHub Actions
Copy link
Contributor

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
For general questions, we recommend using StackOverflow or our discord server.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 25, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
⁉️ question
Projects
None yet
Milestone
No milestone
3 participants
@phryneas @alessbell @vz-tl