Skip to content

Commit

Permalink
test cleanup (#1165)
Browse files Browse the repository at this point in the history
* 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'
```
  • Loading branch information
zburke authored Feb 21, 2022
1 parent ab2e1c5 commit ae1665d
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 31 deletions.
8 changes: 4 additions & 4 deletions .github/workflows/build-npm.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions src/components/About/WarningBanner.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ const WarningBanner = ({
<div className={css.warningContainer}>
<MessageBanner
type="warning"
show={missingModulesCount}
show={!!missingModulesCount}
dismissable
>
<Headline>{missingModulesMsg}</Headline>
Expand All @@ -58,7 +58,7 @@ const WarningBanner = ({

<MessageBanner
type="warning"
show={incompatibleModulesCount}
show={!!incompatibleModulesCount}
dismissable
>
<Headline>{incompatibleModuleMsg}</Headline>
Expand Down
11 changes: 7 additions & 4 deletions src/components/AppIcon/tests/appIcon-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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(
<AppIcon
Expand All @@ -139,5 +138,9 @@ describe('AppIcon', () => {
expect(appIcon.img.offsetHeight).to.equal(iconSizes[size]);
});
});
});
};

sizeTest('small');
sizeTest('medium');
sizeTest('large');
});
3 changes: 0 additions & 3 deletions src/components/CreateResetPassword/CreateResetPassword.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
34 changes: 19 additions & 15 deletions src/components/CreateResetPassword/CreateResetPasswordControl.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,15 @@ class CreateResetPasswordControl extends Component {
}

async componentDidMount() {
this._isMounted = true;
await this.makeCall();

this.setState({ isLoading: false });
}

componentWillUnmount() {
this.props.clearAuthErrors();
this._isMounted = false;
}

handleResponse = (response) => {
Expand Down Expand Up @@ -83,7 +85,7 @@ class CreateResetPasswordControl extends Component {
}
};

makeCall = async (body) => {
makeCall = (body) => {
const {
stripes: {
okapi: {
Expand All @@ -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) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,6 @@ class PasswordHasNotChanged extends Component {
<Headline
size="x-large"
tag="p"
bold={false}
faded
data-test-message
>
Expand Down
2 changes: 1 addition & 1 deletion test/bigtest/network/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);


Expand Down
15 changes: 15 additions & 0 deletions test/bigtest/network/scenarios/changePasswordSuccess.js
Original file line number Diff line number Diff line change
@@ -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, {}, '');
Expand Down
2 changes: 1 addition & 1 deletion test/bigtest/network/scenarios/forgotUsernameSuccess.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
export default (server) => {
server.post('bl-users/forgotten/username', () => {
return new Response(204, {}, '');
}, 204);
});
};
1 change: 1 addition & 0 deletions test/bigtest/tests/useCustomFields-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const setupWithApp = (App, title) => setupApplication({
}],
translations: {
'dummy.title': title,
'login.title': 'login title',
},
});

Expand Down

0 comments on commit ae1665d

Please sign in to comment.