Re: [PATCH 3/4] selftests: can: Use fixtures in test_raw_filter

On Tue. 22 Apr. 2025 at 21:04, Felix Maurer fmaurer@redhat.com wrote:

Use fixtures in test_raw_filter instead of generating the test inputs during execution. This should make tests easier to follow and extend.

Signed-off-by: Felix Maurer fmaurer@redhat.com

.../selftests/net/can/test_raw_filter.c | 311 ++++++++++++------ 1 file changed, 211 insertions(+), 100 deletions(-)

diff --git a/tools/testing/selftests/net/can/test_raw_filter.c b/tools/testing/selftests/net/can/test_raw_filter.c index 7414b9aef823..7fe11e020a1c 100644 --- a/tools/testing/selftests/net/can/test_raw_filter.c +++ b/tools/testing/selftests/net/can/test_raw_filter.c @@ -18,43 +18,13 @@

(...)

+TEST_F(can_filters, test_filter) +{

•   fd_set rdfs;
  

•   struct timeval tv;
  

•   struct can_filter rfilter;
  

•   struct can_frame frame;
  

•   int have_rx;
  

•   int rx;
  

•   int rxbits, rxbitval;
  

•   int ret;
  

Can you reduce the scope of the variables? More precisely, can you move those declarations

fd_set rdfs; struct timeval tv;

to the do {} while(); loop and move those declarations

struct can_frame frame; int rxbitval;

to the if (FD_ISSET(self->sock, &rdfs)) block?

•           TH_LOG("testcase %2d filters : can_id = 0x%08X can_mask = 0x%08X",
  

•                  testcase, rfilter.can_id, rfilter.can_mask);
  

•   rfilter.can_id = variant->id;
  

•   rfilter.can_mask = variant->mask;
  

•   setsockopt(self->sock, SOL_CAN_RAW, CAN_RAW_FILTER,
  

•              &rfilter, sizeof(rfilter));
  
  

•           TH_LOG("testcase %2d sending patterns...", testcase);
  

•   TH_LOG("filters: can_id = 0x%08X can_mask = 0x%08X",
  

•           rfilter.can_id, rfilter.can_mask);
  
  

•           ret = send_can_frames(s, testcase);
  

•           ASSERT_EQ(0, ret)
  

•                   TH_LOG("failed to send CAN frames");
  

•   ret = send_can_frames(self->sock, variant->testcase);
  

•   ASSERT_EQ(0, ret)
  

•           TH_LOG("failed to send CAN frames");
  
  

•           have_rx = 1;
  

•           rx = 0;
  

•           rxbits = 0;
  

•   rx = 0;
  

•   rxbits = 0;
  
  

•           while (have_rx) {
  

•   do {
  

•           have_rx = 0;
  

•           FD_ZERO(&rdfs);
  

•           FD_SET(self->sock, &rdfs);
  

•           tv.tv_sec = 0;
  

•           tv.tv_usec = 50000; /* 50ms timeout */
  
  

•                   have_rx = 0;
  

•                   FD_ZERO(&rdfs);
  

•                   FD_SET(s, &rdfs);
  

•                   tv.tv_sec = 0;
  

•                   tv.tv_usec = 50000; /* 50ms timeout */
  

•           ret = select(self->sock + 1, &rdfs, NULL, NULL, &tv);
  

•           ASSERT_LE(0, ret)
  

•                   TH_LOG("failed select for frame %d (%d)", rx, errno);
  
  

•                   ret = select(s+1, &rdfs, NULL, NULL, &tv);
  

•           if (FD_ISSET(self->sock, &rdfs)) {
  

•                   have_rx = 1;
  

•                   ret = read(self->sock, &frame, sizeof(struct can_frame));
                    ASSERT_LE(0, ret)
  

•                           TH_LOG("failed select for frame %d (%d)", rx, errno);
  

•                   if (FD_ISSET(s, &rdfs)) {
  

•                           have_rx = 1;
  

•                           ret = read(s, &frame, sizeof(struct can_frame));
  

^^^^^^^^^^^^^^^^ sizeof(frame)

•                           ASSERT_LE(0, ret)
  

•                                   TH_LOG("failed to read frame %d (%d)", rx, errno);
  

•                           ASSERT_EQ(ID, frame.can_id & CAN_SFF_MASK)
  

•                                   TH_LOG("received wrong can_id");
  

•                           ASSERT_EQ(testcase, frame.data[0])
  

•                                   TH_LOG("received wrong test case");
  

•                           /* test & calc rxbits */
  

•                           rxbitval = 1 << ((frame.can_id & (CAN_EFF_FLAG|CAN_RTR_FLAG|CAN_ERR_FLAG)) >> 28);
  

•                           /* only receive a rxbitval once */
  

•                           ASSERT_NE(rxbitval, rxbits & rxbitval)
  

•                                   TH_LOG("received rxbitval %d twice", rxbitval);
  

•                           rxbits |= rxbitval;
  

•                           rx++;
  

•                           TH_LOG("testcase %2d rx : can_id = 0x%08X rx = %d rxbits = %d",
  

•                                  testcase, frame.can_id, rx, rxbits);
  

•                   }
  

•                           TH_LOG("failed to read frame %d (%d)", rx, errno);
  

•                   ASSERT_EQ(ID, frame.can_id & CAN_SFF_MASK)
  

•                           TH_LOG("received wrong can_id");
  

•                   ASSERT_EQ(variant->testcase, frame.data[0])
  

•                           TH_LOG("received wrong test case");
  

•                   /* test & calc rxbits */
  

•                   rxbitval = 1 << ((frame.can_id & (CAN_EFF_FLAG|CAN_RTR_FLAG|CAN_ERR_FLAG)) >> 28);
  

•                   /* only receive a rxbitval once */
  

•                   ASSERT_NE(rxbitval, rxbits & rxbitval)
  

•                           TH_LOG("received rxbitval %d twice", rxbitval);
  

•                   rxbits |= rxbitval;
  

•                   rx++;
  

•                   TH_LOG("rx: can_id = 0x%08X rx = %d rxbits = %d",
  

•                          frame.can_id, rx, rxbits);
            }
  

•           /* rx timed out -> check the received results */
  

•           ASSERT_EQ(rx_res[testcase], rx)
  

•                   TH_LOG("wrong number of received frames %d", testcase);
  

•           ASSERT_EQ(rxbits_res[testcase], rxbits)
  

•                   TH_LOG("wrong rxbits value in testcase %d", testcase);
  

•           TH_LOG("testcase %2d ok", testcase);
  

•           TH_LOG("---");
  

•   }
  

•   } while (have_rx);
  
  

•   close(s);
  

•   return;
  

•   /* rx timed out -> check the received results */
  

•   ASSERT_EQ(variant->exp_num_rx, rx)
  

•           TH_LOG("wrong number of received frames");
  

•   ASSERT_EQ(variant->exp_rxbits, rxbits)
  

•           TH_LOG("wrong rxbits value");
  

}

Yours sincerely, Vincent Mailhol