haf / expecto

A smooth testing lib for F#. APIs made for humans! Strong testing methodologies for everyone!
Apache License 2.0
663 stars 96 forks source link

Cannot flush unless inited error (in master only) #297

Closed AnthonyLloyd closed 5 years ago

AnthonyLloyd commented 5 years ago
System.InvalidOperationException: Cannot flush unless inited
   at Expecto.Logging.ANSIOutputWriter.T.flushInner() in C:\Users\Anthony Lloyd\src\expecto\Expecto\Logging.fs:line 1001
   at Expecto.Logging.ANSIOutputWriter.T.flush() in C:\Users\Anthony Lloyd\src\expecto\Expecto\Logging.fs:line 1099
   at Expecto.Logging.ANSIOutputWriter.flush@1135.Invoke(T i) in C:\Users\Anthony Lloyd\src\expecto\Expecto\Logging.fs:line 1135
   at Microsoft.FSharp.Core.OptionModule.Iterate[T](FSharpFunc`2 action, FSharpOption`1 option)
   at Expecto.Tests.runTestsWithCancel(CancellationToken ct, ExpectoConfig config, Test tests) in C:\Users\Anthony Lloyd\src\expecto\Expecto\Expecto.fs:line 1818
   at Expecto.Focused.configTests@176-3.Invoke(Unit _arg4) in C:\Users\Anthony Lloyd\src\expecto\Expecto.Tests\FocusedTests.fs:line 176
   at Expecto.Impl.execTestAsync@924-1.Invoke(Unit unitVar) in C:\Users\Anthony Lloyd\src\expecto\Expecto\Expecto.fs:line 928
   at Microsoft.FSharp.Control.AsyncBuilderImpl.callA@522.Invoke(AsyncParams`1 args)

@haf I can look but do you have an idea. Seems some kind of race.

haf commented 5 years ago

@AnthonyLloyd It's definitely a race. The code as it was was simply not checking any invariants; hence the refactor. I didn't see this error after the refactor; but the other invariants I've been forced to lessen, because there's some serious interleaving of init, flush and dispose going on.

The 'as is' solution is to weaken the precondition check so you can release. I added my list of invariants inside the initial type declaration of T.

haf commented 5 years ago

Generally, I don't want the initing of this to be dependent on calling one of the 'main' methods in Expecto, but to be lazily self-initialising, once and only once.

AnthonyLloyd commented 5 years ago

I have taken out the flush in the duplicate names.

Can I just ask about the color change? It looks less bright now. If it's more correct then that's fine. Just a little worried about user feedback.

haf commented 5 years ago

Well, it is indeed less bright. The colours as they were, were the brightest possible (which is different from the default console white even).

I've also tested the inverted colours on white backgrounds, so they are visible; and I noticed that the Console.{Background,Foreground}Color return -1 on macOS/Core/zsh/iTerm2, so I handled that case as well.

I also translated it to 256 colours (it's 2018 after all), which should be automatically mapped back to 16 colours if the console in use doesn't handle 256.

Lastly, some colours were mapped to each other, i.e. it was a many-to-one mapping. I tried to avoid this, so that the different colourisation types are distinguishable.

AnthonyLloyd commented 5 years ago

Cool. Just setting up a release.