cucumber / cucumber-expressions

Human friendly alternative to Regular Expressions
MIT License
143 stars 49 forks source link

Unable to parse negative number in Norwegian #287

Closed takle-sb1 closed 4 months ago

takle-sb1 commented 4 months ago

Attempting to use a negative number in a step that expects a{double}, in a test case written in Norwegian, doesn't work.

Minimal example:

# language: no

  Egenskap: Betalingseksempel

  Scenario: Negativ rest

    Gitt at du har 10,04 kr
    Når du betaler 12,15 kr
    Så skal du ha -2,11 kr igjen
package com.example.stepdefs;

import io.cucumber.java.no.Gitt;
import io.cucumber.java.no.Når;
import io.cucumber.java.no.Så;

import static org.junit.jupiter.api.Assertions.assertEquals;

public class PaymentStepDefs {

    private double balance;

    @Gitt("at du har {double} kr")
    public void initialBalance(double balance) {
        this.balance = balance;
    }

    @Når("du betaler {double} kr")
    public void makePayment(double payment) {
        this.balance = this.balance - payment;
    }

    @Så("skal du ha {double} kr igjen")
    public void assertRemainingBalance(double expected) {
        assertEquals(expected, this.balance);
    }
}

👓 What did you see?

Cucumber is unable to parse the number:

io.cucumber.core.exception.CucumberException: Could not convert arguments for step [skal du ha {double} kr igjen] defined at 'com.example.stepdefx.PaymentStepDefs.assertRemainingBalance(double)'.

    at io.cucumber.core.runner.PickleStepDefinitionMatch.couldNotConvertArguments(PickleStepDefinitionMatch.java:112)
    at io.cucumber.core.runner.PickleStepDefinitionMatch.runStep(PickleStepDefinitionMatch.java:56)
    at io.cucumber.core.runner.ExecutionMode$1.execute(ExecutionMode.java:10)
    at io.cucumber.core.runner.TestStep.executeStep(TestStep.java:84)
    at io.cucumber.core.runner.TestStep.run(TestStep.java:56)
    at io.cucumber.core.runner.PickleStepTestStep.run(PickleStepTestStep.java:51)
    at io.cucumber.core.runner.TestCase.run(TestCase.java:84)
    at io.cucumber.core.runner.Runner.runPickle(Runner.java:75)
    at io.cucumber.junit.platform.engine.CucumberEngineExecutionContext.lambda$runTestCase$4(CucumberEngineExecutionContext.java:112)
    at io.cucumber.core.runtime.CucumberExecutionContext.lambda$runTestCase$5(CucumberExecutionContext.java:137)
    at io.cucumber.core.runtime.RethrowingThrowableCollector.executeAndThrow(RethrowingThrowableCollector.java:23)
    at io.cucumber.core.runtime.CucumberExecutionContext.runTestCase(CucumberExecutionContext.java:137)
    at io.cucumber.junit.platform.engine.CucumberEngineExecutionContext.runTestCase(CucumberEngineExecutionContext.java:109)
    at io.cucumber.junit.platform.engine.NodeDescriptor$PickleDescriptor.execute(NodeDescriptor.java:168)
    at io.cucumber.junit.platform.engine.NodeDescriptor$PickleDescriptor.execute(NodeDescriptor.java:90)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
    at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
Caused by: io.cucumber.cucumberexpressions.CucumberExpressionException: Failed to parse number
    at io.cucumber.cucumberexpressions.NumberParser.parse(NumberParser.java:41)
    at io.cucumber.cucumberexpressions.NumberParser.parseDouble(NumberParser.java:21)
    at io.cucumber.cucumberexpressions.BuiltInParameterTransformer.doTransform(BuiltInParameterTransformer.java:80)
    at io.cucumber.cucumberexpressions.BuiltInParameterTransformer.transform(BuiltInParameterTransformer.java:22)
    at io.cucumber.cucumberexpressions.ParameterTypeRegistry$8.transform(ParameterTypeRegistry.java:114)
    at io.cucumber.cucumberexpressions.ParameterTypeRegistry$8.transform(ParameterTypeRegistry.java:111)
    at io.cucumber.cucumberexpressions.ParameterType$TransformerAdaptor.transform(ParameterType.java:299)
    at io.cucumber.cucumberexpressions.ParameterType.transform(ParameterType.java:259)
    at io.cucumber.cucumberexpressions.Argument.getValue(Argument.java:42)
    at io.cucumber.core.stepexpression.ExpressionArgument.getValue(ExpressionArgument.java:17)
    at io.cucumber.core.runner.PickleStepDefinitionMatch.runStep(PickleStepDefinitionMatch.java:47)
    ... 15 more
Caused by: java.text.ParseException: Unparseable number: "-2,11"
    at java.base/java.text.NumberFormat.parse(NumberFormat.java:434)
    at io.cucumber.cucumberexpressions.NumberParser.parse(NumberParser.java:39)
    ... 25 more

✅ What did you expect to see?

The text should be parsed to double value -2.11.

📦 Which tool/library version are you using?

Cucumber-java 12.15

mpkorstanje commented 4 months ago

I can't seem to reproduce this. Because your feature is in Norwegian, Cuucmber uses the no locale.

So you should be able to run this without any problems:

import java.text.DecimalFormat;
import java.text.NumberFormat;
import java.text.ParseException;
import java.util.Locale;

public class LocaleTest {

  public static void main(String[] args) throws ParseException {
    Locale locale = Locale.forLanguageTag("no");
    NumberFormat format = DecimalFormat.getNumberInstance(locale);
    Number parsed = format.parse("-2,11");
    System.out.println(parsed);
  }

}

Which leads me to think that perhaps your editor replaced - (ascii 45) with something else.

To debug this, you can put a breakpoint in the NumberParser.parseFloat method, and look at the code points in the string.

https://github.com/cucumber/cucumber-expressions/blob/3366223a23f8dc56dd827c5037ff0df6aef362cc/java/src/main/java/io/cucumber/cucumberexpressions/NumberParser.java#L9-L22

Btw, using double for money is not recommended. Either use a dedicated money type or BigDecimal to avoid rounding errors.

takle-sb1 commented 4 months ago

No, it's definitely ASCII 45. The entire string is [45, 50, 44, 49, 49] when viewing it in the debugger.

I noticed that when using the debugger to feed a unicode minus sign (U+2212) to numberFormat.parseinstead, it sucessfully parses the number.

mpkorstanje commented 4 months ago

Mmh. I tested this on 8.0.352-zulu. Which JVM are you using?

takle-sb1 commented 4 months ago
openjdk version "17.0.5" 2022-10-18
OpenJDK Runtime Environment Temurin-17.0.5+8 (build 17.0.5+8)
OpenJDK 64-Bit Server VM Temurin-17.0.5+8 (build 17.0.5+8, mixed mode, sharing)
mpkorstanje commented 4 months ago

Looks like a variation of https://bugs.openjdk.org/browse/JDK-8298849.

For my Native dutch the hypen is used. Do Norwegian keyboards have a dedicated key for the minus sign or is that just the hypen?

takle-sb1 commented 4 months ago

No, it's impossible to type the minus sign using any sane method. Everyone just uses the normal hyphen (ascii 45).

mpkorstanje commented 4 months ago

Oh that is just lovely. This also applies to:

Northern Sami (Latin, Norway)
Northern Sami (Norway)
Norwegian Bokmål (Svalbard & Jan Mayen)
Basque
Persian
Croatian (Latin, Croatia)
Norwegian
Swiss German
Norwegian (Latin, Norway)
Slovenian
Swiss German (France)
Estonian (Estonia)
Slovenian (Slovenia)
Finnish (Latin, Finland)
Norwegian Bokmål (Norway)
Persian (Arabic, Iran)
Finnish (Finland)
Colognian (Germany)
Romansh (Latin, Switzerland)
Norwegian (Norway)
Lithuanian (Lithuania)
Colognian (Latin, Germany)
Croatian (Croatia)
Slovenian (Latin, Slovenia)
Croatian
Lithuanian
Swiss German (Liechtenstein)
Croatian (Bosnia & Herzegovina)
Swedish (Sweden)
Persian (Afghanistan)
Swedish
Basque (Latin, Spain)
Swedish (Latin, Sweden)
Persian (Iran)
Swedish (Finland)
Faroese (Latin, Faroe Islands)
Colognian
Swiss German (Switzerland)
Estonian
Finnish
Romansh
Norwegian Bokmål (Latin, Norway)
Romansh (Switzerland)
Estonian (Latin, Estonia)
Northern Sami (Finland)
Lithuanian (Latin, Lithuania)
Northern Sami (Sweden)
Northern Sami
Faroese (Denmark)
Faroese (Faroe Islands)
Faroese
Basque (Spain)
Swiss German (Latin, Switzerland)
Swedish (Åland Islands)

So I'm half minded to override the minus sign to a dash when it is used.

takle-sb1 commented 4 months ago

That will probably work. It looks like it needs to be a hyphen in order to be recognized as a parameter anyway, i.e., replacing the hyphen with a unicode minus in the Gherkin file doesn't work.

mpkorstanje commented 4 months ago

Cheers good point.

I presume you also don't use a non-breaking space to separate thousands?

takle-sb1 commented 4 months ago

I presume you also don't use a non-breaking space to separate thousands?

That's correct, it has the same problem with not being possible to type on a keyboard, so only the most techically minded people will use it in practice. Using a regular space isn't uncommon, though, as is using a period.

mpkorstanje commented 4 months ago

Thanks for the explanation.

The fix should be in the latest version of cucumber-jvm.