@samitouri / QOS-React-2 / commits / 8faf751937

Codemod some expiration tests to waitForExpired (#26491)

Continuing my journey to migrate all the Scheduler flush* methods to async versions of the same helpers. `unstable_flushExpired` is a rarely used helper that is only meant to be used to test a very particular implementation detail (update starvation prevention, or what we sometimes refer to as "expiration"). I've prefixed the new helper with `unstable_`, too, to indicate that our tests should almost always prefer one of the other patterns instead.

Andrew Clark committed Mar 27, 2023 at 15:52 UTC 8faf751937e9672fb69dac8143dbfb0929caf77e

2 files changed +76 -37

packages/internal-test-utils/ReactInternalTestUtils.js

+38 -5

@@ -29,7 +29,7 @@ async function waitForMicrotasks() {

29 });

30 }

32 -export async function waitFor(expectedLog) {

32 +export async function waitFor(expectedLog, options) {

33 assertYieldsWereCleared(SchedulerMock);

35 // Create the error object before doing any async work, to get a better

@@ -37,16 +37,15 @@ export async function waitFor(expectedLog) {

37 const error = new Error();

38 Error.captureStackTrace(error, waitFor);

40 + const stopAfter = expectedLog.length;

41 const actualLog = [];

42 do {

43 // Wait until end of current task/microtask.

44 await waitForMicrotasks();

45 if (SchedulerMock.unstable_hasPendingWork()) {

45 - SchedulerMock.unstable_flushNumberOfYields(

46 - expectedLog.length - actualLog.length,

47 - );

46 + SchedulerMock.unstable_flushNumberOfYields(stopAfter - actualLog.length);

47 actualLog.push(...SchedulerMock.unstable_clearLog());

49 - if (expectedLog.length > actualLog.length) {

48 + if (stopAfter > actualLog.length) {

49 // Continue flushing until we've logged the expected number of items.

50 } else {

51 // Once we've reached the expected sequence, wait one more microtask to

@@ -61,6 +60,12 @@ export async function waitFor(expectedLog) {

60 }

61 } while (true);

63 + if (options && options.additionalLogsAfterAttemptingToYield) {

64 + expectedLog = expectedLog.concat(

65 + options.additionalLogsAfterAttemptingToYield,

66 + );

67 + }

68 +

69 if (equals(actualLog, expectedLog)) {

70 return;

71 }

@@ -151,6 +156,34 @@ ${diff(expectedError, x)}

156 } while (true);

157 }

158

159 +// This is prefixed with `unstable_` because you should almost always try to

160 +// avoid using it in tests. It's really only for testing a particular

161 +// implementation detail (update starvation prevention).

162 +export async function unstable_waitForExpired(expectedLog): mixed {

163 + assertYieldsWereCleared(SchedulerMock);

164 +

165 + // Create the error object before doing any async work, to get a better

166 + // stack trace.

167 + const error = new Error();

168 + Error.captureStackTrace(error, unstable_waitForExpired);

169 +

170 + // Wait until end of current task/microtask.

171 + await waitForMicrotasks();

172 + SchedulerMock.unstable_flushExpired();

173 +

174 + const actualLog = SchedulerMock.unstable_clearLog();

175 + if (equals(actualLog, expectedLog)) {

176 + return;

177 + }

178 +

179 + error.message = `

180 +Expected sequence of events did not occur.

181 +

182 +${diff(expectedLog, actualLog)}

183 +`;

184 + throw error;

185 +}

186 +

187 // TODO: This name is a bit misleading currently because it will stop as soon as

188 // React yields for any reason, not just for a paint. I've left it this way for

189 // now because that's how untable_flushUntilNextPaint already worked, but maybe

packages/react-reconciler/src/__tests__/ReactExpiration-test.js

+38 -32

@@ -21,6 +21,7 @@ let useEffect;

21 let assertLog;

22 let waitFor;

23 let waitForAll;

24 +let unstable_waitForExpired;

26 describe('ReactExpiration', () => {

27 beforeEach(() => {

@@ -38,6 +39,7 @@ describe('ReactExpiration', () => {

39 assertLog = InternalTestUtils.assertLog;

40 waitFor = InternalTestUtils.waitFor;

41 waitForAll = InternalTestUtils.waitForAll;

42 + unstable_waitForExpired = InternalTestUtils.unstable_waitForExpired;

44 const textCache = new Map();

@@ -124,18 +126,17 @@ describe('ReactExpiration', () => {

126 expect(ReactNoop).toMatchRenderedOutput('Step 1');

127

128 // Nothing has expired yet because time hasn't advanced.

127 - Scheduler.unstable_flushExpired();

129 + await unstable_waitForExpired([]);

130 expect(ReactNoop).toMatchRenderedOutput('Step 1');

131

132 // Advance time a bit, but not enough to expire the low pri update.

133 ReactNoop.expire(4500);

132 - Scheduler.unstable_flushExpired();

134 + await unstable_waitForExpired([]);

135 expect(ReactNoop).toMatchRenderedOutput('Step 1');

136

137 // Advance by a little bit more. Now the update should expire and flush.

138 ReactNoop.expire(500);

137 - Scheduler.unstable_flushExpired();

138 - assertLog(['Step 2']);

139 + await unstable_waitForExpired(['Step 2']);

140 expect(ReactNoop).toMatchRenderedOutput('Step 2');

141 });

142

@@ -339,8 +340,7 @@ describe('ReactExpiration', () => {

340

341 Scheduler.unstable_advanceTime(10000);

342

342 - Scheduler.unstable_flushExpired();

343 - assertLog(['D', 'E']);

343 + await unstable_waitForExpired(['D', 'E']);

344 expect(root).toMatchRenderedOutput('ABCDE');

345 });

346

@@ -369,8 +369,7 @@ describe('ReactExpiration', () => {

369

370 Scheduler.unstable_advanceTime(10000);

371

372 - Scheduler.unstable_flushExpired();

373 - assertLog(['D', 'E']);

372 + await unstable_waitForExpired(['D', 'E']);

373 expect(root).toMatchRenderedOutput('ABCDE');

374 });

375

@@ -383,6 +382,7 @@ describe('ReactExpiration', () => {

382 const InternalTestUtils = require('internal-test-utils');

383 waitFor = InternalTestUtils.waitFor;

384 assertLog = InternalTestUtils.assertLog;

385 + unstable_waitForExpired = InternalTestUtils.unstable_waitForExpired;

386

387 // Before importing the renderer, advance the current time by a number

388 // larger than the maximum allowed for bitwise operations.

@@ -401,19 +401,17 @@ describe('ReactExpiration', () => {

401 await waitFor(['Step 1']);

402

403 // The update should not have expired yet.

404 - Scheduler.unstable_flushExpired();

405 - assertLog([]);

404 + await unstable_waitForExpired([]);

405

406 expect(ReactNoop).toMatchRenderedOutput('Step 1');

407

408 // Advance the time some more to expire the update.

409 Scheduler.unstable_advanceTime(10000);

411 - Scheduler.unstable_flushExpired();

412 - assertLog(['Step 2']);

410 + await unstable_waitForExpired(['Step 2']);

411 expect(ReactNoop).toMatchRenderedOutput('Step 2');

412 });

413

416 - it('should measure callback timeout relative to current time, not start-up time', () => {

414 + it('should measure callback timeout relative to current time, not start-up time', async () => {

415 // Corresponds to a bugfix: https://github.com/facebook/react/pull/15479

416 // The bug wasn't caught by other tests because we use virtual times that

417 // default to 0, and most tests don't advance time.

@@ -424,15 +422,13 @@ describe('ReactExpiration', () => {

422 React.startTransition(() => {

423 ReactNoop.render('Hi');

424 });

427 - Scheduler.unstable_flushExpired();

428 - assertLog([]);

425 + await unstable_waitForExpired([]);

426 expect(ReactNoop).toMatchRenderedOutput(null);

427

428 // Advancing by ~5 seconds should be sufficient to expire the update. (I

429 // used a slightly larger number to allow for possible rounding.)

430 Scheduler.unstable_advanceTime(6000);

434 - Scheduler.unstable_flushExpired();

435 - assertLog([]);

431 + await unstable_waitForExpired([]);

432 expect(ReactNoop).toMatchRenderedOutput('Hi');

433 });

434

@@ -476,9 +472,9 @@ describe('ReactExpiration', () => {

472 // The remaining work hasn't expired, so the render phase is time sliced.

473 // In other words, we can flush just the first child without flushing

474 // the rest.

479 - Scheduler.unstable_flushNumberOfYields(1);

475 + //

476 // Yield right after first child.

481 - assertLog(['Sync pri: 1']);

477 + await waitFor(['Sync pri: 1']);

478 // Now do the rest.

479 await waitForAll(['Normal pri: 1']);

480 });

@@ -502,8 +498,9 @@ describe('ReactExpiration', () => {

498

499 // The remaining work _has_ expired, so the render phase is _not_ time

500 // sliced. Attempting to flush just the first child also flushes the rest.

505 - Scheduler.unstable_flushNumberOfYields(1);

506 - assertLog(['Sync pri: 2', 'Normal pri: 2']);

501 + await waitFor(['Sync pri: 2'], {

502 + additionalLogsAfterAttemptingToYield: ['Normal pri: 2'],

503 + });

504 });

505 expect(root).toMatchRenderedOutput('Sync pri: 2, Normal pri: 2');

506 });

@@ -606,18 +603,22 @@ describe('ReactExpiration', () => {

603 startTransition(() => {

604 setB(1);

605 });

606 + await waitFor(['B0']);

607 +

608 // Expire both the transitions

609 Scheduler.unstable_advanceTime(10000);

610 // Both transitions have expired, but since they aren't related

611 // (entangled), we should be able to finish the in-progress transition

612 // without also including the next one.

614 - Scheduler.unstable_flushNumberOfYields(1);

615 - assertLog(['B0', 'C']);

613 + await waitFor([], {

614 + additionalLogsAfterAttemptingToYield: ['C'],

615 + });

616 expect(root).toMatchRenderedOutput('A1B0C');

617

618 // The next transition also finishes without yielding.

619 - Scheduler.unstable_flushNumberOfYields(1);

620 - assertLog(['A1', 'B1', 'C']);

619 + await waitFor(['A1'], {

620 + additionalLogsAfterAttemptingToYield: ['B1', 'C'],

621 + });

622 expect(root).toMatchRenderedOutput('A1B1C');

623 });

624 });

@@ -662,8 +663,9 @@ describe('ReactExpiration', () => {

663 Scheduler.unstable_advanceTime(10000);

664

665 // The rest of the update finishes without yielding.

665 - Scheduler.unstable_flushNumberOfYields(1);

666 - assertLog(['B', 'C']);

666 + await waitFor([], {

667 + additionalLogsAfterAttemptingToYield: ['B', 'C'],

668 + });

669 });

670 });

671

@@ -705,8 +707,9 @@ describe('ReactExpiration', () => {

707

708 // Now flush the original update. Because it expired, it should finish

709 // without yielding.

708 - Scheduler.unstable_flushNumberOfYields(1);

709 - assertLog(['A1', 'B1']);

710 + await waitFor(['A1'], {

711 + additionalLogsAfterAttemptingToYield: ['B1'],

712 + });

713 });

714 });

715

@@ -731,16 +734,19 @@ describe('ReactExpiration', () => {

734 assertLog(['A0', 'B0', 'C0', 'Effect: 0']);

735 expect(root).toMatchRenderedOutput('A0B0C0');

736

734 - await act(() => {

737 + await act(async () => {

738 startTransition(() => {

739 root.render(<App step={1} />);

740 });

741 + await waitFor(['A1']);

742 +

743 // Expire the update

744 Scheduler.unstable_advanceTime(10000);

745

746 // The update finishes without yielding. But it does not flush the effect.

742 - Scheduler.unstable_flushNumberOfYields(1);

743 - assertLog(['A1', 'B1', 'C1']);

747 + await waitFor(['B1'], {

748 + additionalLogsAfterAttemptingToYield: ['C1'],

749 + });

750 });

751 // The effect flushes after paint.

752 assertLog(['Effect: 1']);