-
Notifications
You must be signed in to change notification settings - Fork 49
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
Unit tests and refactoring to make it testable #84
Conversation
dependencies: [ | ||
"PostHog", | ||
"Quick", | ||
"Nimble", | ||
"OHHTTPStubs", | ||
.product(name: "OHHTTPStubsSwift", package: "OHHTTPStubs"), | ||
], |
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.
Here is what had to be done to make it work in CI, in combination with swift test
instead of the xcode command for some reason.
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.
Xcode already defines those dependencies as SPM in its projects, but apparently, CI wasn't picking them up, maybe a Xcode version mismatch, an empty project worked just fine.
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.
Mostly skimmed the tests, but the actual functional changes all seem reasonable. Mammoth effort!
What does this PR do?
Add tests for almost every class, also has a lot of fixing found during testing.
Where should the reviewer start?
How should this be manually tested?
Any background context you want to provide?
More tests can be added specifically for
PostHogQueue
, but its ok to add it later as well, its a bit complicated due to time advancing, and theURLSession
callback approach, but I can expand it in another PR so we can unblock this PR that is getting quite big already.What are the relevant tickets?
Screenshots or screencasts (if UI/UX change)
Questions: