Skip to content
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

fix: Application.Restart throws InvalidOperationException #2845

Merged
merged 2 commits into from
May 21, 2020
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1167,7 +1167,7 @@ public static void Restart()
sb.Append(arguments[arguments.Length - 1]);
sb.Append('"');
}
ProcessStartInfo currentStartInfo = Process.GetCurrentProcess().StartInfo;
ProcessStartInfo currentStartInfo = new ProcessStartInfo();
Copy link
Member

Choose a reason for hiding this comment

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

Is there a test we can add?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this method is unit-testable, there are too many moving parts here. Can't think of a relatively low-effort automation either, we haven't done any automated integration tests yet...
However I do agree we some form a verification that it is working as intended.

@EatonZ please have a look at #2801. I have created few PS scripts that generate net472 and netcoreapp5.0 app, and then ran manual tests against them to confirm the fix.
You could tweak the "single app" script and then run a series of tests with and without input parameters.

A verification would be something along the lines of:

# build and run the app, passing parameters to it
dotnet run -- a b c
# app restarted, confirm parameters
Get-WmiObject Win32_Process | where name -eq your_app_name.exe | select commandline

@hughbe any thoughts on how else we can test this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@RussKie It seems like we could use the scripts you made there, with 1 tweak: replace => System.Windows.Forms.Application.ExecutablePath; with => System.Windows.Forms.Application.Restart();, wrap that in a try/catch and report a failed test if it throws an exception.

Copy link
Member

Choose a reason for hiding this comment

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

Yes. We still need to confirm all input arguments are being passed to the new instance.

currentStartInfo.FileName = ExecutablePath;
if (sb.Length > 0)
{