tests: add a workaround for SPM tests #6853
Conversation
compnerd
requested review from
abertelrud,
neonichu,
tomerd and
MaxDesiatov
as code owners
|
@swift-ci please smoke test |
| @@ -328,7 +328,7 @@ class SourceControlPackageContainerTests: XCTestCase { | |||
| func testPreReleaseVersions() throws { | |||
| let fs = InMemoryFileSystem() | |||
|
|
|||
| let repoPath = AbsolutePath.root | |||
| let repoPath = AbsolutePath.root.appending("SourceCache") | |||
There was a problem hiding this comment.
I'm not sure I can connect the description of the PR to the actual change here. It mentions relative path, but only an absolute is stored here. How does adding a SourceCache component make this better for Windows?
There was a problem hiding this comment.
The path to Package.swift is now no longer \Package.swift, as the \SourceCache adds an arc between the root of the package and the drive root. Its a horrible workaround, but I don't have a good solution here.
There was a problem hiding this comment.
Please add this as a comment in code so that it's visible when reviewed in the future by someone else and not "cleaned up" by accident.
|
@swift-ci please smoke test |
1 similar comment
|
@swift-ci please smoke test |
|
@compnerd is this still relevant? |
|
Yes, this is needed to repair the test suite on Windows. |
|
Thanks, no rush, we were just looking at PRs that could potentially be stale this morning and weren't sure about this one. |
The path representation in AbsolutePath is incorrect and stores a relative path as an absolute path (paths with leading `/` or `\` are drive relative, not absolute, absolute paths must start with `[A-Z]:` or `\\[^\]+\[^\]+\`). This adds a root directory so that the path to the manifest is not drive relative.
compnerd
force-pushed
the
relative-is-absolute
branch
from
d19aa3a to
9a2f774
Compare
|
@swift-ci please test |
|
This is really needed for Windows - we cannot run the test suite without this. |
|
I don't see an easy way to do that, as really what it amounts to is effectively saying all absolute paths are relative (as SPM only works with relative paths - |
Co-authored-by: Saleem Abdulrasool <[email protected]>
|
@swift-ci test |
|
@swift-ci please test |
| @@ -234,7 +234,7 @@ class SourceControlPackageContainerTests: XCTestCase { | |||
| func testVersions() throws { | |||
| let fs = InMemoryFileSystem() | |||
|
|
|||
| let repoPath = AbsolutePath.root | |||
| let repoPath = AbsolutePath.root.appending("SourceCache") | |||
There was a problem hiding this comment.
question: Should we use withTemporaryDirectory here instead? Untested e.g
func testVersions() throws {
try withTemporaryDirectory(removeTreeOnDeinit: true) { repoPath in
let filePath = repoPath.appending("Package.swift")
//....
}
}
This way, we won't risk corrupting the host drive folder.
There was a problem hiding this comment.
We can do that in place of the InMemoryFileSystem. I think that the test issues do often arise from the use of InMemoryFileSystem. Note that I care little what solution we go with as long as we can get SPM tests correctly testing SPM on Windows as well :)
The path representation in
AbsolutePathis incorrect and stores a relative path as an absolute path (paths with leading/or\are drive relative, not absolute, absolute paths must start with[A-Z]:or\\[^\]+\[^\]+\). This adds a root directory so that the path to the manifest is not drive relative.