DO NOT MERGE - rewrite test act helpers based on react/#15591

[threepointone]

This needs facebook/react#15591 to land to work correctly.

This simplifies your test helpers to loop until all timers are flushed (including the ones that get queued after updates), and works in concurrent mode.

[threepointone]

(I also wanted to rename actSuspense to actAsync since it isn't exactly suspense related, but I'll leave that to you)

[bvaughn]

Since facebook/react#15591 was merged and we're running on a canary release that includes it, I tried applying these changes in master. Looks like everything works except for inspectedElementContext-test.js which always fails with an infinite loop bug here:

react-devtools-experimental/src/__tests__/utils.js

Lines 24 to 29 in 7aaef6f

	while (jest.getTimerCount() > 0) {
	// $FlowFixMe Flow doens't know about "await act()" yet
	await TestUtils.act(async () => {
	jest.runAllTimers();
	});
	}

Seems like the underlying cause are these bridge messages:

react-devtools-experimental/src/devtools/views/Components/InspectedElementContext.js

Lines 142 to 150 in c841101

	bridge.send('inspectElement', { id: selectedElementID, rendererID });
	};
	// Send the initial inspection request.
	// We'll poll for an update in the response handler below.
	sendRequest();
	// Update the $r variable.
	bridge.send('selectElement', { id: selectedElementID, rendererID });

Since the bridge batches things with a timeout:

react-devtools-experimental/src/bridge.js

Lines 48 to 51 in c841101

	this._messageQueue.push(event, payload, transferable);
	if (!this._timeoutID) {
	this._timeoutID = setTimeout(this._flush, 0);
	}

Not sure why they would cause problems specifically, given that other bridge messages don't cause other tests to fail. I've tried commenting out the setTimeout further down to see if that's somehow related, but it isn't. Maybe we just aren't triggering other bridge messages within the actAsync helper?

[bvaughn]

Let's talk when we're both back in the office on Tuesday, Sunil. In the meanwhile I'll push my work in progress here https://github.com/bvaughn/react-devtools-experimental/pull/new/threepointone-act-async

[threepointone]

boop, have a look and let me know if this solves your problems

[threepointone]

sorry, I should be a commie about this
our problems

[bvaughn]

Cool, thanks Sunil!

[bvaughn]

Hey uh...looks like jest.getTimerCount() doesn't exist in the version of Jest used in the React repo. Wondering what you're suggesting folks use who are using a version of Jest older than v24 (like me now, heh heh)

[threepointone]

Oh hmm. We could just update to jest 24, if you could help resolve this issue with react-is? facebook/react#15778

[threepointone]

(Alternately, there may be a way of getting at the timer count with a hidden api, I’ll have a look)

[bvaughn]

Upgrading might be nice

[SimenB]

I'm pretty sure there's no way to access a count of scheduled timers in 23 without using a custom testEnvironment which could expose the FakeTimers instance on the global injected into the test

[bvaughn]

That doesn't sound out of the question, unless I'm misunderstanding something.

It looks like the fake-timers object exposes a mockGetTimersCount() which I think might satisfy the need my tests have.

Not sure how to get a handle on that object though.

[bvaughn]

Okay. I think that path would entail:

• Extending the default Jest env (e.g. jest-environment-jsdom)
• Specifying a custom testEnvironment in my Jest config to point to the subclass
• Getting a handle on this.fakeTimers in my environment subclass
• Exposing that handle to tests via some global ref the custom env's this.global

The last bit is the part I'm least sure about. I don't know if there are timing/concurrency concerns, or how best to share between the test env and test code. I tried storing a reference on global but it doesn't seem like that's shared between the env and the test (or maybe something is resetting it)

I'm able to inject, but I'm not seeing that function- maybe it was added in a later Jest v23 release. I'll try bumping that.

[bvaughn]

Actually, tracing through the Jest source code- it looks like Jest sets mockGetTimersCount on the global object itself 😆 I don't even need to use a custom environment for this.

[SimenB]

Ah, I forgot about that. Removed in 24 and replaced by getTimerCount in jestjs/jest#7285. Great you were able to find it

[bvaughn]

No problem 😄 Now I'm more familiar with Jest internals, having traced through this, so not wasted effort.

[SimenB]

Awesome! To answer your above question (even though they're striked out, so you probably figured it out), all your steps are correct. Should probably delete in teardown as well to avoid a potential memory leak, but all tests gets their own global, so there shouldn't have been any concurrency issues

[bvaughn]

You rock. Thansk~