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

Some FsMessageTemplate tests failing #294

Closed
haf opened this issue Apr 8, 2018 · 4 comments · Fixed by #378
Closed

Some FsMessageTemplate tests failing #294

haf opened this issue Apr 8, 2018 · 4 comments · Fixed by #378

Comments

@haf
Copy link
Member

haf commented Apr 8, 2018

I rewrapped the tests properly in as Expecto tests:

screen shot 2018-04-08 at 16 04 52

And these are failing:

(examples)

[16:03:51 ERR] logary/message templates/alignment/10: cus #{CustomerId,10:000000}, pleasure to see you failed in 00:00:00.
format should work.
          Expected string to equal:
          "cus #    001234, pleasure to see you"
                ↑
          The string differs at index 5.
          "cus #001234    , pleasure to see you"
                ↑
          String does not match at position 5. Expected char: ' ', but got '0'.
  /Users/h/dev/logibit/logary/src/Logary.Tests/FsMessageTemplates.fs(67,1): Logary.Tests.FsMessageTemplates.RenderedAs@67-8.Invoke(AssertException exn)
 <Expecto>
[16:03:51 ERR] logary/message templates/alignment/9: cus #{CustomerId,10}, pleasure to see you failed in 00:00:00.
format should work.
          Expected string to equal:
          "cus #      1234, pleasure to see you"
                ↑
          The string differs at index 5.
          "cus #1234      , pleasure to see you"
                ↑
          String does not match at position 5. Expected char: ' ', but got '1'.
  /Users/h/dev/logary/src/Logary.Tests/FsMessageTemplates.fs(67,1): Logary.Tests.FsMessageTemplates.RenderedAs@67-8.Invoke(AssertException exn)
 <Expecto>
[16:03:51 ERR] logary/message templates/alignment/8: cus #{CustomerId,-10:000000}, pleasure to see you failed in 00:00:00.
format should work.
          Expected string to equal:
          "cus #001234    , pleasure to see you"
                ↑
          The string differs at index 5.
          "cus #    001234, pleasure to see you"
                ↑
          String does not match at position 5. Expected char: '0', but got ' '.
  /Users/h/dev/logary/src/Logary.Tests/FsMessageTemplates.fs(67,1): Logary.Tests.FsMessageTemplates.RenderedAs@67-8.Invoke(AssertException exn)
 <Expecto>

Repro: cd src/Logary.Tests && dotnet run --framework netcoreapp2.0 => [16:03:52 INF] EXPECTO! 455 tests run in 00:00:02.6205660 for logary – 430 passed, 11 ignored, 14 failed, 0 errored. ( ರ Ĺ̯ ರೃ ) <Expecto>

Do you think you could have a look @adamchester ?

@haf haf added the help wanted label Apr 8, 2018
@adamchester adamchester self-assigned this Apr 8, 2018
@adamchester
Copy link
Contributor

Hey @haf, I couldn't repro this.

~logary\src\Logary.Tests>dotnet run --framework netcoreapp2.0 --sequenced
[15:02:38 INF] EXPECTO? Running tests... <Expecto>
...
[15:02:46 INF] EXPECTO! 460 tests run in 00:00:07.6868294 for logary - 433 passed, 27 ignored, 0 failed, 0 errored. Success! <Expecto>

@adamchester
Copy link
Contributor

I just noticed they're ignored... investigating further...

@adamchester
Copy link
Contributor

OK, I found some gaps in the tests for alignment over in F# MtParserFull which I'll have to address over there first. It will probably result in a new file we can paket-reference here.

However, the first issue here seems to be in Logary.MessageTemplates.Formatting.Literate.tokeniseProperty:

match pt.align.direction with
|  AlignDirection.Right -> formated.PadRight(pt.align.width, ' ')
|  AlignDirection.Left -> formated.PadLeft(pt.align.width, ' ')

When it's AlignDirection.Left then the extra spaces (padding) should be on the right, and vice-versa. So, it should be: Right -> PadLeft and Left -> PadRight

I've confirmed this interpretation with the following tests against Serilog and String.Format:

void Main()
{
    using (var logger = new LoggerConfiguration()
        .WriteTo.Console(outputTemplate: "{Source}: {Message}{NewLine}")
        .CreateLogger())
    {
        var log = logger.ForContext("Source", "Serilog");
        log.Information("{p1,-10} | {p2,10}", "a", "a");
        log.Information("{p1,-10} | {p2,10}", "aa", "aa");
        log.Information("{p1,-10} | {p2,10}", "aaaa", "aaaa");

        log = logger.ForContext("Source", "string.Format (C#)");
        log.Information($"\"{"a",-10}\" | \"{"a",10}\"");
        log.Information($"\"{"aa",-10}\" | \"{"aa",10}\"");
        log.Information($"\"{"aaaa",-10}\" | \"{"aaaa",10}\"");

        log = logger.ForContext("Source", "string.Format (BCL)");
        log.Information(string.Format("{0,-10} | {1,10}", "a", "a"));
        log.Information(string.Format("{0,-10} | {1,10}", "aa", "aa"));
        log.Information(string.Format("{0,-10} | {1,10}", "aaaa", "aaaa"));
    }
}

haf added a commit that referenced this issue Apr 25, 2018
@haf
Copy link
Member Author

haf commented Apr 25, 2018

I've fixed all but two of these tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants