Allow source and transform files to be read-only #17
Conversation
| JsonTransformation transformation = new JsonTransformation(tempSourceFilePath.FilePath, this.logger); | ||
| using (Stream result = transformation.Apply(tempTransformFilePath.FilePath)) | ||
| { | ||
| // succeess |
| JsonTransformation transformation = new JsonTransformation(tempSourceFilePath.FilePath, this.logger); | ||
| using (Stream result = transformation.Apply(tempTransformFilePath.FilePath)) | ||
| { | ||
| // succeess |
There was a problem hiding this comment.
We need more verification for success here, ie result not null, maybe compare result to the expected result.
| return; | ||
| } | ||
|
|
||
| if (!File.Exists(this.FilePath)) |
There was a problem hiding this comment.
Minor, but these two checks can be shortened to:
if (File.Exists(this.FilePath)) { // Do stuff }
|
Thank you for the review, @davilimap. I've addressed the changes you requested and also fixed the appveyor build. Thank you. |
| using (JsonTextReader jsonReader = new JsonTextReader(streamReader)) | ||
| { | ||
| var transformed = JObject.Load(jsonReader); | ||
| var expected = JObject.Parse(@"{ |
There was a problem hiding this comment.
Since you renamed A, the expected result should be:
{ 'Astar': 1 }
In any case, this test doesn't need to (and probably shouldn't) verify the correctness of the transformation. You can just verify that the result stream is not null and no exceptions were recorded, like in TryTransformTest.
|
Thanks for the fix on the appveyor build. I made another comment on the test. Since its purpose is just to verify that we can perform transforms on read-only files, I think it should be isolated from the actual transform logic, so we shouldn't be verifying the actual content of the final transform. Like in |
…f the transformation in the read-only files test
|
I've updated the code to call existing assertion routine instead of checking the results of the transformation as suggested. Would you like me to rebase and squash the multiple commits? Thank you. |
|
Looks good, thanks for the PR! No need no squash, I'll just merge it in. |
|
Hi, When will this be available to download as the part of Slow-Cheetah? Best Regards, |
Fixed #16