From ae1665d9af2b3d3683f3b5f96beb60ebc0ee2484 Mon Sep 17 00:00:00 2001 From: Zak Burke Date: Mon, 21 Feb 2022 16:18:34 -0500 Subject: [PATCH] test cleanup (#1165) * provide missing translation to avoid console pollution * await promises in loops. otherwise, iteration will will proceed asynchronously and check the icon sizes incorrectly, I think. * clean some 204 responses, #948 notwithstanding * do not set deprecated props on `Headline` * correctly set props on `MessageBanner` * avoid `setState` in then blocks if component is unmounted * compile translations _before_ running test to avoid this console noise: ``` WARN: '[@formatjs/intl] "defaultRichTextElements" was specified but "message" was not pre-compiled. Please consider using "@formatjs/cli" to pre-compile your messages for performance. For more details see https://formatjs.io/docs/getting-started/message-distribution' ``` --- .github/workflows/build-npm.yml | 8 ++--- src/components/About/WarningBanner.js | 4 +-- src/components/AppIcon/tests/appIcon-test.js | 11 +++--- .../CreateResetPassword.js | 3 -- .../CreateResetPasswordControl.js | 34 +++++++++++-------- .../PasswordHasNotChanged.js | 1 - test/bigtest/network/config.js | 2 +- .../scenarios/changePasswordSuccess.js | 15 ++++++++ .../scenarios/forgotUsernameSuccess.js | 2 +- test/bigtest/tests/useCustomFields-test.js | 1 + 10 files changed, 50 insertions(+), 31 deletions(-) diff --git a/.github/workflows/build-npm.yml b/.github/workflows/build-npm.yml index 6af196baa..b1243bd82 100644 --- a/.github/workflows/build-npm.yml +++ b/.github/workflows/build-npm.yml @@ -67,13 +67,13 @@ jobs: run: yarn lint continue-on-error: true - - name: Run yarn test - run: xvfb-run --server-args="-screen 0 1024x768x24" yarn test $YARN_TEST_OPTIONS - - name: Run yarn formatjs-compile if : ${{ env.COMPILE_TRANSLATION_FILES == 'true' }} run: yarn formatjs-compile + - name: Run yarn test + run: xvfb-run --server-args="-screen 0 1024x768x24" yarn test $YARN_TEST_OPTIONS + - name: Generate FOLIO module descriptor if: ${{ env.PUBLISH_MOD_DESCRIPTOR == 'true' }} run: yarn build-mod-descriptor @@ -165,7 +165,7 @@ jobs: - name: Exclude some CI-generated artifacts in package if: ${{ github.ref == 'refs/heads/master' || github.ref == 'refs/heads/main' }} - run: | + run: | echo ".github" >> .npmignore echo ".scannerwork" >> .npmignore cat .npmignore diff --git a/src/components/About/WarningBanner.js b/src/components/About/WarningBanner.js index d33538d18..ab85b7eac 100644 --- a/src/components/About/WarningBanner.js +++ b/src/components/About/WarningBanner.js @@ -46,7 +46,7 @@ const WarningBanner = ({
{missingModulesMsg} @@ -58,7 +58,7 @@ const WarningBanner = ({ {incompatibleModuleMsg} diff --git a/src/components/AppIcon/tests/appIcon-test.js b/src/components/AppIcon/tests/appIcon-test.js index 42de68b92..7d2323dc6 100644 --- a/src/components/AppIcon/tests/appIcon-test.js +++ b/src/components/AppIcon/tests/appIcon-test.js @@ -121,9 +121,8 @@ describe('AppIcon', () => { }); }); - - Object.keys(iconSizes).forEach(size => { - describe(`Passing a size of "${size}"`, () => { + const sizeTest = async (size) => { + describe(`Passing a size of "${size}"`, async () => { beforeEach(async () => { await mount( { expect(appIcon.img.offsetHeight).to.equal(iconSizes[size]); }); }); - }); + }; + + sizeTest('small'); + sizeTest('medium'); + sizeTest('large'); }); diff --git a/src/components/CreateResetPassword/CreateResetPassword.js b/src/components/CreateResetPassword/CreateResetPassword.js index 7cd07ec66..931e7dfbd 100644 --- a/src/components/CreateResetPassword/CreateResetPassword.js +++ b/src/components/CreateResetPassword/CreateResetPassword.js @@ -36,9 +36,6 @@ class CreateResetPassword extends Component { onPasswordInputFocus: PropTypes.func.isRequired, submitting: PropTypes.bool, submitIsFailed: PropTypes.bool, - form: PropTypes.shape({ - getState: PropTypes.func.isRequired, - }).isRequired, }; static defaultProps = { diff --git a/src/components/CreateResetPassword/CreateResetPasswordControl.js b/src/components/CreateResetPassword/CreateResetPasswordControl.js index 0ae8c5f7c..c6abbcb4d 100644 --- a/src/components/CreateResetPassword/CreateResetPasswordControl.js +++ b/src/components/CreateResetPassword/CreateResetPasswordControl.js @@ -43,6 +43,7 @@ class CreateResetPasswordControl extends Component { } async componentDidMount() { + this._isMounted = true; await this.makeCall(); this.setState({ isLoading: false }); @@ -50,6 +51,7 @@ class CreateResetPasswordControl extends Component { componentWillUnmount() { this.props.clearAuthErrors(); + this._isMounted = false; } handleResponse = (response) => { @@ -83,7 +85,7 @@ class CreateResetPasswordControl extends Component { } }; - makeCall = async (body) => { + makeCall = (body) => { const { stripes: { okapi: { @@ -102,21 +104,23 @@ class CreateResetPasswordControl extends Component { const path = `${url}/bl-users/password-reset/${isValidToken ? 'reset' : 'validate'}`; - try { - const response = await fetch(path, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - 'x-okapi-token': token, - 'x-okapi-tenant': tenant, - }, - ...(body && { body: JSON.stringify(body) }), + fetch(path, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + 'x-okapi-token': token, + 'x-okapi-tenant': tenant, + }, + ...(body && { body: JSON.stringify(body) }), + }) + .then((response) => { + if (this._isMounted) { + this.handleResponse(response); + } + }) + .catch(error => { + handleBadResponse(error); }); - - this.handleResponse(response); - } catch (error) { - handleBadResponse(error); - } }; handleSubmit = async (values) => { diff --git a/src/components/CreateResetPassword/components/PasswordHasNotChanged/PasswordHasNotChanged.js b/src/components/CreateResetPassword/components/PasswordHasNotChanged/PasswordHasNotChanged.js index bbb7b089a..4ebe2b9d1 100644 --- a/src/components/CreateResetPassword/components/PasswordHasNotChanged/PasswordHasNotChanged.js +++ b/src/components/CreateResetPassword/components/PasswordHasNotChanged/PasswordHasNotChanged.js @@ -55,7 +55,6 @@ class PasswordHasNotChanged extends Component { diff --git a/test/bigtest/network/config.js b/test/bigtest/network/config.js index 9335733de..82e58f915 100644 --- a/test/bigtest/network/config.js +++ b/test/bigtest/network/config.js @@ -40,7 +40,7 @@ export default function configure() { this.get('/bl-users/_self', {}); this.post('/bl-users/password-reset/validate', () => { return new Response(204, {}, ''); - }, 204); + }); this.post('/bl-users/password-reset/reset', {}, 401); diff --git a/test/bigtest/network/scenarios/changePasswordSuccess.js b/test/bigtest/network/scenarios/changePasswordSuccess.js index 52a133dec..2f6f04d83 100644 --- a/test/bigtest/network/scenarios/changePasswordSuccess.js +++ b/test/bigtest/network/scenarios/changePasswordSuccess.js @@ -1,3 +1,18 @@ +// the CORRECT implementation of this response is: +// +// export default (server) => { +// server.post('/bl-users/password-reset/reset', () => { +// return new Response(204, {}, ''); +// }); +// }; +// +// but for reasons unclear this results in a 201 instead of a 204. +// for other responses, e.g. forgotUsernameSuccess, a 204 is returned. +// without the additional 204, createResetPassword-test will fail. +// there's gotta be a config glitch somewhere, but I have not been +// able to find it. so SO frustrating. +// + export default (server) => { server.post('/bl-users/password-reset/reset', () => { return new Response(204, {}, ''); diff --git a/test/bigtest/network/scenarios/forgotUsernameSuccess.js b/test/bigtest/network/scenarios/forgotUsernameSuccess.js index 1fce44b50..eed7af4a5 100644 --- a/test/bigtest/network/scenarios/forgotUsernameSuccess.js +++ b/test/bigtest/network/scenarios/forgotUsernameSuccess.js @@ -1,5 +1,5 @@ export default (server) => { server.post('bl-users/forgotten/username', () => { return new Response(204, {}, ''); - }, 204); + }); }; diff --git a/test/bigtest/tests/useCustomFields-test.js b/test/bigtest/tests/useCustomFields-test.js index b135bd9b5..89bc4560c 100644 --- a/test/bigtest/tests/useCustomFields-test.js +++ b/test/bigtest/tests/useCustomFields-test.js @@ -27,6 +27,7 @@ const setupWithApp = (App, title) => setupApplication({ }], translations: { 'dummy.title': title, + 'login.title': 'login title', }, });