Fix tests on targets that don't support floats #3969

aykevl · 2018-07-22T14:18:06Z

While trying to get the nrf51 emulator to work (which passes most tests now!), I noticed there is no full support for targets that don't support floating point as some tests assume the presence of a floating point unit.

EDIT: offtopic, but with a bigger heap I managed to pass all tests in the nrf51 emulator :D apparently 10kB was a bit too small for a few tests (they all caused a MemoryError).

dpgeorge · 2018-07-23T04:50:38Z

py/objstr.c

@@ -2098,11 +2098,11 @@ bool mp_obj_str_equal(mp_obj_t s1, mp_obj_t s2) {

 STATIC NORETURN void bad_implicit_conversion(mp_obj_t self_in) {
    if (MICROPY_ERROR_REPORTING == MICROPY_ERROR_REPORTING_TERSE) {
-        mp_raise_TypeError("can't convert to str implicitly");
+        mp_raise_TypeError("Can't convert to str implicitly");


Most (I thought it was "all" but apparently not) of the CPython error messages start with a lower case letter, so that's what we try and do consistently in uPy. And we don't match 100% with CPython error messages because it's not really important. So I don't see a reason to make this change.

The reason I changed is that the test would otherwise fail (apparently run-tests is testing against CPython?). But with the changes suggested below this isn't necessary anymore.

dpgeorge · 2018-07-23T04:50:48Z

py/objstr.c

    } else {
        const qstr src_name = mp_obj_get_type(self_in)->name;
        nlr_raise(mp_obj_new_exception_msg_varg(&mp_type_TypeError,
-            "can't convert '%q' object to %q implicitly",
+            "Can't convert '%q' object to %q implicitly",


Same as above comment

dpgeorge · 2018-07-23T04:52:06Z

tests/basics/op_precedence.py

+except SyntaxError:
+    print('0.5')  # skip test
+else:
+    print(2**-1)


Good catch! What it's really testing is order or parsing of ** -, not FP, so I think it's best to change it to something that works without FP. How about 2**-0? Or even 2**-(-1).

Ah, of course! 2**-0 doesn't use floating point so that works.

dpgeorge · 2018-07-23T04:54:12Z

tests/extmod/urandom_extra.py

+    pass
+else:
+    for i in range(50):
+        assert 0 <= random.random() < 1


You'll see that ujson float tests are split out into separate files. Let's do the same here: move all float related functions from this file into urandom_extra_float.py

Done. That's a lot nicer.

dpgeorge · 2018-07-23T04:55:24Z

tests/misc/print_exception.py

@@ -31,7 +31,7 @@ def print_exc(e):

 # basic exception message
 try:
-    1/0
+    '1'+1


How about 1//0, to try and make a minimal change?

dpgeorge · 2018-07-23T04:55:35Z

tests/misc/print_exception.py

@@ -40,7 +40,7 @@ def print_exc(e):
 def f():
    g()
 def g():
-    2/0
+    ''+2


Same as above, 2//0?

dpgeorge · 2018-07-23T05:01:05Z

offtopic, but with a bigger heap I managed to pass all tests in the nrf51 emulator :D apparently 10kB was a bit too small for a few tests (they all caused a MemoryError).

At some point in the past there was an effort to make the tests in tests/basics/ all small enough to run on a 16k heap (IIRC). It might be a good idea to enforce that with a new Travis test, eg ./run-tests --heapsize 16wk

aykevl · 2018-07-23T15:23:00Z

Those are all good suggestions. I've updated the PR.

EDIT: I'll investigate soonish why it doesn't work at the moment. It is probably because divide by zero gives a different error message in CPython and MicroPython (that's what it was the last time I looked).

dpgeorge · 2018-07-24T00:22:44Z

I'll investigate soonish why it doesn't work at the moment. It is probably because divide by zero gives a different error message in CPython and MicroPython (that's what it was the last time I looked).

Yes that's right.

The aim of print_exception.py is to test details of exception and traceback printing. It makes sense that it would work without floats, so ports without float can include this test. So instead of a divide by zero exception (float or integer) I'd suggest changing it to something which has the same error message in both CPy and uPy. A way to do this would be just raise an explicit exception, like raise Exception('message'), instead of the divide by zero.

aykevl · 2018-08-02T14:20:52Z

Fixed it. Together with #3970 I can use my emulator for the PCA10028 passing all tests.

dpgeorge · 2018-08-04T05:15:46Z

Thanks! Merged in 6572029

Fixes and Enhancement for Touch Alarm

dpgeorge reviewed Jul 23, 2018

View reviewed changes

aykevl force-pushed the tests-nofloat branch 2 times, most recently from f4ff3a8 to 5ced89c Compare July 23, 2018 15:20

aykevl force-pushed the tests-nofloat branch from 5ced89c to f936fd1 Compare July 23, 2018 16:20

aykevl force-pushed the tests-nofloat branch from f936fd1 to c1410b2 Compare July 30, 2018 15:11

tests: Make tests work on targets without float support.

c1410b2

dpgeorge closed this Aug 4, 2018

tannewt pushed a commit to tannewt/circuitpython that referenced this pull request Jan 21, 2021

Merge pull request micropython#3969 from microDev1/TouchAlarm

679b465

Fixes and Enhancement for Touch Alarm

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix tests on targets that don't support floats #3969

Fix tests on targets that don't support floats #3969

aykevl commented Jul 22, 2018 •

edited

dpgeorge Jul 23, 2018

aykevl Jul 23, 2018

dpgeorge Jul 23, 2018

dpgeorge Jul 23, 2018

aykevl Jul 23, 2018

dpgeorge Jul 23, 2018

aykevl Jul 23, 2018

dpgeorge Jul 23, 2018

aykevl Jul 23, 2018

dpgeorge Jul 23, 2018

dpgeorge commented Jul 23, 2018

aykevl commented Jul 23, 2018 •

edited

dpgeorge commented Jul 24, 2018

aykevl commented Aug 2, 2018

dpgeorge commented Aug 4, 2018

Fix tests on targets that don't support floats #3969

Fix tests on targets that don't support floats #3969

Conversation

aykevl commented Jul 22, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dpgeorge commented Jul 23, 2018

aykevl commented Jul 23, 2018 • edited

dpgeorge commented Jul 24, 2018

aykevl commented Aug 2, 2018

dpgeorge commented Aug 4, 2018

aykevl commented Jul 22, 2018 •

edited

aykevl commented Jul 23, 2018 •

edited