Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

tests/test_tools: set RIOT_TERMINAL to socat #15882

Closed
wants to merge 1 commit into from

Conversation

jia200x
Copy link
Member

@jia200x jia200x commented Jan 28, 2021

Contribution description

For some reason pyterm sends ANSI escape sequences. The tests fails when
trying to read the clean output from the node.
I believe we should check why pyterm does that, but I propose this fix
in the meantime.

I tested this one on samr21-xpro and iotlab-m3, but I guess all boards should be affected.

Testing procedure

BOARD=samr21-xpro make -C tests/test_tools flash test should pass.

Issues/PRs references

Found while testing in RIOT-OS/Release-Specs#206

For some reason pyterm sends ANSI escape sequences. The tests fails when
trying to read the clean output from the node.
I believe we should check why pyterm does that, but I propose this fix
in the meantime
@jia200x jia200x requested a review from miri64 January 28, 2021 16:23
@jia200x jia200x added Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch Area: tests Area: tests and testing framework labels Jan 28, 2021
@jia200x
Copy link
Member Author

jia200x commented Jan 28, 2021

With this PR:

shellping
socat - open:/dev/ttyACM0,b115200,echo=0,raw 
shellpong
true this should not be echoed
shellping
shellpong
toupper lowercase
LOWERCASE
getchar

getchar 0x0a

make: Leaving directory '/home/jialamos/Development/RIOT/tests/test_tools'

Without:

shellpong
true this should not be echoed
shellping
shellpong
toupper lowercase
LOWERCASE

Traceback (most recent call last):
  File "/home/jialamos/Development/RIOT/tests/test_tools/tests/01-run.py", line 83, in <module>
    sys.exit(run(testfunc))
  File "/home/jialamos/Development/RIOT/dist/pythonlibs/testrunner/__init__.py", line 30, in run
    testfunc(child)
  File "/home/jialamos/Development/RIOT/tests/test_tools/tests/01-run.py", line 76, in testfunc
    _test_clean_output(child)
  File "/home/jialamos/Development/RIOT/tests/test_tools/tests/01-run.py", line 55, in _test_clean_output
    assert retline.strip() == 'LOWERCASE'
AssertionError
make: *** [/home/jialamos/Development/RIOT/makefiles/tests/tests.inc.mk:22: test] Error 1
make: Leaving directory '/home/jialamos/Development/RIOT/tests/test_tools'

@aabadie
Copy link
Contributor

aabadie commented Jan 28, 2021

I can't reproduce the problem locally, it works like a charm on master and 2021.01-branch (tested with a samr21-xpro plugged to my computer).

$ BOARD=samr21-xpro make -C tests/test_tools flash test
make: Entering directory '/work/riot/RIOT/tests/test_tools'
Building application "tests_test_tools" for "samr21-xpro" with MCU "samd21".

"make" -C /work/riot/RIOT/boards/samr21-xpro
"make" -C /work/riot/RIOT/core
"make" -C /work/riot/RIOT/cpu/samd21
"make" -C /work/riot/RIOT/cpu/cortexm_common
"make" -C /work/riot/RIOT/cpu/cortexm_common/periph
"make" -C /work/riot/RIOT/cpu/sam0_common
"make" -C /work/riot/RIOT/cpu/sam0_common/periph
"make" -C /work/riot/RIOT/cpu/samd21/periph
"make" -C /work/riot/RIOT/cpu/samd21/vectors
"make" -C /work/riot/RIOT/drivers
"make" -C /work/riot/RIOT/drivers/periph_common
"make" -C /work/riot/RIOT/sys
"make" -C /work/riot/RIOT/sys/auto_init
"make" -C /work/riot/RIOT/sys/isrpipe
"make" -C /work/riot/RIOT/sys/malloc_thread_safe
"make" -C /work/riot/RIOT/sys/newlib_syscalls_default
"make" -C /work/riot/RIOT/sys/pm_layered
"make" -C /work/riot/RIOT/sys/shell
"make" -C /work/riot/RIOT/sys/stdio_uart
"make" -C /work/riot/RIOT/sys/test_utils/dummy_thread
"make" -C /work/riot/RIOT/sys/tsrb
   text	   data	    bss	    dec	    hex	filename
  10656	    132	   2624	  13412	   3464	/work/riot/RIOT/tests/test_tools/bin/samr21-xpro/tests_test_tools.elf
[INFO] edbg binary not found - building it from source now
CC= CFLAGS= make -C /work/riot/RIOT/dist/tools/edbg
[INFO] edbg binary successfully built!
/work/riot/RIOT/dist/tools/edbg/edbg  --target samr21 --verbose --file /work/riot/RIOT/tests/test_tools/bin/samr21-xpro/tests_test_tools.bin --verify || /work/riot/RIOT/dist/tools/edbg/edbg  --target samr21 --verbose --file /work/riot/RIOT/tests/test_tools/bin/samr21-xpro/tests_test_tools.bin --verify --program
Debugger: ATMEL EDBG CMSIS-DAP ATML2127031800009840 01.1A.00FB (S)
Clock frequency: 16.0 MHz
Target: SAM R21G18 (Rev C)
Verification.............................................. done.
shellping
/work/riot/RIOT/dist/tools/pyterm/pyterm -p "/dev/ttyACM0" -b "115200" --no-reconnect --noprefix --no-repeat-command-on-empty-line 
Twisted not available, please install it if you want to use pyterm's JSON capabilities
Connect to serial port /dev/ttyACM0
Welcome to pyterm!
Type '/exit' to exit.
shellpong
true this should not be echoed
shellping
shellpong
toupper lowercase
LOWERCASE
getchar

getchar 0x0a

make: Leaving directory '/work/riot/RIOT/tests/test_tools'

What is your Python version ? I have 3.8.6 but I don't think that should make a big difference.

@jia200x
Copy link
Member Author

jia200x commented Jan 28, 2021

This is my local setup:

Toolchain

Operating System Environment
----------------------------
         Operating System: "Arch Linux" 
                   Kernel: Linux 5.10.9-arch1-1 x86_64 unknown
             System shell: GNU bash, version 5.1.4(1)-release (x86_64-pc-linux-gnu)
             make's shell: GNU bash, version 5.1.4(1)-release (x86_64-pc-linux-gnu)

Installed compiler toolchains
-----------------------------
               native gcc: gcc (GCC) 10.2.0
        arm-none-eabi-gcc: arm-none-eabi-gcc (GNU Tools for Arm Embedded Processors 7-2017-q4-major) 7.2.1 20170904 (release) [ARM/embedded-7-branch revision 255204]
                  avr-gcc: avr-gcc (GCC) 10.2.0
         mips-mti-elf-gcc: missing
           msp430-elf-gcc: missing
       riscv-none-elf-gcc: missing
  riscv64-unknown-elf-gcc: missing
     riscv-none-embed-gcc: missing
     xtensa-esp32-elf-gcc: missing
   xtensa-esp8266-elf-gcc: missing
                    clang: clang version 11.0.1

Installed compiler libs
-----------------------
     arm-none-eabi-newlib: "2.5.0"
      mips-mti-elf-newlib: missing
        msp430-elf-newlib: missing
    riscv-none-elf-newlib: missing
riscv64-unknown-elf-newlib: missing
  riscv-none-embed-newlib: missing
  xtensa-esp32-elf-newlib: missing
xtensa-esp8266-elf-newlib: missing
                 avr-libc: "2.0.0" ("20150208")

Installed development tools
---------------------------
                   ccache: missing
                    cmake: cmake version 3.19.3
                 cppcheck: missing
                  doxygen: 1.9.1
                      git: git version 2.30.0
                     make: GNU Make 4.3
                  openocd: Open On-Chip Debugger 0.10.0+dev-00226-g1c2e3d41d (2018-01-02-18:28)
                   python: Python 3.9.1
                  python2: Python 2.7.18
                  python3: Python 3.9.1
                   flake8: 3.8.4 (mccabe: 0.6.1, pycodestyle: 2.6.0, pyflakes: 2.2.0) CPython 3.9.1 on
               coccinelle: missing

@miri64 seemed to have the same issue with her Arch Linux

@jia200x
Copy link
Member Author

jia200x commented Jan 28, 2021

so, how to proceed? It would be nice to solve this one before announcing RC3

@aabadie
Copy link
Contributor

aabadie commented Jan 28, 2021

how to proceed? It would be nice to solve this one before announcing RC3

As written in the README:

This test is here to verify the test tools integration with your board and test setup.

It verify the assumptions required for testing on the board behaviour through make term.

(Almost) All tests are using pyterm and if we change pyterm for socat then it means that we don't test the same terminal tools that is used by those tests. I don't like the fix proposed by this PR.
You are running python 3.9 on Arch, which is different from the version available in Ubuntu 20.10 (3.8). So it might be a problem in pyterm with your Python version. If it's very important for you, better fix the real problem than providing a quick workaround fix (that might live longer than you think).

@miri64
Copy link
Member

miri64 commented Jan 29, 2021

No, I tested with python 3.8 on Arch as well. It seems to be a problem when using a terminal emulator in a DE or the like. I can't reproduce the issue when using a raw terminal without GDM (in my case) started. I digged deep enough to figure out, that those ANSI chars are coming from the logging module somehow, which is used by pyterm for printing.

I agree that switching to socat effectively masks a bug in our test infrastructure on certain systems. @jia200x should we rather open an issue mark it as known in the release?

@miri64 miri64 added the Discussion: contested The item of discussion was contested label Jan 29, 2021
@aabadie
Copy link
Contributor

aabadie commented Jan 29, 2021

a terminal emulator in a DE or the like. I can't reproduce the issue when using a raw terminal without GDM (in my case) started

Not sure what you mean with "terminal emulator in a DE", do you mean tmux ? I tested on Ubuntu from terminator and gnome-terminal and no issue.

@miri64
Copy link
Member

miri64 commented Jan 29, 2021

Not sure what you mean with "terminal emulator in a DE", do you mean tmux ? I tested on Ubuntu from terminator and gnome-terminal and no issue.

I mean stuff like gnome-terminal ;-)

@jia200x
Copy link
Member Author

jia200x commented Jan 29, 2021

I tried python3.7 and connecting to my laptop via SSH. Same problem :/
I removed all traces of logs and printed lines directly with sys.stdout.write and print. I still see these ANSI characters.

It works in other Ubuntu machines though.
I will document it in an issue

@jia200x
Copy link
Member Author

jia200x commented Jan 29, 2021

Here's an issue that documents this bug:
#15888

Therefore, I think we can close this one.

@jia200x jia200x closed this Jan 29, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: tests Area: tests and testing framework Discussion: contested The item of discussion was contested Process: needs backport Integration Process: The PR is required to be backported to a release or feature branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants