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

Ledger tests run differently locally and on the test runner #49

Open
olo-ntaylor opened this issue Nov 15, 2021 · 7 comments
Open

Ledger tests run differently locally and on the test runner #49

olo-ntaylor opened this issue Nov 15, 2021 · 7 comments

Comments

@olo-ntaylor
Copy link

My solution passes all tests (un-skipped, unmodified) locally. On the test runner, only 1 test passes. And on the test runner, I get different results (only 2 failures) if I .Trim() the header line of the ledger. But the remaining failures have weird whitespace differences still on various lines. I'm not sure what the differences are, but it seems like there are different whitespace expectations on the test runner than on the unit tests file.

@ErikSchierboom
Copy link
Member

ErikSchierboom commented Nov 16, 2021

Ah yes, I think you have found an issue in the tests, as they all use "\n": https://github.com/exercism/fsharp/blob/main/exercises/practice/ledger/LedgerTests.fs We have two options:

  • Normalize newlines in the tests
  • Add clarification to the instructions that we expect "\n" (or the platform's newline separator) as the separator.

I have a slightly preference for the latter, which we can implement via an append file: https://exercism.org/docs/building/tracks/practice-exercises#h-file-docs-instructions-append-md

Would you be interested in submitting a PR for that?

@olo-ntaylor
Copy link
Author

@ErikSchierboom Are you sure it is only a \n vs Environment.NewLine issue? When I copy/paste the test output from the test runner into Notepad++, e.g., there are differences in how many spaces are at the end of the line as well. Or, am I misunderstanding and the appearance of different amounts of spaces is a symptom of the newline issue. Fwiw, I am using \n in my solution, so I would assume that to match up with the test expectations if they are using /n as well.

Thank you so much for your help on this!

@ErikSchierboom
Copy link
Member

Hmmm, I'm not entirely sure. It is the most plausible explanation, but maybe there is different cause. Could you post your code?

@olo-ntaylor
Copy link
Author

Sure thing, here is the code:

module Ledger

open System
open System.Globalization
open System.Text.RegularExpressions

type Entry =
    { Date: DateTime
      Description: string
      Change: int }

type Alignment =
    | Left
    | Right

let mkEntry date description change =
    { Date = DateTime.Parse(date, CultureInfo.InvariantCulture)
      Description = description
      Change = change }

let formatLedger currency locale entries =
    let header =
        match locale with
        | "en-US" -> "Date       | Description               | Change       "
        | "nl-NL" -> "Datum      | Omschrijving              | Verandering  "
        | _ -> ""

    let currencySymbol =
        match currency with
        | "USD" -> "$"
        | "EUR" -> ""
        | _ -> ""

    let formatDate (date: DateTime) =
        match locale with
        | "nl-NL" -> date.ToString("dd-MM-yyyy")
        | "en-US" -> date.ToString("MM\/dd\/yyyy")
        | _ -> date.ToString()

    let enforceExactLength length alignment str =
        match String.length str, alignment with
        | l, Left when l <= length -> str.PadRight(length)
        | l, Right when l <= length -> str.PadLeft(length)
        | _, _ -> str.Substring(0, length - 3) + "..."

    let formatChange change =
        let formattedCurrencySymbol =
            match locale with
            | "nl-NL" -> currencySymbol + " "
            | _ -> currencySymbol


        let applyNegativePositiveFormat formattedChange =
            match locale, change with
            | "en-US", c when c < 0.0 ->
                sprintf "(%s)" <| Regex("-").Replace(formattedChange, "")
            | "en-US", c
            | "nl-NL", c when c >= 0.0 ->
                formattedChange + " "
            | _, _ ->
                formattedChange


        let formattedCents = (change / 100.0).ToString("#,#0.00", CultureInfo(locale))

        formattedCurrencySymbol + formattedCents
        |> applyNegativePositiveFormat
        |> enforceExactLength 13 Right

    let formatDescription = enforceExactLength 25 Left

    let formatEntry entry =
        sprintf "%s | %s | %s" (formatDate entry.Date) (formatDescription entry.Description) (formatChange (float entry.Change))

    let lineItems =
        entries
        |> List.sortBy (fun e -> e.Date, e.Description, e.Change)
        |> List.map formatEntry

    String.Join("\n", header :: lineItems)

Additionally, you can see that the issue in the test output:
image

Even though the test expectation contains whitespaces:
image

@ErikSchierboom
Copy link
Member

Should be fixed in exercism/fsharp#1045

@olo-ntaylor
Copy link
Author

@ErikSchierboom Now I'm seeing some very interesting behavior. In the last line of my solution I call String.Join("\n", header :: lineItems). In this case, I get 9 test failures, but 1 test passes.

If I instead call String.Join("\n", header**.Trim()** :: lineItems), then I get only 2 test failures.

I'm honestly drawing a blank at how 8 tests could suddenly pass when I trim the header line 🥴

@ErikSchierboom
Copy link
Member

That's very weird!

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

No branches or pull requests

2 participants