Change main file tests to define sources inline and remove tibs testing infrastructure #952
Conversation
|
@swift-ci Please test |
ahoppen
force-pushed
the
ahoppen/no-more-tibs
branch
from
27e988b to
86c33aa
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
| targets: [ | ||
| .target( | ||
| name: "MyLibrary", | ||
| cSettings: [.define("VARIABLE_NAME", to: "fromMyLibrary"), .unsafeFlags(["-Wunused-variable"])] |
There was a problem hiding this comment.
Took me a little bit to realize why we were doing this - worth a comment IMO (ie. that we're defining VARIABLE_NAME here to make sure that the build settings from the main file are being used for the header).
There was a problem hiding this comment.
I mentioned that we use VARIABLE_NAME to communicate the build settings in all test cases now.
| "Sources/MyLibrary/MyLibrary.c": """ | ||
| #include "$TEST_DIR/Sources/shared.h" | ||
| """, | ||
| "Sources/MyFancyLibrary/include/MyFancyLibraryh": "", |
There was a problem hiding this comment.
This file is unused isn't it?
There was a problem hiding this comment.
SwiftPM refuses to build if a C target does not have an include directory. And in the current infrastructure, the only way to create a directory is to put a file in there.
There was a problem hiding this comment.
I see. That's... okay. Could just add a dummy or something though (to make that more clear)
There was a problem hiding this comment.
Good idea. Renamed to dummy.h.
| @@ -134,81 +134,6 @@ Where "N" configures the log verbosity and is one of the following numbers: 0 (e | |||
|
|
|||
| As much as is practical, all code should be covered by tests. New tests can be added under the `Tests` directory and should use `XCTest`. The rest of this section will describe the additional tools available in the `SKTestSupport` module to make it easier to write good and efficient tests. | |||
|
|
|||
There was a problem hiding this comment.
I assume your opinion here is that the tests are now obvious enough without any description?
There was a problem hiding this comment.
My opinion here is that we should write more documentation that’s up-to-date.
This paragraph in particular doesn’t add any value IMO. It’s mostly just: There should be tests and we use XCTest, which, I think is obvious.
There was a problem hiding this comment.
To be clear, my comment was just on the first changed line.
My opinion here is that we should write more documentation that’s up-to-date.
Sounds good 👍
There was a problem hiding this comment.
OK, for those, I would say that
As much as is practical, all code should be covered by tests. New tests can be added under the
Testsdirectory and should useXCTest.
is obvious and not something that needs to be mentioned in documentation.
ahoppen
force-pushed
the
ahoppen/no-more-tibs
branch
from
86c33aa to
2020c98
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
The tests are structured quite differently now but I think the new, smaller test cases cover all the interesting cases from the old test.
All test have been migrated to define their sources inline, so we can remove the usage of the TIBS test infrastructure, also removing our dependency on `ninja` to run tests.
ahoppen
force-pushed
the
ahoppen/no-more-tibs
branch
from
2020c98 to
2d0c818
Compare
|
@swift-ci Please test |
|
@swift-ci Please test Windows |
Migrate the last tests that used the tibs workspace to define sources inline and remove the testing infrastructure that relied on TIBS.
Resolves #879