From 16148ed7466f5361558bce82b42d979b82d38df6 Mon Sep 17 00:00:00 2001 From: Stephan Renatus Date: Fri, 11 Dec 2020 11:06:40 +0100 Subject: [PATCH 1/3] test/cases: disregard ordering in "strings/concat: set" test Since anything can happen, we now list the possibilities (3! = 6), and check that any of them was the result. Fixes #2924. Signed-off-by: Stephan Renatus --- internal/wasm/sdk/test/e2e/exceptions.yaml | 1 - .../testdata/strings/test-strings-0883.yaml | 104 +++--------------- 2 files changed, 17 insertions(+), 88 deletions(-) diff --git a/internal/wasm/sdk/test/e2e/exceptions.yaml b/internal/wasm/sdk/test/e2e/exceptions.yaml index 495578a0cb..436becbaf1 100644 --- a/internal/wasm/sdk/test/e2e/exceptions.yaml +++ b/internal/wasm/sdk/test/e2e/exceptions.yaml @@ -1,6 +1,5 @@ # Exception Format is : "baseandvirtualdocs/base/virtual: conflicts": "document merge conflict - https://github.com/open-policy-agent/opa/issues/2926" -"strings/concat: set": "test result order is not consistent - https://github.com/open-policy-agent/opa/issues/2924" "strings/format_int": "rounding error - https://github.com/open-policy-agent/opa/issues/2923" "strings/format_int: ref dest": "rounding error - https://github.com/open-policy-agent/opa/issues/2923" "strings/format_int: ref dest (2)": "rounding error - https://github.com/open-policy-agent/opa/issues/2923" diff --git a/test/cases/testdata/strings/test-strings-0883.yaml b/test/cases/testdata/strings/test-strings-0883.yaml index 4955668c3b..d469334ad4 100644 --- a/test/cases/testdata/strings/test-strings-0883.yaml +++ b/test/cases/testdata/strings/test-strings-0883.yaml @@ -1,95 +1,25 @@ cases: - data: - a: - - 1 - - 2 - - 3 - - 4 - b: - v1: hello - v2: goodbye - c: - - x: - - true - - false - - foo - "y": - - null - - 3.14159 - z: - p: true - q: false - d: - e: - - bar - - baz - f: - - xs: - - 1 - ys: - - 2 - - xs: - - 2 - ys: - - 3 - g: - a: - - 1 - - 0 - - 0 - - 0 - b: - - 0 - - 2 - - 0 - - 0 - c: - - 0 - - 0 - - 0 - - 4 - h: - - - 1 - - 2 - - 3 - - - 2 - - 3 - - 4 - l: - - a: bob - b: -1 - c: - - 1 - - 2 - - 3 - - 4 - - a: alice - b: 1 - c: - - 2 - - 3 - - 4 - - 5 - d: null - m: [] - numbers: - - "1" - - "2" - - "3" - - "4" - strings: - bar: 2 - baz: 3 - foo: 1 - three: 3 modules: - | - package generated + package test - p = x { - concat(",", {"1", "2", "3"}, x) + # Sets are unordered, so the output is not guaranteed. + # These are theoretically possible: + possibilities = { + "1,2,3", + "2,3,1", + "3,1,2", + "3,2,1", + "2,1,3", + "1,3,2" + } + + p { + x := concat(",", {"1", "2", "3"}) + possibilities[x] } note: 'strings/concat: set' - query: data.generated.p = x + query: data.test.p = x want_result: - - x: 1,2,3 + - x: true From f2d8dda629ff4ab778e166b3d503120266444ab9 Mon Sep 17 00:00:00 2001 From: Stephan Renatus Date: Fri, 11 Dec 2020 13:13:54 +0100 Subject: [PATCH 2/3] wasm: fix rounding mode Same as in topdown, we should have this be true in wasm: round(1.5) == 2 round(2.5) == 3 The default rounding mode for libmpdec's maxcontext was MPD_ROUND_HALF_EVEN, would would round 2.5 to 2. See https://www.bytereef.org/mpdecimal/doc/libmpdec/context.html#rounding Signed-off-by: Stephan Renatus --- .../arithmetic/test-arithmetic-0813.yaml | 84 +------------------ test/wasm/assets/018_builtins.yaml | 6 ++ wasm/src/mpd.c | 1 + wasm/tests/test.c | 2 + 4 files changed, 12 insertions(+), 81 deletions(-) diff --git a/test/cases/testdata/arithmetic/test-arithmetic-0813.yaml b/test/cases/testdata/arithmetic/test-arithmetic-0813.yaml index f15b126853..59cf974e51 100644 --- a/test/cases/testdata/arithmetic/test-arithmetic-0813.yaml +++ b/test/cases/testdata/arithmetic/test-arithmetic-0813.yaml @@ -5,95 +5,17 @@ cases: - 2 - 3 - 4 - b: - v1: hello - v2: goodbye - c: - - x: - - true - - false - - foo - "y": - - null - - 3.14159 - z: - p: true - q: false - d: - e: - - bar - - baz - f: - - xs: - - 1 - ys: - - 2 - - xs: - - 2 - ys: - - 3 - g: - a: - - 1 - - 0 - - 0 - - 0 - b: - - 0 - - 2 - - 0 - - 0 - c: - - 0 - - 0 - - 0 - - 4 - h: - - - 1 - - 2 - - 3 - - - 2 - - 3 - - 4 - l: - - a: bob - b: -1 - c: - - 1 - - 2 - - 3 - - 4 - - a: alice - b: 1 - c: - - 2 - - 3 - - 4 - - 5 - d: null - m: [] - numbers: - - "1" - - "2" - - "3" - - "4" - strings: - bar: 2 - baz: 3 - foo: 1 - three: 3 modules: - | - package generated + package test p[z] { data.a[i] = x - __local0__ = i / x - y = __local0__ + y = i / x round(y, z) } note: arithmetic/divide+round - query: data.generated.p = x + query: data.test.p = x sort_bindings: true want_result: - x: diff --git a/test/wasm/assets/018_builtins.yaml b/test/wasm/assets/018_builtins.yaml index fbf35077ec..1c66819b39 100644 --- a/test/wasm/assets/018_builtins.yaml +++ b/test/wasm/assets/018_builtins.yaml @@ -5,6 +5,12 @@ cases: - note: round built-in query: round(1.4,x) want_result: [{'x': 1}] + - note: round built-in ("halfs up") + query: round(1.5,x) + want_result: [{'x': 2}] + - note: round built-in ("halfs up not to even") + query: round(2.5,x) + want_result: [{'x': 3}] - note: plus built-in query: plus(1,1,x) want_result: [{'x': 2}] diff --git a/wasm/src/mpd.c b/wasm/src/mpd.c index 7893434a8d..1f4c5bd7f0 100644 --- a/wasm/src/mpd.c +++ b/wasm/src/mpd.c @@ -17,6 +17,7 @@ static void init(void) mpd_maxcontext(&max_ctx); max_ctx.traps = 0; + max_ctx.round = MPD_ROUND_HALF_UP; // .5 always rounded up one = mpd_qnew(); diff --git a/wasm/tests/test.c b/wasm/tests/test.c index 1f9f07b156..7f69487e60 100644 --- a/wasm/tests/test.c +++ b/wasm/tests/test.c @@ -1268,6 +1268,8 @@ void test_arithmetic(void) test("round -1.4 (float)", opa_number_as_float(opa_cast_number(opa_arith_round(opa_number_float(-1.4)))) == -1); test("round 1.5 (float)", opa_number_as_float(opa_cast_number(opa_arith_round(opa_number_float(1.5)))) == 2); test("round -1.5 (float)", opa_number_as_float(opa_cast_number(opa_arith_round(opa_number_float(-1.5)))) == -2); + test("round 2.5 (float)", opa_number_as_float(opa_cast_number(opa_arith_round(opa_number_float(2.5)))) == 3); + test("round -2.5 (float)", opa_number_as_float(opa_cast_number(opa_arith_round(opa_number_float(-2.5)))) == -3); test("plus 1+2", opa_number_as_float(opa_cast_number(opa_arith_plus(opa_number_float(1), opa_number_float(2)))) == 3); test("minus 3-2", opa_number_as_float(opa_cast_number(opa_arith_minus(opa_number_float(3), opa_number_float(2)))) == 1); From f292ab4682ba8c207374118d9cefc74c6279ef89 Mon Sep 17 00:00:00 2001 From: Stephan Renatus Date: Fri, 11 Dec 2020 13:46:10 +0100 Subject: [PATCH 3/3] wasm: fix number truncation format_int I'm not exactly sure why we had been rounding here before. Topdown truncates when formatting a decimal number as int: format_int(15.9, 16) == "f" format_int(-15.9, 16) == "-f" Fixes #2923. Signed-off-by: Stephan Renatus --- internal/wasm/sdk/test/e2e/exceptions.yaml | 4 ---- wasm/src/strings.c | 20 +++++++++++--------- wasm/tests/test.c | 2 ++ 3 files changed, 13 insertions(+), 13 deletions(-) diff --git a/internal/wasm/sdk/test/e2e/exceptions.yaml b/internal/wasm/sdk/test/e2e/exceptions.yaml index 436becbaf1..10197031fa 100644 --- a/internal/wasm/sdk/test/e2e/exceptions.yaml +++ b/internal/wasm/sdk/test/e2e/exceptions.yaml @@ -1,9 +1,5 @@ # Exception Format is : "baseandvirtualdocs/base/virtual: conflicts": "document merge conflict - https://github.com/open-policy-agent/opa/issues/2926" -"strings/format_int": "rounding error - https://github.com/open-policy-agent/opa/issues/2923" -"strings/format_int: ref dest": "rounding error - https://github.com/open-policy-agent/opa/issues/2923" -"strings/format_int: ref dest (2)": "rounding error - https://github.com/open-policy-agent/opa/issues/2923" -"strings/format_int: undefined": "rounding error - https://github.com/open-policy-agent/opa/issues/2923" "withkeyword/with virtual doc specific index": "with target conflict issue - https://github.com/open-policy-agent/opa/issues/2922" "withkeyword/with virtual doc not specific index": "with target conflict issue - https://github.com/open-policy-agent/opa/issues/2922" "withkeyword/with virtual doc exact value": "with target conflict issue - https://github.com/open-policy-agent/opa/issues/2922" diff --git a/wasm/src/strings.c b/wasm/src/strings.c index eabbd37e19..7c56d09aac 100644 --- a/wasm/src/strings.c +++ b/wasm/src/strings.c @@ -209,24 +209,26 @@ opa_value *opa_strings_format_int(opa_value *a, opa_value *b) mpd_t *i = mpd_qnew(); uint32_t status = 0; - - mpd_qround_to_intx(i, input, mpd_max_ctx(), &status); - if (status & ~MPD_Rounded) + mpd_qtrunc(i, input, mpd_max_ctx(), &status); + if (status != 0) { - opa_abort("strings: invalid rounding"); + opa_abort("strings: truncate failed"); } - opa_value *n = opa_bf_to_number(i); - opa_number_try_int(opa_cast_number(n), &v); + int32_t w = mpd_qget_i32(i, &status); + if (status != 0) + { + opa_abort("strings: get uint failed"); + } char *str = opa_malloc(21); // enough for int_t (with sign). - if (v < 0) + if (w < 0) { str[0] = '-'; - snprintf(&str[1], 21, format, -v); + snprintf(&str[1], 21, format, -w); } else { - snprintf(str, 21, format, v); + snprintf(str, 21, format, w); } return opa_string_allocated(str, opa_strlen(str)); diff --git a/wasm/tests/test.c b/wasm/tests/test.c index 7f69487e60..1ec91c63db 100644 --- a/wasm/tests/test.c +++ b/wasm/tests/test.c @@ -1796,6 +1796,8 @@ void test_strings(void) test("format_int/16_0", opa_value_compare(opa_strings_format_int(opa_number_float(0), opa_number_int(16)), opa_string_terminated("0")) == 0); test("format_int/16_1", opa_value_compare(opa_strings_format_int(opa_number_float(1), opa_number_int(16)), opa_string_terminated("1")) == 0); test("format_int/16_-1", opa_value_compare(opa_strings_format_int(opa_number_float(-1), opa_number_int(16)), opa_string_terminated("-1")) == 0); + test("format_int/16_15.5", opa_value_compare(opa_strings_format_int(opa_number_float(15.5), opa_number_int(16)), opa_string_terminated("f")) == 0); + test("format_int/16_-15.5", opa_value_compare(opa_strings_format_int(opa_number_float(-15.5), opa_number_int(16)), opa_string_terminated("-f")) == 0); test("format_int/16_16", opa_value_compare(opa_strings_format_int(opa_number_float(16), opa_number_int(16)), opa_string_terminated("10")) == 0); test("format_int/16_31", opa_value_compare(opa_strings_format_int(opa_number_float(31), opa_number_int(16)), opa_string_terminated("1f")) == 0);