Skip to content

TestFoundation: excise tests that use private APIs #2012

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

Closed
wants to merge 1 commit into from

Conversation

compnerd
Copy link
Member

Since we do not expose the CoreFoundation APIs fully these tests are not
possible to build since the symbols cannot be resolved. Rather than
adding additional exports to CoreFoundation for testing only, just drop
the tests.

@compnerd
Copy link
Member Author

@millenomi - was this the solution that you preferred rather than having a new macro to expose the functions?

@spevans
Copy link
Contributor

spevans commented Mar 15, 2019

Could internal methods be added to wrap these then testing enabled inside NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT blocks? I don't like the idea of disabling so many tests

Copy link
Contributor

@millenomi millenomi left a comment

Choose a reason for hiding this comment

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

We're not disabling tests on platforms where they can run.

I'm okay wrapping these in a SPI file in Foundation that is marked for testing only, as API importable via @testable import or similar. Or disabling them on Windows. But not disabling them tout court.

@millenomi
Copy link
Contributor

Or: use @_cdecl to pull in strictly SPI we need in the test target.

@compnerd
Copy link
Member Author

Using @_cdecl wont work, since the symbols are not available through Foundation.dll. Wrapping them in an SPI is probably nicer.

@compnerd
Copy link
Member Author

The only test which needed to be removed is handled elsewhere. The issues that this was tracking are no longer a problem.

@compnerd compnerd closed this Apr 17, 2019
@compnerd compnerd deleted the no-test-for-you branch April 17, 2019 23:47
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