Skip to content

Set NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT when ENABLE_TESTING is set. #2641

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Feb 5, 2020
Merged

Set NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT when ENABLE_TESTING is set. #2641

merged 1 commit into from
Feb 5, 2020

Conversation

spevans
Copy link
Contributor

@spevans spevans commented Feb 2, 2020

  • Disable TestThread.test_threadName() as some of the tests are
    currently broken.

@spevans
Copy link
Contributor Author

spevans commented Feb 2, 2020

@swift-ci test

@spevans
Copy link
Contributor Author

spevans commented Feb 3, 2020

@compnerd Could you test this PR on windows please, I want to check that it doesnt enable any tests that are broken on windows. Thanks

@compnerd
Copy link
Member

compnerd commented Feb 3, 2020

https://dev.azure.com/compnerd/swift-build/_build/results?buildId=25540&view=results

@compnerd
Copy link
Member

compnerd commented Feb 3, 2020

https://dev.azure.com/compnerd/swift-build/_build/results?buildId=25540&view=logs&j=ec47c80d-c7cf-5f2b-7138-13cd268bea3d&t=14607ae4-e7c0-519c-1971-8793f52aee42 ... definitely caused failure

@compnerd
Copy link
Member

compnerd commented Feb 3, 2020

A run with my fix for the Boolean conversion:
https://dev.azure.com/compnerd/swift-build/_build/results?buildId=25545&view=logs&j=ec47c80d-c7cf-5f2b-7138-13cd268bea3d&t=14607ae4-e7c0-519c-1971-8793f52aee42&l=10927

This does cause a regression:
https://dev.azure.com/compnerd/swift-build/_build/results?buildId=25545&view=logs&j=ec47c80d-c7cf-5f2b-7138-13cd268bea3d&t=14607ae4-e7c0-519c-1971-8793f52aee42&l=10927

@spevans spevans force-pushed the pr_testable_import branch from e2b03ce to fed521d Compare February 3, 2020 08:51
@spevans
Copy link
Contributor Author

spevans commented Feb 3, 2020

@swift-ci test

1 similar comment
@spevans
Copy link
Contributor Author

spevans commented Feb 3, 2020

@swift-ci test

@spevans
Copy link
Contributor Author

spevans commented Feb 3, 2020

@swift-ci test macos

1 similar comment
@spevans
Copy link
Contributor Author

spevans commented Feb 3, 2020

@swift-ci test macos

@spevans
Copy link
Contributor Author

spevans commented Feb 3, 2020

@compnerd I disabled a test on Windows where the functionality isnt implemented, could you retest when you get a chance? thanks

@compnerd
Copy link
Member

compnerd commented Feb 3, 2020

https://dev.azure.com/compnerd/swift-build/_build/results?buildId=25579&view=results

@spevans
Copy link
Contributor Author

spevans commented Feb 3, 2020

@compnerd it looks like it built on Windows but there were a large number of test failures, is that expected?

@compnerd
Copy link
Member

compnerd commented Feb 3, 2020 •
edited
Loading

The number of failures seems to have increased with this change (by at least 2)

@spevans
Copy link
Contributor Author

spevans commented Feb 3, 2020

This enabled about 30 extra tests functions so that doesnt seem unreasonable. Is this a blocker to merging?

@compnerd
Copy link
Member

compnerd commented Feb 3, 2020

I really would prefer not to regress the Windows builds further. It seems that the number of failing tests on Windows in Foundation has been increasing. I'd really prefer to not regress it further.

@spevans
Copy link
Contributor Author

spevans commented Feb 3, 2020

I'll disable the NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT on Windows for now

@spevans spevans force-pushed the pr_testable_import branch from fed521d to 6091963 Compare February 3, 2020 21:56
@spevans
Copy link
Contributor Author

spevans commented Feb 3, 2020

@swift-ci test

1 similar comment
@spevans
Copy link
Contributor Author

spevans commented Feb 4, 2020

@swift-ci test

@compnerd
Copy link
Member

compnerd commented Feb 4, 2020

Do you mind just disabling the particular tests that are failing instead?

@spevans spevans force-pushed the pr_testable_import branch from 6091963 to 6c7d249 Compare February 4, 2020 10:53
@spevans
Copy link
Contributor Author

spevans commented Feb 4, 2020

@swift-ci test

2 similar comments
@spevans
Copy link
Contributor Author

spevans commented Feb 4, 2020

@swift-ci test

@spevans
Copy link
Contributor Author

spevans commented Feb 4, 2020

@swift-ci test

@spevans
Copy link
Contributor Author

spevans commented Feb 4, 2020

@compnerd could you retest this on windows, it looks like the logs from the last run have been removed so I cant work out what extra tests this has broken on windows

@compnerd
Copy link
Member

compnerd commented Feb 4, 2020

https://dev.azure.com/compnerd/swift-build/_build/results?buildId=25643&view=results

@compnerd
Copy link
Member

compnerd commented Feb 4, 2020 •
edited
Loading

Foundation Tests Log: https://dev.azure.com/compnerd/swift-build/_build/results?buildId=25643&view=logs&j=ec47c80d-c7cf-5f2b-7138-13cd268bea3d&t=5cfc8e20-74e6-5b9b-ff2a-9c787c40fff4

@spevans
Set NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT when ENABLE_TESTING is set.
30ac274 
- Disable TestThread.test_threadName() as some of the tests are
  currently broken.

- On Windoes, disable testing the following that still fail:

   FileManager._replaceItem()
   Bundle.main.executableURL
   Bundle.init?(_executableURL:)
   TestFileHandle.testOffset()
@spevans spevans force-pushed the pr_testable_import branch from 6c7d249 to 30ac274 Compare February 4, 2020 17:34
@spevans
Copy link
Contributor Author

spevans commented Feb 4, 2020

@swift-ci test

1 similar comment
@spevans
Copy link
Contributor Author

spevans commented Feb 4, 2020

@swift-ci test

@spevans
Copy link
Contributor Author

spevans commented Feb 4, 2020

@compnerd I think I've disabled all of the NS_FOUNDATION_ALLOWS_TESTABLE_IMPORT tests on windows that are still broken now, so could you kick off another test please

@compnerd
Copy link
Member

compnerd commented Feb 4, 2020

https://dev.azure.com/compnerd/swift-build/_build/results?buildId=25659&view=results

@spevans
Copy link
Contributor Author

spevans commented Feb 5, 2020

@swift-ci test and merge

1 similar comment
@spevans
Copy link
Contributor Author

spevans commented Feb 5, 2020

@swift-ci test and merge

@swift-ci swift-ci merged commit 6bd47ad into swiftlang:master Feb 5, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@spevans @compnerd @swift-ci