fix mutable sync/async logging and support async tests

[AnthonyLloyd]

Hopefully the style is ok.

I think for LF vs CRLF you need a .gitattributes file.

I think ultimately there must be a better way to handle sync/async logging. Needs to be able to be switched from async flush to manual flush.

[AnthonyLloyd]

I think I know how this should work. I've just been trying to add support for async tests.

The executions of a test with logging should be made into an async<TestRunResult> which is easy enough.
The parallel ones then can be run together and the sequenced one by one completing their logging.
This should simplify the code and the TestPrinters are now correct with their Async<unit> return values.

[haf]

It's really just the join at the end of the run that should decide the method with which logs are awaited... Just because we sequence tests, we don't have to start their logs immediately. Also, logger.infoWithBP will only back-pressure the logger's buffer, not the printing. So the returned Async<unit> from such a function call will yield as soon as the buffer has accepted the message – a final flush of the logging infrastructure at the end should then be enough.

[haf]

I really like these changes. However, we may have to think about it a bit; see my in-code comment.

[AnthonyLloyd]

Sure. I've been working on the async tests. I'll add that to the PR when its ready and we can discuss.

Currently having some issues with the run taking 20s insead of 2s (logging ACK? not sure)

[haf]

The two are identical in the Facade: https://github.com/logary/logary/blob/master/src/Logary.Facade/Facade.fs#L703-L710

[AnthonyLloyd]

@haf
I've had to limit the number of threads to get this to not take 20s.
I'm not sure why this is necessary.

Could you take a look at some point and let me know if you think I'm on the right track.

[AnthonyLloyd]

@haf,

Just to update you on the status of this.

I've added number of parallel workers to command line and async test tests. Its basically done as far I can see apart from some docs.

There are a few issues outstanding:

• getMethodName - I can't write the code to locate the async tests. --summary-location just gives [:0] for me in Debug and Release for sync tests

• The BenchmarkDotNet tests are still not working. They pass but are excepting and nan in travis.

• There are some tests that are failing for me but not travis (even when I checkout haf/master):

Failed: 7
all/string comparison/different length, actual is shorter [:0]
all/string comparison/different length, actual is longer [:0]
all/string comparison/fail - different content [:0]
expecto/expectations/#containsAll/sequence contains everything expected [:0]
expecto/expectations/#distribution/sequence doesn't contain repeats in expected [:0]
expecto/expectations/#distribution/sequence does contain repeats in expected but should not [:0]
expecto/expectations/#distribution/sequence does not contains everything expected [:0]

[haf]

Your failures may be fixed w/ string changes in #64

[haf]

No 64 is merged now.

[AnthonyLloyd]

rebased but I still see the errors. May be down to LF vs CRLF:

[20:03:12 ERR] expecto/expectations/#distribution/sequence does contain repeats in expected but should not failed in 00:00:00.0855580.
Test failure strings should equal.
          Expected string to equal:
          "Sequence should contain two, two and four.
    Sequence `actual` does not contain every `expected` elements.
        All elements in `actual`:
        {2, 2}
        All elements in `expected` ['item', 'number of expected occurrences']:
        {2: 2, 4: 1}
        Missing elements from `actual`:
        '4' (0/1)"

                                                                                                          ↑
          The string differs at index 306.
          "Sequence should contain two, two and four.
    Sequence `actual` does not contain every `expected` elements.
        All elements in `actual`:
        {2, 2}
        All elements in `expected` ['item', 'number of expected occurrences']:
        {2: 2, 4: 1}
        Missing elements from `actual`:
        '4' (0/1)
" C:\Users\Ant\src\expecto\Expecto\Expect.fs(301,1): Expecto.Expect.distribution[a](IEnumerable`1 actual, FSharpMap`2 expected, String format)

                                                                                                          ↑
          String `actual` was longer than expected, at pos 306 found item '\010'.
  C:\Users\Ant\src\expecto\Expecto.Tests\Tests.fs(518,1): Expecto.Tests.expecto@518-173.Invoke(String msg)

[haf]

If we expect this to be sent all the way to the terminal before the process exists, logWithAck should be used.

[haf]

LogWithAck

[haf]

Do you still have issues with speed?

[AnthonyLloyd]

No its fine with the parallel-workers defaulting to logical processors.
I always thought you never needed to do this with Async because of the TPL only creating a fixed number of threads.
All other unit testing libraries have a config for workers and default to no logical processors.

I'll add docs for this command line later and test if a .gitattributes with git config core.autocrlf false fixes my local test failure.

Will close #2 when docs are done.

[haf]

This is a good PR, I've merged it and tested it to some extent and it's looking good. I've also added the testCaseAsync test function to the README.

[AnthonyLloyd]

Cool

[AnthonyLloyd]

Still have the 7 failing tests. Changing to LF doesn't seem to fix.

[haf]

Can you set up an appveyor.yml file to showcase the test errors?

[AnthonyLloyd]

@haf

Sure will look to.

I also did a contention profile and think that the problem with the slow down when using many workers is down to assertTestFails and multiple full evals running. Still experimenting but this probably could be transformed into a normal test without using the full runner inside the test.

I may also pick up #68 later in the week if no one else has done it. I have an interest in what could be done later with FsCheck (scaling for stress testing, easy stdgen etc)