-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Make StartInfo_TextFile_ShellExecute test more reliable #46133
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
Make StartInfo_TextFile_ShellExecute test more reliable #46133
Conversation
…th .txt files otherwise things like notepad++ gets opened (common on dev machines) and the test fails
|
Tagging subscribers to this area: @eiriktsarpalis Issue Detailsuse notepad.exe in an explicit way (like the two other notepad tests do), don't rely on it being associated with .txt files otherwise, things like notepad++ get opened (common on dev machines) and the test fails Fixes #25823
|
|
/azp run runtime-libraries-coreclr outerloop-windows |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@Anipik @ViktorHofer how can I trigger the "Spanish" CI? |
src/libraries/System.Diagnostics.Process/tests/ProcessStartInfoTests.cs
Outdated
Show resolved
Hide resolved
We should move the test to outer loop. It is ok for outer loop tests to have environment dependencies like having notepad.exe registered with .txt files. |
…iated with .txt files" This reverts commit e7ec66d.
…lways sure that Notepad will be used (not other text editors)
It's already an outer loop test:
I think that we should introduce a new, unique extension and use the registry to associate it with Notepad to be always sure that it's going to be used. PTAL at my most recent commit |
|
/azp run runtime-libraries-coreclr outerloop-windows |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Replied offline but to summarize, the "Send to helix" log shows which queues work was sent to: In this case, |
|
From the CI logs it looks like only |
|
Thanks, this seems like a good experiment to try. Was there any evidence though that the txt mapping was missing/bad? IIRC we log the reg key and it always seemed correct, the launch still failed, which left me confused. I've tried to stabilize this poor test for a long time. |
No, but it was the only reason why it was not passing on my dev machine so I thought that it would be good to improve it. @danmosemsft is it OK if I re-enable now and when it fails get back to it with new logs? |
|
FWIW it's hard to act based on the old logs like #22007 (comment) because they contain source code line number, and the file has been modified many times and the line number does not tell me much |
|
Happy with whatever approach seems best to you. |
@danmosemsft Can I get approval from you so I can merge the PR? |
edit: introduce a new, unique extension and use the registry to associate it with Notepad to be always sure that it's going to be used
Fixes #25823