Skip to content

Fix issue with running test on windows w/no-parallel #8676

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
May 24, 2025
Merged

Fix issue with running test on windows w/no-parallel #8676

merged 1 commit into from
May 24, 2025

Conversation

daveinglis
Copy link
Contributor

The AsyncProcess Tests were using a TSCTestSupport method to temporarily change the PATH env but seems to have a bug where it removes the variable instead of restoring resulting in any future test needing PATH to fail. This switches it to using the similar method that is in SwiftPM Environment.swift

@daveinglis
Copy link
Contributor Author

@swift-ci test

@daveinglis
Copy link
Contributor Author

@swift-ci test windows

@daveinglis daveinglis force-pushed the pr/windows_env_fix branch from e10bdbc to eac75b3 Compare May 16, 2025 14:59
@daveinglis
Copy link
Contributor Author

@swift-ci test

@daveinglis daveinglis force-pushed the pr/windows_env_fix branch 2 times, most recently from 2652bd7 to 54a7a40 Compare May 20, 2025 14:26
@daveinglis
Copy link
Contributor Author

@swift-ci test

@daveinglis
Copy link
Contributor Author

@swift-ci test windows

@daveinglis
Copy link
Contributor Author

@swift-ci test windows self hosted

@daveinglis
Copy link
Contributor Author

@swift-ci test windows

@daveinglis daveinglis force-pushed the pr/windows_env_fix branch from 54a7a40 to 808eaae Compare May 21, 2025 21:17
@daveinglis
Copy link
Contributor Author

@swift-ci test windows

@daveinglis daveinglis force-pushed the pr/windows_env_fix branch from 808eaae to 5ccaeb6 Compare May 21, 2025 21:23
@daveinglis
Copy link
Contributor Author

@swift-ci test windows

@daveinglis
Copy link
Contributor Author

@swift-ci test

1 similar comment
@daveinglis
Copy link
Contributor Author

@swift-ci test

@daveinglis daveinglis force-pushed the pr/windows_env_fix branch from 5ccaeb6 to d999e35 Compare May 22, 2025 12:44
@daveinglis
Copy link
Contributor Author

@swift-ci test

@daveinglis
Copy link
Contributor Author

@swift-ci test windows

1 similar comment
@daveinglis
Copy link
Contributor Author

@swift-ci test windows

@daveinglis daveinglis force-pushed the pr/windows_env_fix branch from d999e35 to d087c3f Compare May 23, 2025 12:38
@daveinglis
Copy link
Contributor Author

@swift-ci test

@daveinglis daveinglis force-pushed the pr/windows_env_fix branch 2 times, most recently from 261a9fd to 039e842 Compare May 23, 2025 13:42
@daveinglis
Copy link
Contributor Author

@swift-ci test

@daveinglis
Fix issue with running test on windows w/no-parallel
4979efe 
The AsyncProcess Tests were using a TSCTestSupport method to temporarly change the PATH env but seems to have a bug where it removes the variable instead of restoring resulting in any future test needing PATH to fail.  This switch it to using the same method that is in SwiftPM Environment.swft
@daveinglis daveinglis force-pushed the pr/windows_env_fix branch from 039e842 to 4979efe Compare May 23, 2025 15:20
@daveinglis
Copy link
Contributor Author

@swift-ci test

@daveinglis
Copy link
Contributor Author

@swift-ci test windows

@dschaefer2
Copy link
Member

Awesome. Thanks @daveinglis!

@dschaefer2 dschaefer2 enabled auto-merge (squash) May 23, 2025 20:20
jakepetroules
jakepetroules approved these changes May 23, 2025
View reviewed changes
Copy link
Contributor

@jakepetroules jakepetroules left a comment

Choose a reason for hiding this comment

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

Thanks!

Also, the way I solved similar cases for the tests in SwiftBuild was to use @TaskLocal (see SWBUtil/UserDefault.swift), which is like a Swift Concurrency version of thread local storage.

I suggest taking a look at adopting that in Environment next, for a more permanent fix. That'll allow us to just make withCustomEnv / Environment.makeCustom completely thread safe.

@dschaefer2
Copy link
Member

@swift-ci please test windows

@dschaefer2 dschaefer2 merged commit ccbf01f into swiftlang:main May 24, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jakepetroules jakepetroules jakepetroules approved these changes

@bkhouri bkhouri bkhouri approved these changes

@dschaefer2 dschaefer2 dschaefer2 approved these changes

@shawnhyam shawnhyam Awaiting requested review from shawnhyam shawnhyam is a code owner

@bripeticca bripeticca Awaiting requested review from bripeticca bripeticca is a code owner

@plemarquand plemarquand Awaiting requested review from plemarquand plemarquand is a code owner

@owenv owenv Awaiting requested review from owenv owenv is a code owner

@cmcgee1024 cmcgee1024 Awaiting requested review from cmcgee1024 cmcgee1024 is a code owner

@daveyc123 daveyc123 Awaiting requested review from daveyc123 daveyc123 is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@daveinglis @dschaefer2 @jakepetroules @bkhouri