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

[API-789] Fix async unit tests #99

Merged
merged 4 commits into from Feb 1, 2022
Merged

Conversation

elseee
Copy link
Contributor

@elseee elseee commented Jan 31, 2022

https://bynder.atlassian.net/browse/API-789

  • Fixed some async tests which were not waiting for the promise to complete
  • Fixed the issues which arose from some tests that turned out to be false positives

const { fileId, mediaId, additional } = data;
const { fileId, mediaId, additional, brandId } = data;

if (!brandId && !mediaId) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need either the brandId (for a new asset) or the mediaId (for a newer version of an existing asset) to save an asset.

Copy link
Contributor

Choose a reason for hiding this comment

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

But this condition is requesting both parameters to be defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The condition is checking if both of them are undefined, hence the !

@@ -131,77 +129,65 @@ describe('#getToken', () => {

describe('#uploadFile', () => {
it('throws an error with no brand ID', () => {
bynder.uploadFile({
return expect(bynder.uploadFile({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To make sure that jest waits for the promise to complete we need to either return the promise from the test, or await it. Also by the expect(...).rejects pattern we ensure that the test fails if the promise were to resolve.

@coveralls
Copy link

coveralls commented Feb 1, 2022

Coverage Status

Coverage increased (+0.4%) to 94.896% when pulling 9b5291e on API-789-fix-failing-tests into b1a4ace on development.

@elseee elseee requested a review from betacar February 1, 2022 12:15
Copy link
Contributor

@betacar betacar left a comment

Choose a reason for hiding this comment

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

@elseee elseee merged commit d3f8653 into development Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants