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.

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

Add warning and failing test decorators #5929

Merged
merged 28 commits into from
May 9, 2024
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -2,23 +2,95 @@ import { Component, ReactElement } from 'react';
import { TestRunner } from './TestRunner';
import { TestComponent } from './TestComponent';
import type { SharedValue } from 'react-native-reanimated';
import { TestConfiguration, TestValue, NullableTestValue } from './types';
import { TestConfiguration, TestValue, NullableTestValue, DescribeDecorator, TestDecorator } from './types';

export { Presets } from './Presets';

const testRunner = new TestRunner();

export const describe = (name: string, buildSuite: () => void) => {
testRunner.describe(name, buildSuite);
};

describe.skip = (name: string, buildSuite: () => void) => {
testRunner.describe(name, buildSuite, false, true);
};

describe.only = (name: string, buildSuite: () => void) => {
testRunner.describe(name, buildSuite, true);
};
type DescribeFunction = (name: string, buildSuite: () => void) => void;
export const describe: {
(name: string, buildSuite: () => void): void;
skip: DescribeFunction;
only: DescribeFunction;
} = Object.assign(
(name: string, buildSuite: () => void) => {
testRunner.describe(name, buildSuite, null);
},
{
skip: (name: string, buildSuite: () => void) => {
testRunner.describe(name, buildSuite, DescribeDecorator.SKIP);
},
only: (name: string, buildSuite: () => void) => {
testRunner.describe(name, buildSuite, DescribeDecorator.SKIP);
},
},
);

type TestEachFunction = <T>(
examples: Array<T>,
) => (name: string, testCase: (example: T, index?: number) => void) => void;
type TestEachFunctionWithWarning = <T>(
examples: Array<T>,
) => (name: string, expectedWarning: string, testCase: (example: T, index?: number) => void) => void;

export const test: {
(name: string, testCase: () => void): void;
each: TestEachFunction;
skip: { (name: string, testCase: () => void): void; each: TestEachFunction };
only: { (name: string, testCase: () => void): void; each: TestEachFunction };
failing: { (name: string, warningMessage: string, testCase: () => void): void; each: TestEachFunctionWithWarning };
warn: { (name: string, warningMessage: string, testCase: () => void): void; each: TestEachFunctionWithWarning };
} = Object.assign(
(name: string, testCase: () => void) => {
testRunner.test(name, testCase, null);
},
{
each: <T>(examples: Array<T>) => {
return testRunner.testEach(examples, null);
},
skip: Object.assign(
(name: string, testCase: () => void) => {
testRunner.test(name, testCase, TestDecorator.SKIP);
},
{
each: <T>(examples: Array<T>) => {
return testRunner.testEach(examples, TestDecorator.SKIP);
},
},
),
Comment on lines +44 to +61
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This Object.assign syntax is quite challenging to read, and there seems to be some code duplication. I believe that you understand this syntax but this is a bit overcomplicated. Could you refactor it? 🙏

only: Object.assign(
(name: string, testCase: () => void) => {
testRunner.test(name, testCase, TestDecorator.ONLY);
},
{
each: <T>(examples: Array<T>) => {
return testRunner.testEach(examples, null);
},
},
),
failing: Object.assign(
(name: string, warningMessage: string, testCase: () => void) => {
testRunner.test(name, testCase, TestDecorator.FAILING, warningMessage);
},
{
each: <T>(examples: Array<T>) => {
return testRunner.testEachErrorMsg(examples, TestDecorator.FAILING);
},
},
),
warn: Object.assign(
(name: string, expectedWarning: string, testCase: () => void) => {
testRunner.test(name, testCase, TestDecorator.WARN);
},
{
each: <T>(examples: Array<T>) => {
return testRunner.testEachErrorMsg(examples, TestDecorator.WARN);
},
},
),
},
);

export function beforeAll(job: () => void) {
testRunner.beforeAll(job);
Expand All @@ -36,33 +108,6 @@ export function afterAll(job: () => void) {
testRunner.afterAll(job);
}

export const test = (name: string, testCase: () => void) => {
testRunner.test(name, testCase);
};

test.each = <T>(examples: Array<T>) => {
return testRunner.testEach(examples);
};

const onlyDecorator = (name: string, testCase: () => void) => {
testRunner.test(name, testCase, true);
};

onlyDecorator.each = <T>(examples: Array<T>) => {
return testRunner.testEach(examples, true);
};
test.only = onlyDecorator;

const skipDecorator = (name: string, testCase: () => void) => {
testRunner.test(name, testCase, false, true);
};

skipDecorator.each = <T>(examples: Array<T>) => {
return testRunner.testEach(examples, false, true);
};

test.skip = skipDecorator;

export async function render(component: ReactElement<Component> | null) {
return testRunner.render(component);
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,31 @@
import { View, Button, StyleSheet } from 'react-native';
import { View, Button, StyleSheet, Text } from 'react-native';
import React, { ReactNode, useEffect, useState } from 'react';
import { runTests, configure } from './RuntimeTestsApi';
import { LockObject } from './types';

let renderLock: LockObject = { lock: false };
export class ErrorBoundary extends React.Component<
{ children: React.JSX.Element | Array<React.JSX.Element> },
{ hasError: boolean }
> {
constructor(props: { children: React.JSX.Element | Array<React.JSX.Element> }) {
super(props);
this.state = { hasError: false };
}

static getDerivedStateFromError(_: unknown) {
// Update state so the next render will show the fallback UI.
return { hasError: true };
}

render() {
if (this.state.hasError) {
return <Text>Something went wrong.</Text>;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
return <Text>Something went wrong.</Text>;
return <Text>Detected exception during component rendering.</Text>;


return this.props.children;
}
}

export default function RuntimeTestsRunner() {
const [component, setComponent] = useState<ReactNode | null>(null);
Expand Down
138 changes: 100 additions & 38 deletions app/src/examples/RuntimeTests/ReanimatedRuntimeTestsRunner/TestRunner.ts
Original file line number Diff line number Diff line change
@@ -1,20 +1,23 @@
import { Component, MutableRefObject, ReactElement, useRef } from 'react';
import type {
NullableTestValue,
LockObject,
Operation,
SharedValueSnapshot,
TestCase,
TestConfiguration,
TestSuite,
TestSummary,
TestValue,
TrackerCallCount,
import {
type NullableTestValue,
type LockObject,
type Operation,
type SharedValueSnapshot,
type TestCase,
type TestConfiguration,
type TestSuite,
type TestSummary,
type TestValue,
type TrackerCallCount,
ComparisonMode,
DescribeDecorator,
TestDecorator,
} from './types';
Comment on lines +2 to 16
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's split it to two imports: import type {} from './type' and import {} from './type'

import { TestComponent } from './TestComponent';
import { render, stopRecordingAnimationUpdates, unmockAnimationTimer } from './RuntimeTestsApi';
import { getTrackerCallCount, render, stopRecordingAnimationUpdates, unmockAnimationTimer } from './RuntimeTestsApi';
import { makeMutable, runOnUI, runOnJS, SharedValue } from 'react-native-reanimated';
import { color, formatString, indentNestingLevel } from './stringFormatUtils';
import { applyMarkdown, color, formatString, indentNestingLevel } from './stringFormatUtils';
import { createUpdatesContainer } from './UpdatesContainer';
import { Matchers, nullableMatch } from './Matchers';
import { assertMockedAnimationTimestamp, assertTestCase, assertTestSuite } from './Asserts';
Expand Down Expand Up @@ -86,16 +89,20 @@ export class TestRunner {
}
this._wasRenderedNull = !component;
this._renderLock.lock = true;
this._renderHook(component);
try {
this._renderHook(component);
} catch (e) {
console.log(e);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we ignore all exceptions? 🤔

}
return this.waitForPropertyValueChange(this._renderLock, 'lock');
}

public async clearRenderOutput() {
return await this.render(null);
}

public describe(name: string, buildSuite: () => void, only = false, skip = false) {
if (only) {
public describe(name: string, buildSuite: () => void, decorator: DescribeDecorator | null) {
if (decorator === DescribeDecorator.ONLY) {
this._includesOnly = true;
}

Expand All @@ -116,38 +123,63 @@ export class TestRunner {
}

this._testSuites.splice(index, 0, {
name,
name: applyMarkdown(name),
buildSuite,
testCases: [],
nestingLevel: (this._currentTestSuite?.nestingLevel || 0) + 1,
only: !!(only || this._currentTestSuite?.only),
skip: !!(skip || this._currentTestSuite?.skip),
decorator: decorator ? decorator : this._currentTestSuite?.decorator ? this._currentTestSuite?.decorator : null,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could we avoid nesting inline conditions?

});
}

public test(name: string, run: () => void, only = false, skip = false) {
public test(name: string, run: () => void, decorator: TestDecorator | null, warningMessage = '') {
assertTestSuite(this._currentTestSuite);
if (only) {
if (decorator === TestDecorator.ONLY) {
this._includesOnly = true;
}
this._currentTestSuite.testCases.push({
name,
run,
componentsRefs: {},
callsRegistry: {},
errors: [],
only: only,
skip: skip,
});
this._currentTestSuite.testCases.push(
decorator === TestDecorator.WARN || decorator === TestDecorator.FAILING
? {
name: applyMarkdown(name),
run,
componentsRefs: {},
callsRegistry: {},
errors: [],
decorator,
warningMessage: warningMessage,
}
: {
name: applyMarkdown(name),
run,
componentsRefs: {},
callsRegistry: {},
errors: [],
decorator,
},
);
Comment on lines +139 to +158
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about this?

const hasMessage = TestDecorator.WARN || decorator === TestDecorator.FAILING;
this._currentTestSuite.testCases.push({
  name: applyMarkdown(name),
  run,
  componentsRefs: {},
  callsRegistry: {},
  errors: [],
  decorator,
  warningMessage: hasMessage ? warningMessage : undefined,
});

}

public testEach<T>(examples: Array<T>, only = false, skip = false) {
public testEachErrorMsg<T>(examples: Array<T>, decorator: TestDecorator) {
return (name: string, expectedWarning: string, testCase: (example: T) => void) => {
examples.forEach((example, index) => {
const currentTestCase = async () => {
await testCase(example);
};
this.test(
formatString(name, example, index),
currentTestCase,
decorator,
formatString(expectedWarning, example, index),
);
});
};
}
public testEach<T>(examples: Array<T>, decorator: TestDecorator | null) {
return (name: string, testCase: (example: T, index?: number) => void) => {
examples.forEach((example, index) => {
const currentTestCase = async () => {
await testCase(example, index);
};
this.test(formatString(name, example, index), currentTestCase, only, skip);
this.test(formatString(name, example, index), currentTestCase, decorator);
});
};
}
Expand Down Expand Up @@ -221,16 +253,14 @@ export class TestRunner {
let skipTestSuite = testSuite.skip;

if (this._includesOnly) {
skipTestSuite = skipTestSuite || !testSuite.only;
skipTestSuite = skipTestSuite || !(testSuite.decorator === DescribeDecorator.ONLY);

for (const testCase of testSuite.testCases) {
if (testCase.only) {
if (testCase.decorator === TestDecorator.ONLY) {
skipTestSuite = false;
} else testCase.skip = testCase.skip || !testSuite.only;
delete testCase.only;
} else testCase.skip = testCase.skip || !(testSuite.decorator === DescribeDecorator.ONLY);
}
}
delete testSuite.only;
testSuite.skip = skipTestSuite;
}

Expand Down Expand Up @@ -280,7 +310,39 @@ export class TestRunner {
await testSuite.beforeEach();
}

await testCase.run();
if (testCase.decorator === TestDecorator.FAILING || testCase.decorator === TestDecorator.WARN) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This branch should be moved to separate function

const consoleTrackerRef = testCase.decorator === TestDecorator.FAILING ? 'console.error' : 'console.warn';
const message = makeMutable('');

const newConsoleFuncJS = (warning: string) => {
this.callTracker(consoleTrackerRef);
message.value = warning.split('\n\nThis error is located at:')[0];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if console.warn will be called more than once?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then the test doesn't pass. Do you think we need a possibility to handle multiple console.warn call?

};
console.error = newConsoleFuncJS;
console.warn = newConsoleFuncJS;
Comment on lines +321 to +322
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you ever restore original implementation of console.warn?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, since you have to reload the app to re-run the tests and it resets it.


const callTrackerCopy = this.callTracker;

runOnUI(() => {
'worklet';
const newConsoleFuncUI = (warning: string) => {
callTrackerCopy(consoleTrackerRef);
message.value = warning.split('\n\nThis error is located at:')[0];
};
console.error = newConsoleFuncUI;
console.warn = newConsoleFuncUI;
})();
Comment on lines +326 to +334
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since runOnUI is an asynchronous function, to prevent potential non-deterministic behaviors, it would be better to use runOnUISync.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you mean runOnUI blocking?


await testCase.run();

this.expect(getTrackerCallCount(consoleTrackerRef)).toBeCalled(1);
if (testCase.warningMessage) {
this.expect(message.value).toBe(testCase.warningMessage, ComparisonMode.STRING);
}
Comment on lines +338 to +341
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The expect function should be utilized by the user during testing, not in our internal logic. This functionality is not the responsibility of the expect method. We can simply use a single shared value for it instead of a callTracker.

} else {
await testCase.run();
}

this.showTestCaseSummary(testCase, testSuite.nestingLevel);

if (testSuite.afterEach) {
Expand Down