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

Update DEVGUIDE to mention net472 #6242

Merged
merged 1 commit into from
Feb 20, 2019
Merged

Update DEVGUIDE to mention net472 #6242

merged 1 commit into from
Feb 20, 2019

Conversation

cartermp
Copy link
Contributor

As per #6226 this is required

As per #6226 this is required
@cartermp
Copy link
Contributor Author

This flaky test again:

Failed   Test request for parse and check doesn't check whole project
Error Message:
   Expected: true
Actual: false
  Expected: True
  But was:  False

Stack Trace:
   at FsUnit.shouldEqual[a](a x, a y) in D:\a\1\s\tests\service\FsUnit.fs:line 19
   at FSharp.Compiler.Service.Tests.PerfTests.Test request for parse and check doesn't check whole project() in D:\a\1\s\tests\service\PerfTests.fs:line 75

@dsyme can we consider disabling or removing this test? It's horribly unreliable and I don't know if it's worth having in the first place

@cartermp cartermp closed this Feb 18, 2019
@cartermp cartermp reopened this Feb 18, 2019
@dsyme
Copy link
Contributor

dsyme commented Feb 19, 2019

@dsyme can we consider disabling or removing this test? It's horribly unreliable and I don't know if it's worth having in the first place

Hmmmm I've not seen this test fail. But it's testing something pretty darn important (that we are not triggering entire rechecks of projects). We should at least add better diagnostics to work out what's failing and then dig into why

[<Test>]
let ``Test request for parse and check doesn't check whole project`` () = 

    let backgroundParseCount = ref 0 
    let backgroundCheckCount = ref 0 
    checker.FileChecked.Add (fun x -> incr backgroundCheckCount)
    checker.FileParsed.Add (fun x -> incr backgroundParseCount)

    let pB, tB = FSharpChecker.GlobalForegroundParseCountStatistic, FSharpChecker.GlobalForegroundTypeCheckCountStatistic
    let parseResults1 = checker.ParseFile(Project1.fileNames.[5], Project1.fileSources2.[5], Project1.parsingOptions)  |> Async.RunSynchronously
    let pC, tC = FSharpChecker.GlobalForegroundParseCountStatistic, FSharpChecker.GlobalForegroundTypeCheckCountStatistic
    (pC - pB) |> shouldEqual 1
    (tC - tB) |> shouldEqual 0
    backgroundParseCount.Value |> shouldEqual 0
    backgroundCheckCount.Value |> shouldEqual 0
    let checkResults1 = checker.CheckFileInProject(parseResults1, Project1.fileNames.[5], 0, Project1.fileSources2.[5], Project1.options)  |> Async.RunSynchronously
    let pD, tD = FSharpChecker.GlobalForegroundParseCountStatistic, FSharpChecker.GlobalForegroundTypeCheckCountStatistic
    backgroundParseCount.Value |> shouldEqual 5
    backgroundCheckCount.Value |> shouldEqual 5
    (pD - pC) |> shouldEqual 0
    (tD - tC) |> shouldEqual 1

    let checkResults2 = checker.CheckFileInProject(parseResults1, Project1.fileNames.[7], 0, Project1.fileSources2.[7], Project1.options)  |> Async.RunSynchronously
    let pE, tE = FSharpChecker.GlobalForegroundParseCountStatistic, FSharpChecker.GlobalForegroundTypeCheckCountStatistic
    (pE - pD) |> shouldEqual 0
    (tE - tD) |> shouldEqual 1
    (backgroundParseCount.Value  <= 9) |> shouldEqual true // but note, the project does not get reparsed
    (backgroundCheckCount.Value  <= 9) |> shouldEqual true // only two extra typechecks of files

    // A subsequent ParseAndCheck of identical source code doesn't do any more anything
    let checkResults2 = checker.ParseAndCheckFileInProject(Project1.fileNames.[7], 0, Project1.fileSources2.[7], Project1.options)  |> Async.RunSynchronously
    let pF, tF = FSharpChecker.GlobalForegroundParseCountStatistic, FSharpChecker.GlobalForegroundTypeCheckCountStatistic
    (pF - pE) |> shouldEqual 0  // note, no new parse of the file
    (tF - tE) |> shouldEqual 0  // note, no new typecheck of the file
    (backgroundParseCount.Value <= 9) |> shouldEqual true // but note, the project does not get reparsed
    (backgroundCheckCount.Value <= 9) |> shouldEqual true // only two extra typechecks of files
    ()

@cartermp
Copy link
Contributor Author

I've seen it fail probably 10-20% of the time (see #4457 as well), but nothing in what it's testing gets changed in those PRs. So there's either a bug in FCS or it's a flaky test, but given recent history my first thought is it's the test.

@dsyme
Copy link
Contributor

dsyme commented Feb 19, 2019

I've seen it fail probably 10-20% of the time (see #4457 as well), but nothing in what it's testing gets changed in those PRs. So there's either a bug in FCS or it's a flaky test, but given recent history my first thought is it's the test.

OK thanks. We should dig into what's failing by adding better diagnostics

@cartermp cartermp closed this Feb 19, 2019
@cartermp cartermp reopened this Feb 19, 2019
@KevinRansom KevinRansom merged commit 138f2b0 into master Feb 20, 2019
@brettfo brettfo deleted the cartermp-patch-1 branch April 7, 2019 15:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants