-
Notifications
You must be signed in to change notification settings - Fork 564
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
Set BridgeResourceFolder at Bridge startup #337
Conversation
Prior to this change, the BridgeResourceFolder was specified by BridgeClient when tests ran. However, this was not a valid strategy for x-platform use, otherwise both client and Bridge would need to share a x-OS folder. With this change, Bridge.exe is required to have a BridgeResourceFolder when it is asked to start, and the BridgeClient no longer alters that. Also adds a 'reset' option to Bridge.exe so that when tests complete the Bridge can be asked to release all remote resources. They will be reacquired the next time Bridge requests are made. Also move the Http DELETE handling to the ResourceController where it really belonged. An Http DELETE to the /resource endpoint releases all resources. Fixes dotnet#334
pushd %~dp0..\..\..\..\bin\wcf\tools\Bridge | ||
echo Invoking Bridge.exe -reset %* ... | ||
call Bridge.exe -reset %* | ||
popd | ||
echo The Bridge was left running because BridgeKeepRunning is true |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIRC, CleanupWcfTestService.cmd is called immediately after a run happens. Do we necessarily want to get rid of all resources that the bridge is holding?
My flow of work is typically the following:
- run startBridge.cmd
- run build.cmd to look at what's wrong
- make changes
- rebuild.
Sometimes, in the "rebuild" step I run build.cmd, other times I build the project and test directly.
My suspicion is that if someone has left the bridge running, we should probably leave any resources opened alone, and rely on the developer to either exit
the Bridge or call reset
manually
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CleanupWcfTestService.cmd is the script called to clean up all resources -- that's its purpose. In this case, cleanup means remove all certs and firewall rules created, shutdown the AppDomains, and continue running. The Bridge will reacquire resources the next time it receives requests.
I think it is fairly critical that after a run all DLL's loaded from the BridgeResourceFolder be released, otherwise the build will fail because it cannot write the new DLL's into the BridgeResourceFolder. I think asking the developer to do an explicit 'reset' is just going to lead to confusing build failures.
I don't see any issues with this at a glance, so LGTM. |
Set BridgeResourceFolder at Bridge startup
Prior to this change, the BridgeResourceFolder was specified by
BridgeClient when tests ran. However, this was not a valid
strategy for x-platform use, otherwise both client and Bridge
would need to share a x-OS folder.
With this change, Bridge.exe is required to have a BridgeResourceFolder
when it is asked to start, and the BridgeClient no longer alters that.
Also adds a 'reset' option to Bridge.exe so that when tests complete
the Bridge can be asked to release all remote resources. They will
be reacquired the next time Bridge requests are made.
Also move the Http DELETE handling to the ResourceController where it
really belonged. An Http DELETE to the /resource endpoint releases all
resources.
Fixes #334