Skip to content

Conversation

cbaker6
Copy link
Contributor

@cbaker6 cbaker6 commented Jul 22, 2020

Items added:

  • URLMock - this can be used for mocking any url call or error response
  • APICommandTests
  • ParseObjectCommandTests
  • UserCommandTests
  • Modernized URLSession to leverage Result type

This is ready for review...

@cbaker6
Copy link
Contributor Author

cbaker6 commented Jul 22, 2020

@pranjalsatija I attempted to minimize merge conflicts with #13 by touching little of the code you edited. I know you made a modification to getEncoder, you will see that I had to slightly modify getEncoder by creating another that doesn't skip keys. This was done for testing purposes as I needed something that would act like the parse-server with keys like objectId, createdAt, etc that would encode. You will need to do something similar so we can mock the server in tests

@cbaker6 cbaker6 mentioned this pull request Jul 23, 2020
8 tasks
@cbaker6
Copy link
Contributor Author

cbaker6 commented Jul 24, 2020

@TomWFox @pranjalsatija when you get a chance can you look this over? It’s mostly unit tests verifying that the codebase is working as expected.It also has some minor updates

Copy link
Contributor

@pranjalsatija pranjalsatija left a comment

Choose a reason for hiding this comment

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

Everything looks good except for a few really minor things!

@pranjalsatija
Copy link
Contributor

LGTM! We can touch one or two things up once all the open PRs are merged.

@TomWFox
Copy link
Contributor

TomWFox commented Jul 29, 2020

@pranjalsatija can you take another look at this and then I can merge?

@cbaker6
Copy link
Contributor Author

cbaker6 commented Jul 29, 2020

@pranjalsatija the only real changes I made since your last review are the minor ones I made to MockURLProtocol.swift and MockURLResponse.swift, the other SPM files I added, I deleted later as they weren't needed

Copy link
Contributor

@pranjalsatija pranjalsatija left a comment

Choose a reason for hiding this comment

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

LGTM! Sorry about the delay / general unresponsiveness, I'm out of town this week, but I'll be back in action on Friday.

@cbaker6
Copy link
Contributor Author

cbaker6 commented Jul 29, 2020

@TomWFox no more changes from me on this one

Copy link
Contributor

@TomWFox TomWFox left a comment

Choose a reason for hiding this comment

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

Thanks for changing the org name!

@TomWFox TomWFox merged commit 4828762 into parse-community:master Jul 29, 2020
@cbaker6 cbaker6 deleted the unit_tests branch July 30, 2020 13:00
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.

3 participants