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

BB-730: Rewrite test files using async/await syntax #979

Merged
merged 2 commits into from
Jun 5, 2023

Conversation

faraz16iqbal
Copy link
Contributor

Problem
BB-673:
Across the codebase, we have asynchronous code written using promises, which is hard to read and to maintain and error-prone.

Solution
All this code was rewritten using the async/await syntax.

Areas of Impact
The changes were reflected in test/common.js

@faraz16iqbal
Copy link
Contributor Author

@MonkeyDo can you check if this file is correctly modified? If yes, I will go ahead and convert all the other test files using the same format.

@MonkeyDo
Copy link
Member

MonkeyDo commented May 4, 2023

Sorry it took so long to answer @faraz16iqbal !
Yes, the modifications look great.
Should I merge the PR now or do you want to modify other files in the same PR?

@faraz16iqbal
Copy link
Contributor Author

@MonkeyDo thank you for the review! I think I'll use this PR to convert all the files in one go.

@faraz16iqbal
Copy link
Contributor Author

@MonkeyDo I mentioned there were some files that used the following syntax:
if (days === funRunnerDays) { editPromise = Promise.resolve(threshold); }

Do these also need to be changed?

@MonkeyDo
Copy link
Member

Looking at that file with that code I don't think it's necessary to rewrite those as an async function.
ESLint will tell you an async function needs an await expression, and we don't have anything to await in that in this particular case.
I think you can leave those as-is.

@faraz16iqbal
Copy link
Contributor Author

@MonkeyDo I have converted all tests files to follow the async-await syntax. Please review whenever you can!

@faraz16iqbal
Copy link
Contributor Author

@MonkeyDo waiting on this!

Copy link
Member

@MonkeyDo MonkeyDo left a comment

Choose a reason for hiding this comment

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

And thank you for this one as well ! :)

@MonkeyDo MonkeyDo merged commit 04e5ece into metabrainz:master Jun 5, 2023
@faraz16iqbal
Copy link
Contributor Author

And thank you for this one as well ! :)

Hahaha no worries at all! Love contributing however I can 👯

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.

2 participants