Add tests for utility functions of library #23903
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
the
CLA Signed
|
|
@gandreadis these types of PRs are loved! |
|
Thanks! I have no idea why that one CircleCI build is failing, possibly a flaky test? If so, could someone with the right permissions rerun that check? It does not seem to be related to my changes, in any case. |
cpojer
approved these changes
Mar 15, 2019
facebook-github-bot
approved these changes
Mar 15, 2019
There was a problem hiding this comment.
@cpojer is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
@gandreadis These types of PRs are extremely appreciated, thank you!! Please keep sending them :) |
|
This pull request was successfully merged by @gandreadis in 47e0615. When will my fix make it into a release? | Upcoming Releases |
facebook-github-bot
pushed a commit
that referenced
this pull request
Mar 18, 2019
Summary: This PR add tests for several utilities in `Libraries/Utilities`, as a follow-up of #23903. The following utilities are now tested: * `clamp.js` * `binareToBase64.js` * `DeviceInfo.js` * `mergeIntoFast.js` * `PixelRatio.js` * `infoLog.js` * `logError.js` * `warnOnce.js` * `mapWithSeparator` (added a missing test) Not applicable, since it only adds tests. Pull Request resolved: #23989 Differential Revision: D14502806 Pulled By: cpojer fbshipit-source-id: e2c3b3a35f4f765d5336b998ab92dba14eeac7bc
facebook-github-bot
pushed a commit
that referenced
this pull request
Mar 18, 2019
Summary: This PR adds a number of unit tests for the Geolocation module, as a follow-up of #23903. I also added two missing documentation strings to that module, with references to the online documentation, for consistency with the other methods in the same module. Not applicable, since it only adds tests. Pull Request resolved: #23987 Differential Revision: D14502848 Pulled By: cpojer fbshipit-source-id: 8f7c1cee6be3fae081d9770e5e942fadda65e6c2
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
I was looking at the coverage report of the JavaScript code in the
Librariesfolder, and found some of the modules and functions to be (partially) untested. I believe that adding tests to them would formally capture their behaviour and avoid future regressions. In this PR, I've added some unit tests for 3 utility components.Perhaps a more general question: Are these kinds of PRs appreciated? I'd be interested in submitting more of them in the future.
Changelog
Not applicable, since it only adds tests.
Test Plan
This is a testing PR, so I do not believe that is necessary :)