Re: [Qemu-devel] [PATCH v4 3/3] tests/microbit-test: Check nRF51 UART functionality

[Thomas Huth]

On 2019-01-17 17:16, Julia Suvorova via Qemu-devel wrote:
> Some functional tests for:
>     Basic reception/transmittion
>     Suspending
>     INTEN* registers
> 
> Signed-off-by: Julia Suvorova <address@hidden>
> ---
>  tests/microbit-test.c | 84 +++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 84 insertions(+)
> 
> diff --git a/tests/microbit-test.c b/tests/microbit-test.c
> index afeb6b082a..3da6d9529f 100644
> --- a/tests/microbit-test.c
> +++ b/tests/microbit-test.c
> @@ -19,10 +19,93 @@
>  #include "libqtest.h"
>  
>  #include "hw/arm/nrf51.h"
> +#include "hw/char/nrf51_uart.h"
>  #include "hw/gpio/nrf51_gpio.h"
>  #include "hw/timer/nrf51_timer.h"
>  #include "hw/i2c/microbit_i2c.h"
>  
> +#include <sys/socket.h>
> +#include <sys/un.h>
> +
> +static bool uart_wait_for_event(QTestState *qts, uint32_t event_addr)
> +{
> +    int i;
> +
> +    for (i = 0; i < 1000; i++) {
> +        if (qtest_readl(qts, event_addr) == 1) {
> +            qtest_writel(qts, event_addr, 0x00);
> +            return true;
> +        }
> +        g_usleep(10000);
> +    }

So this times out after 10 seconds? ... this is likely plenty on a
normal host, but we run the tests on overloaded CI systems sometimes,
where 10 seconds are not that much...

I'd suggest to replace the condition in the for-loop with "i < 30000" or
even "i < 60000", just to be sure.

> +    return false;
> +}
> +
> +static void uart_rw_to_rxd(QTestState *qts, int sock_fd, const char *in,
> +                           char *out)
> +{
> +    int i, in_len = strlen(in);
> +
> +    g_assert(write(sock_fd, in, in_len) == in_len);

Sorry for being pedantic, but I'd really prefer to write it without
side-effects in g_assert() please. (same for the other occurrences in
this patch)

 Thomas