tests/tweetnacl: Move from unittests to regular test by bergzand · Pull Request #10184 · RIOT-OS/RIOT

bergzand · 2018-10-17T19:58:41Z

Contribution description

Similar to #10183.

I think only qDSA and HACL* require a lot of stack size, removing those from the unittests probably removes the large stack requirement

Testing procedure

I suspect that the test works on AVR and msp430, but I'm not sure. Testing this is preferred in case those need to be blacklisted

Insufficient memory list is manually generated based on the tests/pkg_libcose blacklist and memory list.

Issues/PRs references

Part of #10187

kaspar030

ACK.

bergzand · 2018-10-17T20:58:05Z

This one is not as bad as the others, "only" 33 seconds:

2018-10-17 22:56:52,569 - INFO # main(): This is RIOT! (Version: 2018.10-devel-1213-gc45e1-dystopia-pr/tests/tweetnacl_move)
2018-10-17 22:57:25,576 - INFO # ...
2018-10-17 22:57:25,577 - INFO # OK (3 tests)

bergzand · 2018-10-18T13:35:45Z

rebased

bergzand · 2018-10-18T13:36:13Z

@kaspar030 Ready to squash? Does your ACK still hold with the recent changes?

cladmi · 2018-10-21T13:49:03Z

I get a 'context before hardfault' error on samr21-xpro and on iotlab-m3:

main(): This is RIOT! (Version: 2018.10-devel-1232-g2dd3-hal-pr/riot/10184/Move_from_unittests_to_regular_test)
...
OK (3 tests)

Context before hardfault:

make -C examples/hello-world/ BOARD=iotlab-m3 term
make: Entering directory '/home/cladmi/git/work/RIOT/examples/hello-world'
/home/cladmi/git/work/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyUSB1" -b "500000"
Twisted not available, please install it if you want to use pyterm's JSON capabilities
2018-10-21 15:42:32,140 - INFO # Connect to serial port /dev/ttyUSB1

Welcome to pyterm!
Type '/exit' to exit.


2018-10-21 15:42:44,828 - INFO # �main(): This is RIOT! (Version: 2018.10-devel-1232-g2dd3-hal-pr/riot/10184/Move_from_unittests_to_regular_test)

2018-10-21 15:42:52,073 - INFO # ...
2018-10-21 15:42:52,073 - INFO # OK (3 tests)
2018-10-21 15:42:52,073 - INFO #
2018-10-21 15:42:52,074 - INFO # Context before hardfault:
2018-10-21 15:42:52,074 - INFO #    r0: 0x20000200
2018-10-21 15:42:52,075 - INFO #    r1: 0x00000024
2018-10-21 15:42:52,075 - INFO #    r2: 0x20000294
2018-10-21 15:42:52,075 - INFO #    r3: 0x20000298
2018-10-21 15:42:52,076 - INFO #   r12: 0x0000001c
2018-10-21 15:42:52,076 - INFO #    lr: 0x080003a1
2018-10-21 15:42:52,077 - INFO #    pc: 0x20000200
2018-10-21 15:42:52,077 - INFO #   psr: 0x2000000b
2018-10-21 15:42:52,077 - INFO #
2018-10-21 15:42:52,078 - INFO # FSR/FAR:
2018-10-21 15:42:52,078 - INFO #  CFSR: 0x00020000
2018-10-21 15:42:52,079 - INFO #  HFSR: 0x40000000
2018-10-21 15:42:52,079 - INFO #  DFSR: 0x00000000
2018-10-21 15:42:52,080 - INFO #  AFSR: 0x00000000
2018-10-21 15:42:52,080 - INFO # Misc
2018-10-21 15:42:52,081 - INFO # EXC_RET: 0xfffffff1
2018-10-21 15:42:52,082 - INFO # Attempting to reconstruct state for debugging...
2018-10-21 15:42:52,082 - INFO # In GDB:
2018-10-21 15:42:52,083 - INFO #   set $pc=0x20000200
2018-10-21 15:42:52,083 - INFO #   frame 0
2018-10-21 15:42:52,084 - INFO #   bt
2018-10-21 15:42:52,084 - INFO #
2018-10-21 15:42:52,084 - INFO # ISR stack overflowed by at least 48 bytes.

My toolchain:

/dist/tools/ci/print_toolchain_versions.sh 

Operating System Environment
-----------------------------
       Operating System: "Arch Linux" 
                 Kernel: Linux 4.18.14-arch1-1-ARCH x86_64 unknown

Installed compiler toolchains
-----------------------------
             native gcc: gcc (GCC) 8.2.1 20180831
      arm-none-eabi-gcc: arm-none-eabi-gcc (Arch Repository) 8.2.0
                avr-gcc: avr-gcc (GCC) 8.2.0
       mips-mti-elf-gcc: mips-mti-elf-gcc (Codescape GNU Tools 2017.10-08 for MIPS MTI Bare Metal) 6.3.0
             msp430-gcc: msp430-gcc (mspgcc_20120406) 4.6.3 20120301 (mspgcc LTS 20120406 unpatched)
   riscv-none-embed-gcc: missing
                  clang: clang version 7.0.0 (tags/RELEASE_700/final)

Installed compiler libs
-----------------------
   arm-none-eabi-newlib: "3.0.0"
    mips-mti-elf-newlib: "2.5.0"
riscv-none-embed-newlib: missing
               avr-libc: "2.0.0" ("20150208")

Installed development tools
---------------------------
                  cmake: cmake version 3.12.3
               cppcheck: missing
                doxygen: 1.8.14
                 flake8: 3.5.0 (mccabe: 0.6.1, pycodestyle: 2.3.1, pyflakes: 1.6.0) CPython 3.7.0 on Linux
                    git: git version 2.19.1
                openocd: Open On-Chip Debugger 0.10.0
                 python: Python 3.7.0
                python2: Python 2.7.15
                python3: Python 3.7.0
             coccinelle: missing

cladmi · 2018-10-21T13:52:17Z

On the iotlab-m3:

gdb) set $pc=0x20000200
(gdb) frame 0
#0  0x20000200 in heap_top ()
(gdb) bt
#0  0x20000200 in heap_top ()
#1  0xfffffffe in ?? ()
Backtrace stopped: previous frame identical to this frame (corrupt stack?)

bergzand · 2018-10-21T13:53:44Z

Ah, never noticed the hard fault because the make tests exits before it occurs :)

cladmi · 2018-10-21T13:56:01Z

tests/pkg_tweetnacl/Makefile

+
+TEST_ON_CI_WHITELIST += native
+
+CFLAGS += -DTHREAD_STACKSIZE_MAIN=\(4*THREAD_STACKSIZE_DEFAULT\)


Using the stack size value used in unittests works without crash:

Suggested change

CFLAGS += -DTHREAD_STACKSIZE_MAIN=\(4*THREAD_STACKSIZE_DEFAULT\)

CFLAGS += -DTHREAD_STACKSIZE_MAIN=\(4*THREAD_STACKSIZE_DEFAULT+THREAD_EXTRA_STACKSIZE_PRINTF\)

I've incremented the "4" to "5".

Agreed if you put the comment in the commit too as mentioned in #10185 (comment)

cladmi

Currently tests are broken.

bergzand · 2018-10-21T13:57:14Z

@cladmi Thanks for testing, I've increased the stack size a bit. With that the test succeeds on the samr21-xpro

cladmi

ACK the tests now works on iotlab-m3 and samr21-xpro please squash.

When doing it please include something like this in the commit message to explain the stack size: #10185 (comment)

bergzand · 2018-10-21T18:08:00Z

squashed. Included the same information in the PR as with #10185

cladmi · 2018-10-23T14:58:19Z

For info on stack Size (usage):

iotlab-m3: 5120 ( 4196)
samr21-xpro: 5120 ( 4272)

Currently it crashes on arduino-mega2560 I think because of stack usage (I will try to give some numbers). And it is also really really slow, so maybe it can be blacklisted anyway.

There is also one board that should be removed because of not enough memory: nucleo-f042k6.

cladmi · 2018-10-23T15:05:07Z

I agree with the commit message.

cladmi · 2018-10-23T15:07:38Z

For arduino-mega2560 with a stack size of 16* the default size, I get this stack usage reported by ps: 4096 ( 4062) and it takes almost 8 minutes D:

So better blacklist it anyway.

bergzand · 2018-10-23T15:19:20Z

For arduino-mega2560 with a stack size of 16* the default size, I get this stack usage reported by ps: 4096 ( 4062) and it takes almost 8 minutes D:

But it doesn't fail 😆

cladmi · 2018-10-23T15:21:51Z

tests/pkg_tweetnacl/Makefile


 BOARD_INSUFFICIENT_MEMORY := arduino-duemilanove \
                             arduino-uno \
+			     arduino-mega2560 \


Spaces instead of tabs :)

Not anymore

Update not pushed or github lagging ?

Ssssssht ☕

bergzand · 2018-10-23T15:22:49Z

@cladmi I've blacklisted both the arduino-mega2560 and the mega-xplained. That last one is probably as slow as as the first one.

cladmi · 2018-10-23T16:38:53Z

Please rebase and squash with the commit message saying we now blacklisted the boards with the reason.

cladmi · 2018-10-23T16:40:16Z

No need for "run tests" as native tests are always run.

bergzand · 2018-10-24T08:08:29Z

Squashed and rebased. Mentioned the arduino boards in the commit message

cladmi · 2018-10-24T09:07:02Z

tests/pkg_tweetnacl/Makefile

+                             arduino-mega2560 \
+                             mega-xplained \
+                             nucleo-f031k6 \
+                             #


Just missing the nucleo-f042k6 here to make murdock happy. You can inline squash it to test it directly so it is ready to merge after.

Just missing the nucleo-f042k6 here to make murdock happy. You can inline squash it to test it directly so it is ready to merge after.

Fixed -_-

No wait, tabs!

Now it is fixed :)

tests/pkg_tweetnacl/tests/01-run.py

cladmi · 2018-10-24T14:50:51Z

Just waiting for murdock and it can be merged!

cladmi · 2018-10-24T14:51:18Z

Thank you for following with the fixes and updates.

bergzand · 2018-10-24T14:52:45Z

Do you want me to squash that last commit?

cladmi · 2018-10-24T14:55:36Z

As you wish.
But indeed it makes sense to fix it in the same commit. You can ignore my authorship.

Stack size is changed from 4 times the default + printf to 5 times the default stack size. Only on the lpc2387 this reduces the resulting stack space. the test is not rerun for the lpc2387 and is untested. Tests are disable for the Arduino. While they "might" work, it takes around 8 minutes to complete the tests.

cladmi · 2018-10-25T09:16:53Z

Unreliable native test again… I restart murdock.

cladmi · 2018-10-25T10:27:35Z

Thank you for taking care of the migration ! :)

bergzand added Area: tests Area: tests and testing framework Area: pkg Area: External package ports Type: cleanup The issue proposes a clean-up / The PR cleans-up parts of the codebase / documentation labels Oct 17, 2018

miri64 requested a review from kaspar030 October 17, 2018 20:06

bergzand added the CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR label Oct 17, 2018

kaspar030 approved these changes Oct 17, 2018

View reviewed changes

bergzand mentioned this pull request Oct 17, 2018

tests/unittests: Move large unittests to individual tests #10187

Open

16 tasks

This was referenced Oct 18, 2018

tests/cn-cbor: move from unittests to regular test #10188

Merged

tests/relic: move from unittests to regular test #10189

Merged

bergzand force-pushed the pr/tests/tweetnacl_move branch 3 times, most recently from 4ad9705 to 2dd3dba Compare October 18, 2018 13:33

smlng mentioned this pull request Oct 19, 2018

tests/heatshrink: move from unittests to regular test #10202

Merged

cladmi reviewed Oct 21, 2018

View reviewed changes

cladmi suggested changes Oct 21, 2018

View reviewed changes

cladmi approved these changes Oct 21, 2018

View reviewed changes

bergzand force-pushed the pr/tests/tweetnacl_move branch from 9a0fffc to 7bba650 Compare October 21, 2018 18:07

cladmi reviewed Oct 23, 2018

View reviewed changes

bergzand force-pushed the pr/tests/tweetnacl_move branch 2 times, most recently from b416de8 to ed84bc7 Compare October 24, 2018 08:08

cladmi reviewed Oct 24, 2018

View reviewed changes

bergzand force-pushed the pr/tests/tweetnacl_move branch 2 times, most recently from 86b82fc to 78cbfc8 Compare October 24, 2018 09:26

cladmi reviewed Oct 24, 2018

View reviewed changes

tests/pkg_tweetnacl/tests/01-run.py Outdated Show resolved Hide resolved

bergzand force-pushed the pr/tests/tweetnacl_move branch from e0d3225 to 137e482 Compare October 24, 2018 14:56

cladmi added CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR and removed CI: ready for build If set, CI server will compile all applications for all available boards for the labeled PR labels Oct 25, 2018

cladmi merged commit 40187e8 into RIOT-OS:master Oct 25, 2018

cladmi added this to the Release 2018.10 milestone Oct 25, 2018


		TEST_ON_CI_WHITELIST += native

		CFLAGS += -DTHREAD_STACKSIZE_MAIN=\(4*THREAD_STACKSIZE_DEFAULT\)

Conversation

bergzand commented Oct 17, 2018 • edited by cladmi Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Contribution description

Testing procedure

Issues/PRs references

Uh oh!

kaspar030 left a comment

Choose a reason for hiding this comment

Uh oh!

bergzand commented Oct 17, 2018

Uh oh!

bergzand commented Oct 18, 2018

Uh oh!

bergzand commented Oct 18, 2018

Uh oh!

cladmi commented Oct 21, 2018

Uh oh!

cladmi commented Oct 21, 2018

Uh oh!

bergzand commented Oct 21, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cladmi left a comment

Choose a reason for hiding this comment

Uh oh!

bergzand commented Oct 21, 2018

Uh oh!

cladmi left a comment

Choose a reason for hiding this comment

Uh oh!

bergzand commented Oct 21, 2018

Uh oh!

cladmi commented Oct 23, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

cladmi commented Oct 23, 2018

Uh oh!

cladmi commented Oct 23, 2018

Uh oh!

bergzand commented Oct 23, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bergzand commented Oct 23, 2018

Uh oh!

cladmi commented Oct 23, 2018

Uh oh!

cladmi commented Oct 23, 2018

Uh oh!

bergzand commented Oct 24, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cladmi commented Oct 24, 2018

Uh oh!

cladmi commented Oct 24, 2018

Uh oh!

bergzand commented Oct 24, 2018

Uh oh!

cladmi commented Oct 24, 2018

Uh oh!

bergzand commented Oct 17, 2018 •

edited by cladmi

Loading

cladmi commented Oct 23, 2018 •

edited

Loading