Re: [libgpiod][PATCH 3/7] tests: event: extend test coverage for reading multiple line events at once

[Kent Gibson <warthog618@xxxxxxxxx>]

On Wed, Dec 18, 2019 at 03:24:45PM +0100, Bartosz Golaszewski wrote:
> From: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
> 
> Add test cases for new helpers allowing users to read multiple events
> at once.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@xxxxxxxxxxxx>
> ---
>  tests/tests-event.c | 83 +++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 83 insertions(+)
> 
> diff --git a/tests/tests-event.c b/tests/tests-event.c
> index d425d1a..1f4a2eb 100644
> --- a/tests/tests-event.c
> +++ b/tests/tests-event.c
> @@ -552,3 +552,86 @@ GPIOD_TEST_CASE(invalid_fd, 0, { 8 })
>  	g_assert_cmpint(ret, ==, -1);
>  	g_assert_cmpint(errno, ==, EINVAL);
>  }
> +
> +GPIOD_TEST_CASE(read_multiple_events, 0, { 8 })
> +{
> +	g_autoptr(gpiod_chip_struct) chip = NULL;
> +	struct gpiod_line_event events[3];
> +	struct timespec ts = { 1, 0 };
> +	struct gpiod_line *line;
> +	gint ret;
> +
> +	chip = gpiod_chip_open(gpiod_test_chip_path(0));
> +	g_assert_nonnull(chip);
> +	gpiod_test_return_if_failed();
> +
> +	line = gpiod_chip_get_line(chip, 4);
> +	g_assert_nonnull(line);
> +	gpiod_test_return_if_failed();
> +
> +	ret = gpiod_line_request_both_edges_events(line, GPIOD_TEST_CONSUMER);
> +	g_assert_cmpint(ret, ==, 0);
> +
> +	gpiod_test_chip_set_pull(0, 4, 1);
> +	usleep(10000);
> +	gpiod_test_chip_set_pull(0, 4, 0);
> +	usleep(10000);
> +	gpiod_test_chip_set_pull(0, 4, 1);
> +	usleep(10000);
> +

I assume the sleep is to wait for the event to be generated from the
call gpiod_test_chip_set_pull, which is not guaranteed to occur before
the call returns, otherwise you can toggle the line too fast and may
miss events.
Arbitrary sleeps in code, including tests, should be avoided as they
are brittle and obsure what you are actually waiting for.
An alternative in this case is to add a second event fd and wait for
the event to arrive there before continuing.

Kent.

> +	ret = gpiod_line_event_wait(line, &ts);
> +	g_assert_cmpint(ret, ==, 1);
> +
> +	ret = gpiod_line_event_read_multiple(line, events, 3);
> +	g_assert_cmpint(ret, ==, 3);
> +
> +	g_assert_cmpint(events[0].event_type, ==,
> +			GPIOD_LINE_EVENT_RISING_EDGE);
> +	g_assert_cmpint(events[1].event_type, ==,
> +			GPIOD_LINE_EVENT_FALLING_EDGE);
> +	g_assert_cmpint(events[2].event_type, ==,
> +			GPIOD_LINE_EVENT_RISING_EDGE);
> +}
> +
> +GPIOD_TEST_CASE(read_multiple_events_fd, 0, { 8 })
> +{
> +	g_autoptr(gpiod_chip_struct) chip = NULL;
> +	struct gpiod_line_event events[3];
> +	struct timespec ts = { 1, 0 };
> +	struct gpiod_line *line;
> +	gint ret, fd;
> +
> +	chip = gpiod_chip_open(gpiod_test_chip_path(0));
> +	g_assert_nonnull(chip);
> +	gpiod_test_return_if_failed();
> +
> +	line = gpiod_chip_get_line(chip, 4);
> +	g_assert_nonnull(line);
> +	gpiod_test_return_if_failed();
> +
> +	ret = gpiod_line_request_both_edges_events(line, GPIOD_TEST_CONSUMER);
> +	g_assert_cmpint(ret, ==, 0);
> +
> +	gpiod_test_chip_set_pull(0, 4, 1);
> +	usleep(10000);
> +	gpiod_test_chip_set_pull(0, 4, 0);
> +	usleep(10000);
> +	gpiod_test_chip_set_pull(0, 4, 1);
> +	usleep(10000);
> +
> +	ret = gpiod_line_event_wait(line, &ts);
> +	g_assert_cmpint(ret, ==, 1);
> +
> +	fd = gpiod_line_event_get_fd(line);
> +	g_assert_cmpint(fd, >=, 0);
> +
> +	ret = gpiod_line_event_read_fd_multiple(fd, events, 3);
> +	g_assert_cmpint(ret, ==, 3);
> +
> +	g_assert_cmpint(events[0].event_type, ==,
> +			GPIOD_LINE_EVENT_RISING_EDGE);
> +	g_assert_cmpint(events[1].event_type, ==,
> +			GPIOD_LINE_EVENT_FALLING_EDGE);
> +	g_assert_cmpint(events[2].event_type, ==,
> +			GPIOD_LINE_EVENT_RISING_EDGE);
> +}
> -- 
> 2.23.0
>