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

FakeVar enhancements #1978

Merged
merged 26 commits into from
Jun 5, 2018
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
b6f0a86
Add ability to get fake var with a type
BlythMeister Jun 4, 2018
5fcfbac
Add function to get fakevar or fail if not present
BlythMeister Jun 4, 2018
a6467ab
Add get fakevar or default function
BlythMeister Jun 4, 2018
0e0453e
Added tests on FakeVar logic
BlythMeister Jun 4, 2018
d44b388
Update to use the type passing on fakevar
BlythMeister Jun 4, 2018
1298eb0
Build and test fix
BlythMeister Jun 4, 2018
cac5c48
Remove Expecto flip opens
BlythMeister Jun 4, 2018
7f33f85
Migrate FakeVar usage into separate module
BlythMeister Jun 4, 2018
73da350
Fix AssemblyInfo warnings
BlythMeister Jun 4, 2018
d5e51e8
Fix more warnings
BlythMeister Jun 4, 2018
3c44abb
Fix some more warnings
BlythMeister Jun 4, 2018
cdd194c
Add the new file into FakeLib.fsproj
BlythMeister Jun 4, 2018
b71163e
Added tests for type mismatch
BlythMeister Jun 4, 2018
e1f8edd
Add [<RequireQualifiedAccess>] to variables.fs
BlythMeister Jun 4, 2018
6dba481
Update error message on cast error
BlythMeister Jun 4, 2018
5882326
Fix tests now error has changed
BlythMeister Jun 4, 2018
4a9867b
Improve cast error message
BlythMeister Jun 4, 2018
d662d63
Fix compilation
matthid Jun 4, 2018
baca2d8
Rename Fake.Core.Variables to Fake.Core.FakeVar
BlythMeister Jun 4, 2018
2b66f5a
add vscode restore task & nest FakeVar project in Fake.sln
BlythMeister Jun 5, 2018
1671636
Test fixes
BlythMeister Jun 5, 2018
9448868
Remove redundant statements
BlythMeister Jun 5, 2018
7faa14c
Correct FakeVar error message
BlythMeister Jun 5, 2018
e649774
Test fixes
BlythMeister Jun 5, 2018
b9fb1c2
Simplify and add P2P="true"
BlythMeister Jun 5, 2018
99e7402
Fixing INT tests
BlythMeister Jun 5, 2018
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
14 changes: 12 additions & 2 deletions src/app/Fake.Core.Context/Context.fs
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,20 @@ let forceFakeContext () =
invalidOp "no Fake Execution context was found. You can initialize one via Fake.Core.Context.setExecutionContext"
| RuntimeContext.Fake e -> e

let getFakeVar name =
let getFakeVar<'a> name =
forceFakeContext()
|> getFakeContext name
|> Option.map (fun o -> o :?> 'a)

let getFakeVarOrFail<'a> name =
match getFakeVar<'a> name with
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer this to be in a new module and try to keep the api surface (and changes) of the context module as minimal as possible. This is the only module where you have to update the runner in order to get the changes. Therefore your suggestion will break on old runners

Adding the generic parameter is (I believe) harmless.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ohhh really :s

i wonder if it's possible to remove all the variable stuff?

Copy link
Member

Choose a reason for hiding this comment

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

i wonder if it's possible to remove all the variable stuff?

We maybe could move it somewhere else but have to leave an obsolete attribute for now (we can do the breaking change sometime after the release)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Exactly what I've started working on 😂
All internal usage I'll migrate away from obsolete.

| Some v -> v
| _ -> failwithf "Unable to find FakeVar '%s'" name

let getFakeVarOrDefault<'a> name defaultValue =
match getFakeVar<'a> name with
| Some v -> v
| _ -> defaultValue

let removeFakeVar name =
forceFakeContext()
Expand Down Expand Up @@ -145,4 +155,4 @@ let fakeVarAllowNoContext name =
(fun (v : 'a) ->
if isFakeContext() then
setFakeVar name v |> ignore
else varWithoutContext <- Some v)
else varWithoutContext <- Some v)
4 changes: 2 additions & 2 deletions src/app/Fake.Core.Process/Process.fs
Original file line number Diff line number Diff line change
Expand Up @@ -263,12 +263,12 @@ module Process =
//let startedProcesses = HashSet()
let private startedProcessesVar = "Fake.Core.Process.startedProcesses"
let private getStartedProcesses, _, private setStartedProcesses =
Fake.Core.Context.fakeVar startedProcessesVar
Fake.Core.Context.fakeVar<ProcessList> startedProcessesVar

let private doWithProcessList f =
if Fake.Core.Context.isFakeContext () then
match getStartedProcesses () with
| Some (h:ProcessList) -> Some(f h)
| Some h -> Some(f h)
| None ->
let h = new ProcessList()
setStartedProcesses (h)
Expand Down
27 changes: 27 additions & 0 deletions src/test/Fake.Core.UnitTests/Fake.Core.Context.fs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ open System
open Fake.Core
open Expecto
open Expecto.Flip
open FParsec.ErrorMessage

[<Tests>]
let tests =
Expand All @@ -19,5 +20,31 @@ let tests =
Fake.Core.Context.forceFakeContext() |> ignore
Tests.failtest "Expected exception"
with :? System.InvalidOperationException as e -> ()

testCase "Ability to set and get fake variables" <| fun _ ->
Fake.Core.Context.setFakeVar "Test" "TestValue" |> ignore
let value = Fake.Core.Context.getFakeVar<string> "Test"
Expect.isSome "FakeVar 'Test' is none" value
Expect.equal "TestValue" value.Value "FakeVar 'Test' is incorrect"

testCase "Ability to set and get fake variables with default - when found" <| fun _ ->
Fake.Core.Context.setFakeVar "Test" "TestValue" |> ignore
let value = Fake.Core.Context.getFakeVarOrDefault<string> "Test" "DefaultValue"
Expect.equal "TestValue" value "FakeVar 'Test' is incorrect"

testCase "Ability to set and get fake variables with default - when not found" <| fun _ ->
let value = Fake.Core.Context.getFakeVarOrDefault<string> "Test" "DefaultValue"
Expect.equal "DefaultValue" value "FakeVar 'Test' is not the default"

testCase "Ability to set and get fake variables with failure - when found" <| fun _ ->
Fake.Core.Context.setFakeVar "Test" "TestValue" |> ignore
let value = Fake.Core.Context.getFakeVarOrFail<string> "Test"
Expect.equal "TestValue" value "FakeVar 'Test' is incorrect"

testCase "Ability to set and get fake variables with default - when not found" <| fun _ ->
try
Fake.Core.Context.getFakeVarOrFail<string> "Test" |> ignore
Tests.failtest "Expected exception"
with e ->
Expect.equal "Unable to find FakeVar 'Test'" e.Message "Incorrect failure message for FakeVar failure case"
]