From 41cc624ead0eab2b4610da1d6f75c0d0c5780bd3 Mon Sep 17 00:00:00 2001 From: Justin Coyne Date: Thu, 30 May 2019 10:16:22 -0500 Subject: [PATCH] Extract the mapping functions from state to properties so they can be tested One option for testing mapStateToProps directly would have required exporting private code as public, just for the purpose of testing. That's a bit of a code smell. Another option would be to pass a mock store into the component. That was something we could do in react-redux v5 and which was restored in v7, but it was removed in v6, which is what we are on. See https://github.com/reduxjs/react-redux/issues/1161 This seems like the best way of testing this mapping without upgrading react-redux or unnecessarily breaking encapsulation. --- __tests__/selectors.test.js | 67 +++++++++++++++++++ src/components/LoginPanel.jsx | 10 ++- src/components/editor/Editor.jsx | 4 +- .../editor/ImportResourceTemplate.jsx | 3 +- src/selectors.js | 8 +++ 5 files changed, 84 insertions(+), 8 deletions(-) create mode 100644 __tests__/selectors.test.js create mode 100644 src/selectors.js diff --git a/__tests__/selectors.test.js b/__tests__/selectors.test.js new file mode 100644 index 000000000..786995db4 --- /dev/null +++ b/__tests__/selectors.test.js @@ -0,0 +1,67 @@ +// Copyright 2019 Stanford University see LICENSE for license + +import { getCurrentUser, getCurrentSession, getAuthenticationError, getAuthenticationState } from '../src/selectors'; + +describe('getCurrentUser', () => { + const currentUser = { hello: 'world' } + + const state = { + authenticate: { + authenticationState: { + currentUser: currentUser + } + } + }; + + it('returns user', () => { + expect(getCurrentUser(state)).toBe(currentUser) + }) +}) + +describe('getCurrentSession', () => { + const currentSession = { hello: 'world' } + + const state = { + authenticate: { + authenticationState: { + currentSession: currentSession + } + } + }; + + it('returns currentSession', () => { + expect(getCurrentSession(state)).toBe(currentSession) + }) +}) + +describe('getAuthenticationError', () => { + const authenticationError = { hello: 'world' } + + const state = { + authenticate: { + authenticationState: { + authenticationError: authenticationError + } + } + }; + + it('returns authentication error', () => { + expect(getAuthenticationError(state)).toBe(authenticationError) + }) +}) + +describe('getAuthenticationState', () => { + const authenticationState = { authenticationError: 'broken' } + + const state = { + authenticate: { + authenticationState: authenticationState + } + }; + + it('returns a copy of the authentication state', () => { + const result = getAuthenticationState(state) + expect(result).not.toBe(authenticationState) + expect(result).toEqual(authenticationState) + }) +}) diff --git a/src/components/LoginPanel.jsx b/src/components/LoginPanel.jsx index 550279bf8..d79cce18e 100644 --- a/src/components/LoginPanel.jsx +++ b/src/components/LoginPanel.jsx @@ -6,7 +6,7 @@ import Config from '../Config' import CognitoUtils from '../CognitoUtils' import { connect } from 'react-redux' import { authenticationFailure, authenticationSuccess, signOutSuccess } from '../actions/index' - +import { getCurrentUser, getCurrentSession, getAuthenticationError } from '../selectors'; class LoginPanel extends Component { constructor(props){ @@ -126,13 +126,11 @@ LoginPanel.propTypes = { signout: PropTypes.func } -//TODO: make testing these part of the new tests you write (that is, unit test mapStateToProps and mapDispatchToProps, since -// you're testing the bare WrappedComponent, and not the HoC generated by calling connect(...)(LoginPanel)) const mapStateToProps = (state) => { return { - currentUser: state.authenticate.authenticationState ? state.authenticate.authenticationState.currentUser : null, - currentSession: state.authenticate.authenticationState ? state.authenticate.authenticationState.currentSession : null, - authenticationError: state.authenticate.authenticationState ? state.authenticate.authenticationState.authenticationError : null + currentUser: getCurrentUser(state), + currentSession: getCurrentSession(state), + authenticationError: getAuthenticationError(state) } } diff --git a/src/components/editor/Editor.jsx b/src/components/editor/Editor.jsx index b6b6dd161..57856396e 100644 --- a/src/components/editor/Editor.jsx +++ b/src/components/editor/Editor.jsx @@ -7,6 +7,8 @@ import PropTypes from 'prop-types' import ResourceTemplate from './ResourceTemplate' import Header from './Header' import RDFModal from './RDFModal' +import { getAuthenticationState } from '../../selectors'; + const _ = require('lodash') class Editor extends Component { @@ -87,7 +89,7 @@ Editor.propTypes = { const mapStateToProps = (state) => { return { - authenticationState: Object.assign({}, state.authenticate.authenticationState) + authenticationState: getAuthenticationState(state) } } diff --git a/src/components/editor/ImportResourceTemplate.jsx b/src/components/editor/ImportResourceTemplate.jsx index 66f0609b0..6691e8b4c 100644 --- a/src/components/editor/ImportResourceTemplate.jsx +++ b/src/components/editor/ImportResourceTemplate.jsx @@ -8,6 +8,7 @@ import SinopiaResourceTemplates from './SinopiaResourceTemplates' import UpdateResourceModal from './UpdateResourceModal' import { createResourceTemplate, updateResourceTemplate } from '../../sinopiaServer' import { connect } from 'react-redux' +import { getAuthenticationState } from '../../selectors'; class ImportResourceTemplate extends Component { constructor(props) { @@ -164,7 +165,7 @@ ImportResourceTemplate.propTypes = { const mapStateToProps = (state) => { return { - authenticationState: Object.assign({}, state.authenticate.authenticationState) + authenticationState: getAuthenticationState(state) } } diff --git a/src/selectors.js b/src/selectors.js new file mode 100644 index 000000000..fa47c7c4c --- /dev/null +++ b/src/selectors.js @@ -0,0 +1,8 @@ + +export const getCurrentUser = (state) => (getAuthenticationState(state)?.currentUser) + +export const getCurrentSession = (state) => (getAuthenticationState(state)?.currentSession) + +export const getAuthenticationError = (state) => (getAuthenticationState(state)?.authenticationError) + +export const getAuthenticationState = (state) => ({...state.authenticate.authenticationState})