IGListKit
IGListKit copied to clipboard
Instagram
Run unit tests for tvOS
Changes in this pull request
As mentioned before in #1395, the unit tests are not executed for tvOS on the CI.
As you will see, they won't pass either.
The later commit in this PR removes certain files from the tvOS test target to make them pass. Namely the tests concerning nibs/storyboards, since those are not supported on tvOS.
Checklist
- [x] All tests pass. Demo project builds and runs.
- [x] I added tests, an experiment, or detailed why my change isn't tested.
- [ ] I added an entry to the
CHANGELOG.mdfor any breaking changes, enhancements, or bug fixes. - [x] I have reviewed the contributing guide
We'd better create a different set of Test files for tvOS.
How do you envision this? That we copy relevant test files to a TVOSTests folder or something? But then again, almost all tests pass for tvOS, so why would you want to duplicate tests?
Another thing I noticed, there are quite a few tests prefixed with DISABLED_, do we have to keep them?
DISABLED_, do we have to keep them?
I think those are flaky tests and we have internal CI to auto-prefix with DISABLED_ to disable the test, the solution should actually fix the broken fix and remove that prefix here.
That we copy relevant test files to a TVOSTests folder or something?
Nah, def not duplicate the test files here. If all the tests work for iOS target, we should have pretty good confidence that the code works for tv_os. I would rather we only add tv_os specific test cases instead of de-colocate tests in IGListAdapterTests.m.
Also from the optimization perspective, we should def focus more on iOS instead of tv_os, which had way less usage than the iOS use cases.
Can you try add @available(iOS 11.0) for those two test cases inline inside IGListAdapterTests.m ?
Can you try add @available(iOS 11.0) for those two test cases inline inside
IGListAdapterTests.m?
@available didn't work, but using the TARGET_OS_TV macro does.
Lemme just pipe up in here to say I'm looking at this. I'm actually surprised we didn't have the tests running on tvOS before.
There's some additional hurdles to handle now (Such as the test XIB files needing to be duplicated and modified for tvOS support), but I'd definitely like to get this set up when we can!
