fix: added url path encoding #376
Conversation
|
Pull request title looks good 👍! If this pull request gets merged, it will cause a new release of the software. Example: If this project's latest release version is All merged pull requests will eventually get deployed. But some types of pull requests will trigger a deployment (such as features and bug fixes) while some pull requests will wait to get deployed until a later time. This project uses a special format for pull requests titles. Expand this section to learn more (expand by clicking the ᐅ symbol on the left side of this sentence)...This project uses a special format for pull requests titles. Don't worry, it's easy! This pull request title should be in this format: If your pull request introduces breaking changes to the code, use this format: where
Examples:Need more examples? Want to learn more about this format? Check out the official docs. Note: If your pull request does multiple things such as adding a feature and makes changes to the CI server and fixes some bugs then you might want to consider splitting this pull request up into multiple smaller pull requests. |
Sample app builds 📱Below you will find the list of the latest versions of the sample apps. It's recommended to always download the latest builds of the sample apps to accurately test the pull request.
|
| @@ -44,4 +44,29 @@ class HttpEndpointTest: UnitTest { | |||
|
|
|||
| XCTAssertEqual(actual, expected) | |||
| } | |||
|
|
|||
| func test_getUrlString_givenRandomPathNeedsEncoding_expectEncodedPath() { | |||
| let identifierWithSpecialChar = String.randomStringWithSpecialCharacters() | |||
There was a problem hiding this comment.
I would prefer if we hard-coded test data instead of randomly generate it. Maybe the randomly generated string gives us a char with no special chars, for example.
The other test function is great in that it includes a special character that we have identified causes issues for customers. I like the idea of hard-coding test strings that we know could cause an issue.
There was a problem hiding this comment.
Good point, but random values does extend range for testing more characters. We can perhaps update randomStringWithSpecialCharacters to must include special character every time.
There was a problem hiding this comment.
What if we hard-code abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-._~:/?#[]@!$&'()*+,;=% as the identifier?
I think that would do the same thing as randomStringWithSpecialCharacters if not be more valuable because every time we run the test suite, we are checking all special characters intead of some.
| @@ -31,6 +31,11 @@ public extension String { | |||
| String((0 ..< length).map { _ in abcLetters.randomElement()! }) | |||
| } | |||
|
|
|||
| static func randomStringWithSpecialCharacters(length: Int = 20) -> String { | |||
There was a problem hiding this comment.
If we do end up using this function, move it into Tests/Shared since it's a function used only for tests.
## [2.8.1](2.8.0...2.8.1) (2023-08-31) ### Bug Fixes * added url path encoding ([#376](#376)) ([fbfa384](fbfa384))
closes: customerio/customerio-reactnative#191
Complete each step to get your pull request merged in. Learn more about the workflow this project uses.