Updated Service response to EnablePrjFS request#1151
Conversation
|
Thanks! The change and explanation look straight forward enough. Is there any tests we could add to cover this behavior? |
kewillford
left a comment
kewillford
left a comment
There was a problem hiding this comment.
Are there any unit or functional tests that we can write for this change?
|
We have several service functional tests turned off, does it make sense to turn any on with this PR https://github.com/microsoft/VFSForGit/blob/master/GVFS/GVFS.FunctionalTests/Tests/MultiEnlistmentTests/ServiceVerbTests.cs |
| [TestCase] | ||
| public void ServiceHandlesEnablePrjfsRequest() | ||
| { | ||
| string expectedServiceResponse = "TRequestResponse|{\"State\":1,\"ErrorMessage\":null}"; |
There was a problem hiding this comment.
Let me know if this string looks too strict (making the test fragile.)
Ideally we should be having UT for all the Service Request Handler classes. Created a ticket for that. For this specific PR, I've added a new test in |
|
If we haven't created a cc: @jrbriggs |
Service (RequestHandler.cs) on the Mac, was sending responses to
EnableProjFS requests in an incorrect format. This was causing
`gvfs service --mount-all` to fail with "un-expected response
message" error. Updated RequestHandler to first create a Message
object from EnableProjFS Response and use it in its reply to
client. (It was sending string representation of the response
before.) This helps create a client parsable response with
expected header and body.
Also added new UT in MacServiceTests.
Service response before the change -
`GVFS.Common.NamedPipes.NamedPipeMessages+EnableAndAttachProjFSRequest+Response`
Service response after the change -
`TRequestResponse|{\"State\":1,\"ErrorMessage\":null}`
Fixes #1053
|
@alameenshah what was the verdict on this comment?
cc: @jeschu1 |
6 participants
Service (
RequestHandler.cs) on the Mac, was sending responses to EnableProjFS requests in an incorrect format. This was causinggvfs service --mount-allto fail with “un-expected response message” error. Updated RequestHandler to first create a Message object from EnableProjFS Response and use it in its reply to client. This helps create a client parsable response with expected header and body.Service response before the change -
GVFS.Common.NamedPipes.NamedPipeMessages+EnableAndAttachProjFSRequest+ResponseService response after the change -
TRequestResponse|{\"State\":1,\"ErrorMessage\":null}”}Fixes #1053