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

Update test comments with explanations #21857

Merged
merged 1 commit into from
Jul 19, 2021
Merged

Update test comments with explanations #21857

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 17 additions & 20 deletions packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -164,34 +164,31 @@ describe('ReactHooksWithNoopRenderer', () => {
expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);

// Schedule some updates
if (gate(flags => flags.enableSyncDefaultUpdates)) {
React.startTransition(() => {
// TODO: Batched updates need to be inside startTransition?
rickhanlonii marked this conversation as resolved.
Show resolved Hide resolved
ReactNoop.batchedUpdates(() => {
act(() => {
if (gate(flags => flags.enableSyncDefaultUpdates)) {
React.startTransition(() => {
counter.current.updateCount(1);
counter.current.updateCount(count => count + 10);
});
});
} else {
ReactNoop.batchedUpdates(() => {
} else {
counter.current.updateCount(1);
counter.current.updateCount(count => count + 10);
});
}
}

// Partially flush without committing
expect(Scheduler).toFlushAndYieldThrough(['Count: 11']);
expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);
// Partially flush without committing
expect(Scheduler).toFlushAndYieldThrough(['Count: 11']);
expect(ReactNoop.getChildren()).toEqual([span('Count: 0')]);

// Interrupt with a high priority update
ReactNoop.flushSync(() => {
ReactNoop.render(<Counter label="Total" />);
});
expect(Scheduler).toHaveYielded(['Total: 0']);
// Interrupt with a high priority update
ReactNoop.flushSync(() => {
ReactNoop.render(<Counter label="Total" />);
});
expect(Scheduler).toHaveYielded(['Total: 0']);

// Resume rendering
expect(Scheduler).toFlushAndYield(['Total: 11']);
expect(ReactNoop.getChildren()).toEqual([span('Total: 11')]);
// Resume rendering
expect(Scheduler).toFlushAndYield(['Total: 11']);
expect(ReactNoop.getChildren()).toEqual([span('Total: 11')]);
});
});

it('throws inside class components', () => {
Expand Down
47 changes: 15 additions & 32 deletions packages/react-reconciler/src/__tests__/ReactIncrementalScheduling-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -230,7 +230,7 @@ describe('ReactIncrementalScheduling', () => {
state = {tick: 0};

componentDidMount() {
ReactNoop.deferredUpdates(() => {
React.startTransition(() => {
Scheduler.unstable_yieldValue(
'componentDidMount (before setState): ' + this.state.tick,
);
Expand All @@ -242,7 +242,7 @@ describe('ReactIncrementalScheduling', () => {
}

componentDidUpdate() {
ReactNoop.deferredUpdates(() => {
React.startTransition(() => {
Scheduler.unstable_yieldValue(
'componentDidUpdate: ' + this.state.tick,
);
Expand Down Expand Up @@ -280,38 +280,21 @@ describe('ReactIncrementalScheduling', () => {
expect(Scheduler).toFlushAndYield(['render: 1', 'componentDidUpdate: 1']);
expect(ReactNoop).toMatchRenderedOutput(<span prop={1} />);

// Increment the tick to 2. This will trigger an update inside cDU. Flush
// the first update without flushing the second one.
if (gate(flags => flags.enableSyncDefaultUpdates)) {
React.startTransition(() => {
instance.setState({tick: 2});
});

// TODO: why does this flush sync?
expect(Scheduler).toFlushAndYieldThrough([
'render: 2',
'componentDidUpdate: 2',
'componentDidUpdate (before setState): 2',
'componentDidUpdate (after setState): 2',
'render: 3',
'componentDidUpdate: 3',
]);
expect(ReactNoop).toMatchRenderedOutput(<span prop={3} />);
} else {
React.startTransition(() => {
instance.setState({tick: 2});
});

expect(Scheduler).toFlushAndYieldThrough([
'render: 2',
'componentDidUpdate: 2',
'componentDidUpdate (before setState): 2',
'componentDidUpdate (after setState): 2',
]);
expect(ReactNoop).toMatchRenderedOutput(<span prop={2} />);

// Now flush the cDU update.
expect(Scheduler).toFlushAndYield(['render: 3', 'componentDidUpdate: 3']);
expect(ReactNoop).toMatchRenderedOutput(<span prop={3} />);
}
expect(Scheduler).toFlushUntilNextPaint([
'render: 2',
'componentDidUpdate: 2',
'componentDidUpdate (before setState): 2',
'componentDidUpdate (after setState): 2',
]);
expect(ReactNoop).toMatchRenderedOutput(<span prop={2} />);

// Now flush the cDU update.
expect(Scheduler).toFlushAndYield(['render: 3', 'componentDidUpdate: 3']);
expect(ReactNoop).toMatchRenderedOutput(<span prop={3} />);
});

it('performs Task work even after time runs out', () => {
Expand Down
9 changes: 6 additions & 3 deletions packages/react-reconciler/src/__tests__/ReactIncrementalUpdates-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -196,11 +196,13 @@ describe('ReactIncrementalUpdates', () => {
// Now flush the remaining work. Even though e and f were already processed,
// they should be processed again, to ensure that the terminal state
// is deterministic.
// TODO: should d, e, f be flushed again first?
expect(Scheduler).toFlushAndYield([
// Since 'g' is in a transition, we'll process 'd' separately first.
rickhanlonii marked this conversation as resolved.
Show resolved Hide resolved
// That causes us to process 'd' with 'e' and 'f' rebased.
'd',
'e',
'f',
// Then we'll re-process everything for 'g'.
'a',
'b',
'c',
Expand Down Expand Up @@ -290,18 +292,19 @@ describe('ReactIncrementalUpdates', () => {
});

// The sync updates should have flushed, but not the async ones.
// TODO: should 'd' have flushed?
// TODO: should 'f' have flushed? I don't know what enqueueReplaceState is.
expect(Scheduler).toHaveYielded(['e', 'f']);
expect(ReactNoop.getChildren()).toEqual([span('f')]);

// Now flush the remaining work. Even though e and f were already processed,
// they should be processed again, to ensure that the terminal state
// is deterministic.
expect(Scheduler).toFlushAndYield([
// Since 'g' is in a transition, we'll process 'd' separately first.
// That causes us to process 'd' with 'e' and 'f' rebased.
'd',
'e',
'f',
// Then we'll re-process everything for 'g'.
'a',
'b',
'c',
Expand Down