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.

Sign up for GitHub

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

Jump to bottom

Fix EvalScript by using verbatim string for #Load #184

Merged
merged 2 commits into from
Jul 29, 2014
Merged

Fix EvalScript by using verbatim string for #Load #184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
fe7e31e
Fix EvalScript by using verbatim string for #Load
Jul 23, 2014
5e78fb0
Add regression test for #184
Jul 28, 2014
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
2 changes: 1 addition & 1 deletion src/fsharp/fsi/fsi.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2075,7 +2075,7 @@ type internal FsiInteractionProcessor

member this.EvalScript(scriptPath) =
// Todo: this runs the script as expected but errors are displayed one line to far in debugger
let sourceText = sprintf "#load \"%s\" " scriptPath
let sourceText = sprintf "#load @\"%s\" " scriptPath
this.EvalInteraction sourceText

member __.EvalExpression(sourceText) =
Expand Down
20 changes: 20 additions & 0 deletions tests/service/FsiTests.fs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,25 @@ let ``Bad arguments to session creation 2``() =
Assert.False (String.IsNullOrEmpty (errStream.Read())) // error stream contains some output
Assert.True (String.IsNullOrEmpty (outStream.Read())) // output stream contains no output

[<Test>]
// Regression test for #184
let ``EvalScript accepts paths verbatim``() =
// Path contains escape sequences (\b and \n)
// Let's ensure the exception thrown (if any) is FileNameNotResolved
(try
let scriptPath = @"C:\bad\path\no\donut.fsx"
fsiSession.EvalScript scriptPath |> ignore
true
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be "false"? No success if no exception was thrown?

Copy link
Author

Choose a reason for hiding this comment

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

Honestly either would work as far as I'm concerned. I seriously doubt someone would create that file and run the regression test, so returning false is fine too. The nice thing is that regardless of the file's existence, the test would fail on regression.

with
| e ->
// Microsoft.FSharp.Compiler.Build is internal, so we can't access the exception class here
if String.Equals(e.InnerException.GetType().FullName,
"Microsoft.FSharp.Compiler.Build+FileNameNotResolved",
StringComparison.InvariantCultureIgnoreCase)
then true
else false)
|> shouldEqual true

let RunManually() =
``EvalExpression test 1``()
``EvalExpression fsi test``()
Expand All @@ -198,5 +217,6 @@ let RunManually() =
``ParseAndCheckInteraction test 1``()
``Bad arguments to session creation 1``()
``Bad arguments to session creation 2``()
``EvalScript accepts paths verbatim``()

#endif