Skip to content

Tests: Enable more WorkspaceTests on Windows #8457

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

bkhouri
Copy link
Contributor

@bkhouri bkhouri commented Apr 8, 2025

Many WorkspaceTests instantiate a MockWorkspace, which takes a path as an input. The tests were defining a POSIX-like path as the sandbox directory, however, some assertions were incorrect as, on these *nix like filesystem, the canonical path name and the path were identical.

Update the WorkspaceTests expectation accordingly, and add some automated tests to cover the InMemoryFileSystem (which is commented out as a crash occurs in the Linux Platform CI build with Swift Testing tests).

Relates: #8433
rdar://148248105

@bkhouri bkhouri force-pushed the t/main/enable_workspace_tests_on_windows_gh8433_rdar148248105 branch from 625718d to 1cb7705 Compare April 8, 2025 12:34
@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 8, 2025

@swift-ci please test

@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 8, 2025

@swift-ci test self hosted windows

@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 8, 2025

@swift-ci test windows

@bkhouri bkhouri added windows test suite improvements to SwiftPM test suite and removed test suite improvements to SwiftPM test suite labels Apr 8, 2025
@bkhouri bkhouri marked this pull request as draft April 9, 2025 14:38
@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 9, 2025

After speaking with @MaxDesiatov offline, I will try to undo the changes to the sandbox root path change.

@MaxDesiatov
Copy link
Contributor

MaxDesiatov commented Apr 9, 2025

After speaking with @MaxDesiatov offline, I will try to undo the changes to the sandbox root path change.

To clarify, root path changes make sense to me, especially if they fix the tests. It's the separator changes that I'm not sure are required (or needed).

Also, if there's a way to remove the whitespace changes that would be great too.

@bkhouri bkhouri marked this pull request as ready for review April 10, 2025 16:06
@bkhouri bkhouri marked this pull request as draft April 10, 2025 17:25
@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 10, 2025

To clarify, root path changes make sense to me, especially if they fix the tests. It's the separator changes that I'm not sure are required (or needed).

@MaxDesiatov Are you referring to the separator changes as follows? If so, those are required. if not, can you please provide an example?
Screenshot 2025-04-09 at 10 02 01 AM

@bkhouri bkhouri force-pushed the t/main/enable_workspace_tests_on_windows_gh8433_rdar148248105 branch 2 times, most recently from 0869c3c to 167144e Compare April 14, 2025 17:41
@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 14, 2025

@swift-ci please test

@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 14, 2025

@swift-ci test self hosted windows

@bkhouri bkhouri marked this pull request as ready for review April 14, 2025 17:53
@bkhouri bkhouri force-pushed the t/main/enable_workspace_tests_on_windows_gh8433_rdar148248105 branch from 167144e to e9ea09a Compare April 14, 2025 17:59
@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 14, 2025

@swift-ci test

@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 14, 2025

@swift-ci test self hosted windows

@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 14, 2025

@swift-ci test windows

Copy link
Contributor

@MaxDesiatov MaxDesiatov left a comment

Choose a reason for hiding this comment

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

Formatting nits, otherwise LGTM. Thanks!

@bkhouri bkhouri force-pushed the t/main/enable_workspace_tests_on_windows_gh8433_rdar148248105 branch 2 times, most recently from fbfaf2d to 7a5e62e Compare April 16, 2025 16:45
@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 16, 2025

@swift-ci test

@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 16, 2025

@swift-ci test self hosted windows

@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 16, 2025

@swift-ci test windows

bkhouri added 2 commits April 22, 2025 11:35
Many WorkspaceTests instantiate a MockWorkspace, which takes a path as
an input. The tests were defining a POSIX-like path as the sandbox
directory, however, some assertions were incorrect as, on these *nix like
filesystem, the canonical path name and the path were identical.

Update the WorkspaceTests expectation accordingly, and add some
automated tests to cover the InMemoryFileSystem.
@bkhouri bkhouri force-pushed the t/main/enable_workspace_tests_on_windows_gh8433_rdar148248105 branch from 7a5e62e to a5ed5f5 Compare April 22, 2025 15:37
@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 22, 2025

@swift-ci test

@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 22, 2025

@swift-ci test windows

@bkhouri bkhouri enabled auto-merge (squash) April 22, 2025 15:50
@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 22, 2025

@swift-ci test linux

@bkhouri
Copy link
Contributor Author

bkhouri commented Apr 22, 2025

@swift-ci test windows

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test suite improvements to SwiftPM test suite windows
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants