Running test fails

[StefMa]

Hey guys,
greetings from Germany. We typically write numbers with , instead of . 🤓
✅ 1,20 Euro
❌ 1.20 Euro.

Why is this interesting? Cause right now I get errors when running

./gradlew test

expected: 
  "module test
    facts
      ✘ fail
         4 == 9 (/tempDir/test.pkl, line xx)
  
  0.0% tests pass [1/1 failed], 50.0% asserts pass [1/2 failed]
  "
 but was: 
  "module test
    facts
      ✘ fail
         4 == 9 (/tempDir/test.pkl, line xx)
  
  0,0% tests pass [1/1 failed], 50,0% asserts pass [1/2 failed]
  "

Right now these tests are failing:

CliTestRunner fail test(Path)
CliTestRunner succeed test(Path)
CliTestRunner with thrown error in examples -- existing expected output(Path)
CliTestRunner with thrown error in examples -- no expected output(Path)
CliTestRunner with thrown error in facts(Path)
example length mismatch(Path)

[odenix]

Other things that could be improved about this message: don’t mix present and past tense, don’t use percentage for passed tests and fraction for failed tests. Not sure the assert stats are useful.

[bioball]

Hm, that's interesting.

It's probably a good thing that our test report formats numbers that are locale-specific, but our Grade build should nevertheless be consistent.

For now, you should be able to get the tests passing with LC_ALL=en_US.UTF-8 ./gradlew test.

Other things that could be improved about this message: don’t mix present and past tense, don’t use percentage for passed tests and fraction for failed tests. Not sure the assert stats are useful.

Yeah, I agree, the summary can be polished up a bit. But, the number of assertions is meaningful for some users (CC @jjmaestro).

[jjmaestro]

@odenix my rationale when adding this was that the % was a nicer metric for pass because you always want that to be 100% and knowing the exact number of tests that fail was a good metric for fail since that's the stuff you need to fix. Likewise for asserts since that also tells you the work that needs doing (fail) and the amount of testing you actually have (e.g. it's not the same having 1 test with 1 assert VS 1 test with a lot of asserts, the second case usually covers more corners so it's nice to know as a metric).

Plus, it doesn't hurt just having some summary at the end of the test run, right? :) Most test frameworks report these types of metric!

[odenix]

It's probably a good thing that our test report formats numbers that are locale-specific

I don't think that English messages should use German number formatting. It would, of course, make sense in German messages.

[StefMa]

Honestly I don't know why this happen. My MacBook is set up in English/US, Keyboard is set up to English/US. My IDE is in English/US. I am "only" located in Germany. So I'm also super curious why this even happened.

I am with @odenix here. This is a developer tool, I guess it should stick to english defaults and shouldn't introduce local-specifics.

[bioball]

Is the decimal different based on the language being used (as opposed to locale)? I don't have strong opinions here; we can use period decimals consistently if that's the norm.

[odenix]

A locale is a combination of language and country/region. By honoring the user's locale but displaying messages in English, we effectively change their locale to an unusual combination of language and country/region, which can be confusing.

It sounds like @StefMa has his locale set to en-US, but apparently his Pkl executable thinks it's en-DE or de-DE.

[bioball]

Ah! I didn't realize "locale" was a technical term. My question was, really, does the decimal change based on language and not region?

To answer that: doesn't look like it does. According to LocalePlanet, the decimal for en-DE is ,.

But, yeah, I agree that we're mis-formatting numbers here. All of our messages are written in en-US (Pkl currently does not change its messages based on locale), so changing the decimal here is a mistake.

[StefMa]

I opened a PR to fix that issue.

The discussion what do display here and in which form can be discussed somewhere else.
I guess it is important to make this stable first.