tests: improve wait_multiple coverage

PATCH Applied Kent Gibson 2 participants 10 messages Oct 14, 2020 GPIO

lore

Share & tools

lore

Kent Gibson

author

2020-10-14 09:09 UTC #0 lore raw

cover tests: improve wait_multiple coverage

A couple of patches for the v2 branch that improve the coverage of the wait_multiple test case.

The first creates a mismatch between the chip offsets and bulk offsets to highlight the problem with my initial implementation of gpiod_line_event_wait_bulk() for uAPI v2.

The second adds a check on the event.offset field added for uAPI v2.

Kent Gibson (2):
tests: create mismatch between chip and bulk offsets in wait_multiple
tests: add check of event offset to wait_multiple

 tests/tests-event.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Signature · 1 line

2.28.0

Bartosz Golaszewski

2020-10-14 08:29 UTC #1 lore raw

Quoted 14 lines · Kent Gibson

A couple of patches for the v2 branch that improve the coverage of the wait_multiple test case.
The first creates a mismatch between the chip offsets and bulk offsets to highlight the problem with my initial implementation of gpiod_line_event_wait_bulk() for uAPI v2.
The second adds a check on the event.offset field added for uAPI v2.
Kent Gibson (2):
tests: create mismatch between chip and bulk offsets in wait_multiple
tests: add check of event offset to wait_multiple

Quoted 6 lines

 tests/tests-event.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

--
2.28.0

I suppose that, if we don't want to do the backward compatible port for now, these can be ignored?

Bartosz

Kent Gibson

author

2020-10-14 08:37 UTC #2 lore raw

Quoted 18 lines · Bartosz Golaszewski

Quoted 17 lines · Kent Gibson
A couple of patches for the v2 branch that improve the coverage of the wait_multiple test case.
The first creates a mismatch between the chip offsets and bulk offsets to highlight the problem with my initial implementation of gpiod_line_event_wait_bulk() for uAPI v2.
The second adds a check on the event.offset field added for uAPI v2.
Kent Gibson (2):
tests: create mismatch between chip and bulk offsets in wait_multiple
tests: add check of event offset to wait_multiple
 tests/tests-event.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Quoted 7 lines

Quoted 3 lines
--
2.28.0
I suppose that, if we don't want to do the backward compatible port for now, these can be ignored?

It wouldn't hurt to have them either way - in my book it never hurts to increase test coverage.

Cheers,
Kent.

Bartosz Golaszewski

2020-10-14 08:40 UTC #3 lore raw

Quoted 28 lines · Kent Gibson

Quoted 18 lines · Bartosz Golaszewski
Quoted 17 lines · Kent Gibson
A couple of patches for the v2 branch that improve the coverage of the wait_multiple test case.
The first creates a mismatch between the chip offsets and bulk offsets to highlight the problem with my initial implementation of gpiod_line_event_wait_bulk() for uAPI v2.
The second adds a check on the event.offset field added for uAPI v2.
Kent Gibson (2):
tests: create mismatch between chip and bulk offsets in wait_multiple
tests: add check of event offset to wait_multiple
 tests/tests-event.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
Quoted 7 lines
Quoted 3 lines
--
2.28.0
I suppose that, if we don't want to do the backward compatible port for now, these can be ignored?

Quoted 5 lines

It wouldn't hurt to have them either way - in my book it never hurts to increase test coverage.
Cheers,
Kent.

Ok I thought they only apply to your v2 port but I see they'll work in master too.

Bartosz

Kent Gibson

author

2020-10-14 08:46 UTC #4 lore raw

Quoted 29 lines · Bartosz Golaszewski

Quoted 28 lines · Kent Gibson
Quoted 25 lines · Bartosz Golaszewski
Quoted 14 lines · Kent Gibson
A couple of patches for the v2 branch that improve the coverage of the wait_multiple test case.
The first creates a mismatch between the chip offsets and bulk offsets to highlight the problem with my initial implementation of gpiod_line_event_wait_bulk() for uAPI v2.
The second adds a check on the event.offset field added for uAPI v2.
Kent Gibson (2):
tests: create mismatch between chip and bulk offsets in wait_multiple
tests: add check of event offset to wait_multiple
Quoted 6 lines
 tests/tests-event.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
--
2.28.0
I suppose that, if we don't want to do the backward compatible port for now, these can be ignored?

Quoted 9 lines

Quoted 5 lines
It wouldn't hurt to have them either way - in my book it never hurts to increase test coverage.
Cheers,
Kent.
Ok I thought they only apply to your v2 port but I see they'll work in master too.

Yes and No, i.e. the first is general, the second is requires the offset field in the event, and so only works for a uAPI v2 port.

Kent.

Kent Gibson

author

2020-10-14 09:09 UTC #5 lore raw

PATCH 1/2 tests: create mismatch between chip and bulk offsets in wait_multiple

In reply to Kent Gibson

Create a mismatch between the chip offsets and the bulk offsets to ensure the implementation is not dependent on them matching.

Signed-off-by Kent Gibson <[email protected]>

---
 tests/tests-event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tests/tests-event.c +1 -1

index 6066e45..a43f865 100644
@@ -546,7 +546,7 @@ GPIOD_TEST_CASE(wait_multiple, 0, { 8 })
 	gpiod_test_return_if_failed();
 
 	gpiod_line_bulk_init(&bulk);
-	for (i = 0; i < 8; i++) {
+	for (i = 1; i < 8; i++) {
 		line = gpiod_chip_get_line(chip, i);
 		g_assert_nonnull(line);
 		gpiod_test_return_if_failed();

Signature · 1 line

2.28.0

Bartosz Golaszewski

Applied

2020-10-14 08:46 UTC #6 lore raw

Re: [libgpiod][PATCH 1/2] tests: create mismatch between chip and bulk offsets in wait_multiple

Quoted 23 lines · Kent Gibson

Create a mismatch between the chip offsets and the bulk offsets to ensure the implementation is not dependent on them matching.

Signed-off-by Kent Gibson <[email protected]>

---
 tests/tests-event.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

tests/tests-event.c +1 -2

index 6066e45..a43f865 100644
@@ -546,7 +546,7 @@ GPIOD_TEST_CASE(wait_multiple, 0, { 8 })
        gpiod_test_return_if_failed();
 
        gpiod_line_bulk_init(&bulk);
-       for (i = 0; i < 8; i++) {
+       for (i = 1; i < 8; i++) {
                line = gpiod_chip_get_line(chip, i);
                g_assert_nonnull(line);
                gpiod_test_return_if_failed();
--

Quoted 2 lines

2.28.0

Applied, thanks!

Bartosz

Kent Gibson

author

2020-10-14 09:09 UTC #7 lore raw

PATCH 2/2 tests: add check of event offset to wait_multiple

In reply to Kent Gibson

The offset field is added for uAPI v2, so extend the test to check that the value returned is correct.

Signed-off-by Kent Gibson <[email protected]>

---
 tests/tests-event.c | 1 +
 1 file changed, 1 insertion(+)

tests/tests-event.c +1 -0

index a43f865..0e5effe 100644
@@ -570,6 +570,7 @@ GPIOD_TEST_CASE(wait_multiple, 0, { 8 })
 	ret = gpiod_line_event_read(line, &ev);
 	g_assert_cmpint(ret, ==, 0);
 	g_assert_cmpint(ev.event_type, ==, GPIOD_LINE_EVENT_RISING_EDGE);
+	g_assert_cmpint(ev.offset, ==, 4);
 }
 
 GPIOD_TEST_CASE(get_fd_when_values_requested, 0, { 8 })

Signature · 1 line

2.28.0

Bartosz Golaszewski

2020-10-14 08:51 UTC #8 lore raw

Re: [libgpiod][PATCH 2/2] tests: add check of event offset to wait_multiple

Quoted 22 lines · Kent Gibson

The offset field is added for uAPI v2, so extend the test to check that the value returned is correct.

Signed-off-by Kent Gibson <[email protected]>

---
 tests/tests-event.c | 1 +
 1 file changed, 1 insertion(+)

tests/tests-event.c +1 -1

index a43f865..0e5effe 100644
@@ -570,6 +570,7 @@ GPIOD_TEST_CASE(wait_multiple, 0, { 8 })
        ret = gpiod_line_event_read(line, &ev);
        g_assert_cmpint(ret, ==, 0);
        g_assert_cmpint(ev.event_type, ==, GPIOD_LINE_EVENT_RISING_EDGE);
+       g_assert_cmpint(ev.offset, ==, 4);
 }
 
 GPIOD_TEST_CASE(get_fd_when_values_requested, 0, { 8 })
--

Quoted 2 lines

2.28.0

This one relies on your v2 port but since it breaks tests, I can't apply it to master and same for this patch. We'll have to revisit this once we figure out the new API for the library.

Bartosz

Bartosz Golaszewski

2020-10-14 13:56 UTC #9 lore raw

Re: [libgpiod][PATCH 2/2] tests: add check of event offset to wait_multiple

In reply to Kent Gibson

Quoted 22 lines · Kent Gibson

The offset field is added for uAPI v2, so extend the test to check that the value returned is correct.

Signed-off-by Kent Gibson <[email protected]>

---
 tests/tests-event.c | 1 +
 1 file changed, 1 insertion(+)

tests/tests-event.c +1 -1

index a43f865..0e5effe 100644
@@ -570,6 +570,7 @@ GPIOD_TEST_CASE(wait_multiple, 0, { 8 })
        ret = gpiod_line_event_read(line, &ev);
        g_assert_cmpint(ret, ==, 0);
        g_assert_cmpint(ev.event_type, ==, GPIOD_LINE_EVENT_RISING_EDGE);
+       g_assert_cmpint(ev.offset, ==, 4);
 }
 
 GPIOD_TEST_CASE(get_fd_when_values_requested, 0, { 8 })
--

Quoted 2 lines

2.28.0

Now applied to branch for-linux-v5.10-rc1. Thanks!

Bartosz