From 619661e8f7cc583d82551d936d66cf3397f07293 Mon Sep 17 00:00:00 2001 From: Aleck Greenham Date: Sat, 6 May 2017 09:57:18 +0100 Subject: [PATCH 1/4] =?UTF-8?q?[Test=20Suite]=20Don=E2=80=99t=20run=20focu?= =?UTF-8?q?sed=20pending=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/.gitignore | 1 + tests/__tests__/src/tests/pendingTestTests.js | 78 +++++++++++++++++++ tests/lib/TestSuite.js | 20 ++--- 3 files changed, 89 insertions(+), 10 deletions(-) create mode 100644 tests/.gitignore diff --git a/tests/.gitignore b/tests/.gitignore new file mode 100644 index 00000000..5afd82db --- /dev/null +++ b/tests/.gitignore @@ -0,0 +1 @@ +__tests__/build/ diff --git a/tests/__tests__/src/tests/pendingTestTests.js b/tests/__tests__/src/tests/pendingTestTests.js index e7c128a9..1dafed1c 100644 --- a/tests/__tests__/src/tests/pendingTestTests.js +++ b/tests/__tests__/src/tests/pendingTestTests.js @@ -103,6 +103,84 @@ function pendingTestTests({ it: _it, describe: _describe }) { otherTest.should.be.called(); }); }); + + _describe('when an outer context is focused', () => { + _it('a pending test will still not run', async () => { + const pendingTest = sinon.spy(); + const otherTest = sinon.spy(); + const unfocusedTest = sinon.spy(); + + const testSuite = new TestSuite('', '', {}); + + testSuite.addTests(({ fdescribe, it, xit }) => { + fdescribe('', () => { + xit('', pendingTest); + + it('', otherTest); + }); + + it('', unfocusedTest); + }); + + testSuite.setStore({ + getState: () => { return {}; }, + }); + + const testIdsToRun = Object.keys(testSuite.testDefinitions.focusedTestIds).reduce((memo, testId) => { + if (!testSuite.testDefinitions.pendingTestIds[testId]) { + memo.push(testId); + } + + return memo; + }, []); + + await testSuite.run(testIdsToRun); + + pendingTest.should.not.be.called(); + otherTest.should.be.called(); + unfocusedTest.should.not.be.called(); + }); + }); + + _describe('when an outer context is focused', () => { + _it('a pending context will still not run', async () => { + const pendingTest = sinon.spy(); + const otherTest = sinon.spy(); + const unfocusedTest = sinon.spy(); + + const testSuite = new TestSuite('', '', {}); + + testSuite.addTests(({ fdescribe, it, xdescribe }) => { + fdescribe('', () => { + xdescribe('', () => { + it('', pendingTest); + }); + + it('', otherTest); + }); + + it('', unfocusedTest); + }); + + testSuite.setStore({ + getState: () => { return {}; }, + }); + + const testIdsToRun = Object.keys(testSuite.testDefinitions.focusedTestIds).reduce((memo, testId) => { + if (!testSuite.testDefinitions.pendingTestIds[testId]) { + memo.push(testId); + } + + return memo; + }, []); + + await testSuite.run(testIdsToRun); + + pendingTest.should.not.be.called(); + otherTest.should.be.called(); + unfocusedTest.should.not.be.called(); + }); + }); } export default pendingTestTests; diff --git a/tests/lib/TestSuite.js b/tests/lib/TestSuite.js index 3924bb8b..aaa6735c 100644 --- a/tests/lib/TestSuite.js +++ b/tests/lib/TestSuite.js @@ -111,19 +111,19 @@ class TestSuite { */ async run(testIds = undefined) { const testsToRun = (() => { - if (testIds) { - return testIds.map((id) => { - const test = this.testDefinitions.tests[id]; + return (testIds || Object.keys(this.testDefinitions.tests)).reduce((memo, id) => { + const test = this.testDefinitions.tests[id]; - if (!test) { - throw new RangeError(`ReactNativeFirebaseTests.TestRunError: Test with id ${id} not found in test suite ${this.name}`); - } + if (!test) { + throw new RangeError(`ReactNativeFirebaseTests.TestRunError: Test with id ${id} not found in test suite ${this.name}`); + } - return test; - }); - } + if (!this.testDefinitions.pendingTestIds[id]) { + memo.push(test); + } - return Object.values(this.testDefinitions.tests); + return memo; + }, []); })(); const testRun = new TestRun(this, testsToRun.reverse(), this.testDefinitions); From 591cf735d645556ce3fbffac1ab50b82a72a5fff Mon Sep 17 00:00:00 2001 From: Aleck Greenham Date: Sat, 6 May 2017 11:49:44 +0100 Subject: [PATCH 2/4] [TestSuite] Show full test description on Test screen --- tests/src/screens/Test.js | 69 +++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 31 deletions(-) diff --git a/tests/src/screens/Test.js b/tests/src/screens/Test.js index e716ece1..5ba50f27 100644 --- a/tests/src/screens/Test.js +++ b/tests/src/screens/Test.js @@ -51,33 +51,25 @@ class Test extends React.Component { setParams({ test }); } - renderError() { - const { test: { message } } = this.props; - - if (message) { - return ( - - Test Error - - {message} - - - ); - } - - return null; - } - - render() { - const { test: { func, status, time } } = this.props; + const { test: { message, description, func, status, time }, testContextName } = this.props; return ( {Test.renderBanner({ status, time })} - - {this.renderError()} - Test Code Preview + + + {testContextName}:{description} + + + Test Error + + {message || 'None'} + + + + Test Code Preview + {beautify(removeLastLine(removeFirstLine(func.toString())), { indent_size: 4, break_chained_methods: true })} @@ -95,8 +87,11 @@ Test.propTypes = { time: PropTypes.number, message: PropTypes.string, func: PropTypes.function, + description: PropTypes.string, }).isRequired, + testContextName: PropTypes.string, + navigation: PropTypes.shape({ setParams: PropTypes.func.isRequired, }).isRequired, @@ -107,27 +102,39 @@ const styles = StyleSheet.create({ flex: 1, backgroundColor: '#ffffff', }, - content: {}, - code: { - backgroundColor: '#3F373A', - color: '#c3c3c3', - padding: 5, - fontSize: 12, - }, - codeHeader: { + header: { fontWeight: '600', fontSize: 18, backgroundColor: '#000', color: '#fff', padding: 5, }, + code: { + backgroundColor: '#3F373A', + color: '#c3c3c3', + padding: 5, + fontSize: 12, + }, + description: { + padding: 5, + fontSize: 14, + }, + testLabel: { + fontWeight: '600', + fontSize: 16 + } }); -function select({ tests }, { navigation: { state: { params: { testId } } } }) { +function select({ tests, testContexts }, { navigation: { state: { params: { testId } } } }) { const test = tests[testId]; + let testContext = testContexts[test.testContextId]; + while(testContext.parentContextId && testContexts[testContext.parentContextId].parentContextId) { + testContext = testContexts[testContext.parentContextId]; + } return { test, + testContextName: testContext.name, }; } From 6874c83efd1987035241b01a9de43cc8014b96a6 Mon Sep 17 00:00:00 2001 From: Aleck Greenham Date: Sat, 6 May 2017 12:47:31 +0100 Subject: [PATCH 3/4] [TestSuite] Clean up TestSuite code further --- tests/lib/TestRun.js | 38 ++++++++++++++++++++++++-------------- tests/src/screens/Test.js | 28 +++++++++------------------- 2 files changed, 33 insertions(+), 33 deletions(-) diff --git a/tests/lib/TestRun.js b/tests/lib/TestRun.js index c927949d..593d6d3d 100644 --- a/tests/lib/TestRun.js +++ b/tests/lib/TestRun.js @@ -306,7 +306,7 @@ class TestRun { } async _safelyRunFunction(func, timeOutDuration, description) { - const syncResultOrPromise = tryCatcher(func); + const syncResultOrPromise = captureThrownErrors(func); if (syncResultOrPromise.error) { // Synchronous Error @@ -314,49 +314,59 @@ class TestRun { } // Asynchronous Error - return promiseToCallBack(syncResultOrPromise.value, timeOutDuration, description); + return capturePromiseErrors(syncResultOrPromise.result, timeOutDuration, description); } } /** - * Try catch to object - * @returns {{}} + * Call a function and capture any errors that are immediately thrown. + * @returns {Object} Object containing result of executing the function, or the error + * message that was captured * @private */ -function tryCatcher(func) { +function captureThrownErrors(func) { const result = {}; try { - result.value = func(); - } catch (e) { - result.error = e; + result.result = func(); + } catch (error) { + result.error = error; } return result; } /** - * Make a promise callback-able to trap errors - * @param promise + * Wraps a promise so that if it's rejected or an error is thrown while it's being + * evaluated, it's captured and thrown no further + * @param {*} target - Target to wrap. If a thenable object, it's wrapped so if it's + * rejected or an error is thrown, it will be captured. If a non-thenable object, + * wrapped in resolved promise and returned. + * @param {Number} timeoutDuration - Number of milliseconds the promise is allowed + * to pend before it's considered timed out + * @param {String} description - Description of the context the promises is defined + * in, used for reporting where a timeout occurred in the resulting error message. * @private */ -function promiseToCallBack(promise, timeoutDuration, description) { +function capturePromiseErrors(target, timeoutDuration, description) { let returnValue = null; try { - returnValue = Promise.resolve(promise) + returnValue = Promise.resolve(target) .then(() => { return null; }, (error) => { return Promise.resolve(error); }) - .timeout(timeoutDuration, `${description} took longer than ${timeoutDuration}ms. This can be extended with the timeout option.`) .catch((error) => { return Promise.resolve(error); - }); + }) + .timeout(timeoutDuration, + `${description} took longer than ${timeoutDuration}ms. This can be extended with the timeout option.` + ); } catch (error) { returnValue = Promise.resolve(error); } diff --git a/tests/src/screens/Test.js b/tests/src/screens/Test.js index 5ba50f27..b4499656 100644 --- a/tests/src/screens/Test.js +++ b/tests/src/screens/Test.js @@ -62,16 +62,16 @@ class Test extends React.Component { {testContextName}:{description} - Test Error - + Test Error + {message || 'None'} - + Test Code Preview - + {beautify(removeLastLine(removeFirstLine(func.toString())), { indent_size: 4, break_chained_methods: true })} @@ -102,27 +102,17 @@ const styles = StyleSheet.create({ flex: 1, backgroundColor: '#ffffff', }, - header: { + testLabel: { + padding: 5, + backgroundColor: '#0288d1', fontWeight: '600', - fontSize: 18, - backgroundColor: '#000', - color: '#fff', - padding: 5, - }, - code: { - backgroundColor: '#3F373A', - color: '#c3c3c3', - padding: 5, - fontSize: 12, + color: '#ffffff', + fontSize: 16 }, description: { padding: 5, fontSize: 14, }, - testLabel: { - fontWeight: '600', - fontSize: 16 - } }); function select({ tests, testContexts }, { navigation: { state: { params: { testId } } } }) { From 86d771ba1bf4ac1f41fb8ce9d2b938082b30eff4 Mon Sep 17 00:00:00 2001 From: Aleck Greenham Date: Sat, 6 May 2017 13:10:09 +0100 Subject: [PATCH 4/4] [TestSuite] Add stack traces to failed tests --- tests/lib/TestRun.js | 3 +-- tests/src/actions/TestActions.js | 3 ++- tests/src/reducers/testsReducers.js | 1 + tests/src/screens/Test.js | 26 +++++++++++++++----------- 4 files changed, 19 insertions(+), 14 deletions(-) diff --git a/tests/lib/TestRun.js b/tests/lib/TestRun.js index 593d6d3d..82bd4484 100644 --- a/tests/lib/TestRun.js +++ b/tests/lib/TestRun.js @@ -286,8 +286,7 @@ class TestRun { suiteId: this.testSuite.id, status: RunStatus.ERR, time: Date.now() - this.runStartTime, - message: `Test suite failed: ${error.message}`, - stackTrace: error.stack, + message: `Test suite failed: ${error.message}` }); }); } diff --git a/tests/src/actions/TestActions.js b/tests/src/actions/TestActions.js index a2ed5fe3..71487b0a 100644 --- a/tests/src/actions/TestActions.js +++ b/tests/src/actions/TestActions.js @@ -14,13 +14,14 @@ export function setSuiteStatus({ suiteId, status, time, message, progress }) { }; } -export function setTestStatus({ testId, status, time = 0, message = null }) { +export function setTestStatus({ testId, status, stackTrace, time = 0, message = null }) { return { type: TEST_SET_STATUS, testId, status, message, + stackTrace, time, }; } diff --git a/tests/src/reducers/testsReducers.js b/tests/src/reducers/testsReducers.js index 204e600f..cfd6fb07 100644 --- a/tests/src/reducers/testsReducers.js +++ b/tests/src/reducers/testsReducers.js @@ -12,6 +12,7 @@ function testsReducers(state = initState.tests, action: Object): State { flattened[`${action.testId}.status`] = action.status; flattened[`${action.testId}.message`] = action.message; flattened[`${action.testId}.time`] = action.time; + flattened[`${action.testId}.stackTrace`] = action.stackTrace; return unflatten(flattened); } diff --git a/tests/src/screens/Test.js b/tests/src/screens/Test.js index b4499656..d69cb412 100644 --- a/tests/src/screens/Test.js +++ b/tests/src/screens/Test.js @@ -52,25 +52,26 @@ class Test extends React.Component { } render() { - const { test: { message, description, func, status, time }, testContextName } = this.props; + const { test: { stackTrace, description, func, status, time }, testContextName } = this.props; return ( {Test.renderBanner({ status, time })} - - {testContextName}:{description} + + {testContextName} + {description} - - Test Error + + Test Error - {message || 'None'} + {stackTrace || 'None.'} - + Test Code Preview - + {beautify(removeLastLine(removeFirstLine(func.toString())), { indent_size: 4, break_chained_methods: true })} @@ -85,8 +86,8 @@ Test.propTypes = { test: PropTypes.shape({ status: PropTypes.string, time: PropTypes.number, - message: PropTypes.string, func: PropTypes.function, + stackTrace: PropTypes.function, description: PropTypes.string, }).isRequired, @@ -102,12 +103,15 @@ const styles = StyleSheet.create({ flex: 1, backgroundColor: '#ffffff', }, - testLabel: { + sectionContainer: { + minHeight: 100, + }, + heading: { padding: 5, backgroundColor: '#0288d1', fontWeight: '600', color: '#ffffff', - fontSize: 16 + fontSize: 16, }, description: { padding: 5,