-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
refactor: modernize HTTPRequest tests #7604
refactor: modernize HTTPRequest tests #7604
Conversation
…ne callbacks, etc
Thanks for opening this pull request!
|
Yay! Thanks for this, i'm glad to see someone else working towards cleaning up the tests 🎉 |
Of course! Happy to help out. Let me know if there's anything I missed or could be written a better way 😃 |
I would normally say to prefer The other point I would make is to prefer template literal syntax in the URL request. But other than that looks good! |
Codecov Report
@@ Coverage Diff @@
## master #7604 +/- ##
=======================================
Coverage 93.94% 93.95%
=======================================
Files 181 181
Lines 13319 13319
=======================================
+ Hits 12513 12514 +1
+ Misses 806 805 -1
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Code modernization is too often overlooked unfortunately, even more glad to see this PR!
I will reformat the title to use the proper commit message syntax. |
Thanks for cleaning up the changelog & title! 😃 |
@brandongregoryscott if you would like to help with the modernization, cleaning up the tests really help. It gets a little more difficult with core code as we don't want to introduce huge merge conflicts that discourage open PRs. If you're thinking of cleaning up any code, feel free to open a draft PR at its inception and we can discuss if it's good to go, or if it's better to wait until conflicting PRs are merged. Thanks again for this PR and welcome to the Parse Contributor community 😃 |
🎉 This change has been released in version 5.0.0-beta.1 |
🎉 This change has been released in version 5.0.0 |
New Pull Request Checklist
Issue Description
Refactor some existing tests to use
async
/await
as described in #7563. Additionally, removes unnecessarydone
callbacks, and fixes some deprecation warnings with theBuffer
constructor.Related issue: Modernize Codebase #7563
Approach
Tests were refactored using a red/green testing strategy. All functionality/guards of the tests should remain the same.
TODOs before merging
None that I am aware of