| Message ID | 20240506111359.224579-1-laura.nao@collabora.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | Modify the watchdog selftest for execution with kselftest runner | expand |
Hi Shuah, On 5/6/24 13:13, Laura Nao wrote: > The watchdog selftest script supports various parameters for testing > different IOCTLs. The watchdog ping functionality is validated by > starting > a loop where the watchdog device is periodically pet, which can only > be > stopped by the user interrupting the script. > > This results in a timeout when running this test using the kselftest > runner > with no non-oneshot parameters (or no parameters at all): > > TAP version 13 > 1..1 > # timeout set to 45 > # selftests: watchdog: watchdog-test > # Watchdog Ticking Away! > # .............................................# > not ok 1 selftests: watchdog: watchdog-test # TIMEOUT 45 seconds > > To address this issue, the first patch in this series limits the loop > to 5 > iterations by default and adds support for a new '-c' option to > customize > the number of pings as required. > > The second patch conforms the test output to the KTAP format. > Gentle ping - any thoughts on this series? It would simplify running the watchdog kselftest in CI environments by leveraging the runner. Thanks! Laura
On 6/6/24 03:57, Laura Nao wrote: > Hi Shuah, > > On 5/6/24 13:13, Laura Nao wrote: >> The watchdog selftest script supports various parameters for testing >> different IOCTLs. The watchdog ping functionality is validated by >> starting >> a loop where the watchdog device is periodically pet, which can only >> be >> stopped by the user interrupting the script. >> >> This results in a timeout when running this test using the kselftest >> runner >> with no non-oneshot parameters (or no parameters at all): Sorry for the delay on this. This test isn't include in the default kselftest run? How are you running this? >> >> TAP version 13 >> 1..1 >> # timeout set to 45 >> # selftests: watchdog: watchdog-test >> # Watchdog Ticking Away! >> # .............................................# >> not ok 1 selftests: watchdog: watchdog-test # TIMEOUT 45 seconds >> >> To address this issue, the first patch in this series limits the loop >> to 5 >> iterations by default and adds support for a new '-c' option to >> customize >> the number of pings as required. >> >> The second patch conforms the test output to the KTAP format. >> > > Gentle ping - any thoughts on this series? It would simplify running the > watchdog kselftest in CI environments by leveraging the runner. > This test isn't intended to be included in the default run. It requires loading a watchdog driver first. Do you load the driver from the runner? thanks, -- Shuah
Hi Shuah, On 6/7/24 01:03, Shuah Khan wrote: > On 6/6/24 03:57, Laura Nao wrote: >> Hi Shuah, >> >> On 5/6/24 13:13, Laura Nao wrote: >>> The watchdog selftest script supports various parameters for testing >>> different IOCTLs. The watchdog ping functionality is validated by >>> starting >>> a loop where the watchdog device is periodically pet, which can only >>> be >>> stopped by the user interrupting the script. >>> >>> This results in a timeout when running this test using the kselftest >>> runner >>> with no non-oneshot parameters (or no parameters at all): > > > Sorry for the delay on this. > > This test isn't include in the default kselftest run? How are you > running this? > The goal of this series is to enable the test to be run using the kselftest runner individually, not as part of the default run. So for example w/out args: make -C tools/testing/selftests TARGETS=watchdog run_tests or with args: KSELFTEST_WATCHDOG_TEST_ARGS='-b -d -e -s -t 12 -T 3 -n 7 -N -L' make -C tools/testing/selftests TARGETS=watchdog run_tests >>> >>> TAP version 13 >>> 1..1 >>> # timeout set to 45 >>> # selftests: watchdog: watchdog-test >>> # Watchdog Ticking Away! >>> # .............................................# >>> not ok 1 selftests: watchdog: watchdog-test # TIMEOUT 45 seconds >>> >>> To address this issue, the first patch in this series limits the >>> loop >>> to 5 >>> iterations by default and adds support for a new '-c' option to >>> customize >>> the number of pings as required. >>> >>> The second patch conforms the test output to the KTAP format. >>> >> >> Gentle ping - any thoughts on this series? It would simplify running >> the >> watchdog kselftest in CI environments by leveraging the runner. >> > > This test isn't intended to be included in the default run. It > requires > loading a watchdog driver first. Do you load the driver from the > runner? > I get this test requires watchdog drivers to be loaded (which in this case can't be added to a config fragment that goes with the selftest, as they are platform-specific) and therefore cannot be included in the default run. However, having ktap output support and limiting the ping loop would allow the test to be run individually in the same way as other selftests (so through the kselftest runner). Naturally, driver dependencies must be met for the test to run and produce valid results. From my understanding the runner itself cannot ensure this, so in this case it would be up to the user or CI to enable/load the appropriate drivers before running the test. If these dependencies are not satisfied, the test could just exit with a skip code. Does this make sense to you? or is the kselftest runner intended to be used to run exclusively a subset of tests in the selftests directory (i.e. the ones that don't have platform-specific driver requirements)? Thanks, Laura
On 6/7/24 03:53, Laura Nao wrote: > Hi Shuah, > > On 6/7/24 01:03, Shuah Khan wrote: >> On 6/6/24 03:57, Laura Nao wrote: >>> Hi Shuah, >>> >>> On 5/6/24 13:13, Laura Nao wrote: >>>> The watchdog selftest script supports various parameters for testing >>>> different IOCTLs. The watchdog ping functionality is validated by >>>> starting >>>> a loop where the watchdog device is periodically pet, which can only >>>> be >>>> stopped by the user interrupting the script. >>>> >>>> This results in a timeout when running this test using the kselftest >>>> runner >>>> with no non-oneshot parameters (or no parameters at all): >> >> >> Sorry for the delay on this. >> >> This test isn't include in the default kselftest run? How are you >> running this? >> > > The goal of this series is to enable the test to be run using the > kselftest runner individually, not as part of the default run. So for > example w/out args: > > make -C tools/testing/selftests TARGETS=watchdog run_tests > > or with args: > > KSELFTEST_WATCHDOG_TEST_ARGS='-b -d -e -s -t 12 -T 3 -n 7 -N -L' make -C > tools/testing/selftests TARGETS=watchdog run_tests > >>>> >>>> TAP version 13 >>>> 1..1 >>>> # timeout set to 45 >>>> # selftests: watchdog: watchdog-test >>>> # Watchdog Ticking Away! >>>> # .............................................# >>>> not ok 1 selftests: watchdog: watchdog-test # TIMEOUT 45 seconds >>>> >>>> To address this issue, the first patch in this series limits the >>>> loop >>>> to 5 >>>> iterations by default and adds support for a new '-c' option to >>>> customize >>>> the number of pings as required. >>>> >>>> The second patch conforms the test output to the KTAP format. >>>> >>> >>> Gentle ping - any thoughts on this series? It would simplify running >>> the >>> watchdog kselftest in CI environments by leveraging the runner. >>> >> >> This test isn't intended to be included in the default run. It >> requires >> loading a watchdog driver first. Do you load the driver from the >> runner? >> > > I get this test requires watchdog drivers to be loaded (which in this > case can't be added to a config fragment that goes with the selftest, as > they are platform-specific) and therefore cannot be included in the > default run. However, having ktap output support and limiting the ping > loop would allow the test to be run individually in the same way as > other selftests (so through the kselftest runner). > > Naturally, driver dependencies must be met for the test to run and > produce valid results. From my understanding the runner itself cannot > ensure this, so in this case it would be up to the user or CI to > enable/load the appropriate drivers before running the test. > If these dependencies are not satisfied, the test could just exit > with a skip code. > > Does this make sense to you? or is the kselftest runner intended to be > used to run exclusively a subset of tests in the selftests directory > (i.e. the ones that don't have platform-specific driver requirements)? > There are several tests that aren't included in the default run because they have dependencies and potentially damaging to the running system. These tests are not included for a reason. The first step would to be ensure writing shell script to load and unload the watchdog module and then pass in existing parameters such as the oneshot to run the test. There is no need to add a new parameter yet. Also there is no need to convert this to ktap yet. thanks, -- Shuah
Hi Shuah, On 6/7/24 23:07, Shuah Khan wrote: > On 6/7/24 03:53, Laura Nao wrote: >> Hi Shuah, >> >> On 6/7/24 01:03, Shuah Khan wrote: >>> On 6/6/24 03:57, Laura Nao wrote: >>>> Hi Shuah, >>>> >>>> On 5/6/24 13:13, Laura Nao wrote: >>>>> The watchdog selftest script supports various parameters for testing >>>>> different IOCTLs. The watchdog ping functionality is validated by >>>>> starting >>>>> a loop where the watchdog device is periodically pet, which can only >>>>> be >>>>> stopped by the user interrupting the script. >>>>> >>>>> This results in a timeout when running this test using the kselftest >>>>> runner >>>>> with no non-oneshot parameters (or no parameters at all): >>> >>> >>> Sorry for the delay on this. >>> >>> This test isn't include in the default kselftest run? How are you >>> running this? >>> >> >> The goal of this series is to enable the test to be run using the >> kselftest runner individually, not as part of the default run. So for >> example w/out args: >> >> make -C tools/testing/selftests TARGETS=watchdog run_tests >> >> or with args: >> >> KSELFTEST_WATCHDOG_TEST_ARGS='-b -d -e -s -t 12 -T 3 -n 7 -N -L' make -C >> tools/testing/selftests TARGETS=watchdog run_tests >>>>> >>>>> TAP version 13 >>>>> 1..1 >>>>> # timeout set to 45 >>>>> # selftests: watchdog: watchdog-test >>>>> # Watchdog Ticking Away! >>>>> # .............................................# >>>>> not ok 1 selftests: watchdog: watchdog-test # TIMEOUT 45 seconds >>>>> >>>>> To address this issue, the first patch in this series limits the >>>>> loop >>>>> to 5 >>>>> iterations by default and adds support for a new '-c' option to >>>>> customize >>>>> the number of pings as required. >>>>> >>>>> The second patch conforms the test output to the KTAP format. >>>>> >>>> >>>> Gentle ping - any thoughts on this series? It would simplify running >>>> the >>>> watchdog kselftest in CI environments by leveraging the runner. >>>> >>> >>> This test isn't intended to be included in the default run. It >>> requires >>> loading a watchdog driver first. Do you load the driver from the >>> runner? >>> >> >> I get this test requires watchdog drivers to be loaded (which in this >> case can't be added to a config fragment that goes with the selftest, as >> they are platform-specific) and therefore cannot be included in the >> default run. However, having ktap output support and limiting the ping >> loop would allow the test to be run individually in the same way as >> other selftests (so through the kselftest runner). >> >> Naturally, driver dependencies must be met for the test to run and >> produce valid results. From my understanding the runner itself cannot >> ensure this, so in this case it would be up to the user or CI to >> enable/load the appropriate drivers before running the test. >> If these dependencies are not satisfied, the test could just exit >> with a skip code. >> >> Does this make sense to you? or is the kselftest runner intended to be >> used to run exclusively a subset of tests in the selftests directory >> (i.e. the ones that don't have platform-specific driver requirements)? >> > > There are several tests that aren't included in the default run because > they have dependencies and potentially damaging to the running system. > These tests are not included for a reason. > > The first step would to be ensure writing shell script to load and > unload the watchdog module and then pass in existing parameters such > as the oneshot to run the test. > > There is no need to add a new parameter yet. Also there is no need to > convert this to ktap yet. > To clarify, I understand that this test is not suitable for the default run, and I do not intend to change that. The patch series is meant to make the test usable in a non-interactive environment, such as a CI, when explicitly enabled and with the required modules already loaded. Thanks, Laura
On 6/18/24 07:40, Laura Nao wrote: > Hi Shuah, > > On 6/7/24 23:07, Shuah Khan wrote: >> On 6/7/24 03:53, Laura Nao wrote: >>> Hi Shuah, >>> >>> On 6/7/24 01:03, Shuah Khan wrote: >>>> On 6/6/24 03:57, Laura Nao wrote: >>>>> Hi Shuah, >>>>> >>>>> On 5/6/24 13:13, Laura Nao wrote: >>>>>> The watchdog selftest script supports various parameters for testing >>>>>> different IOCTLs. The watchdog ping functionality is validated by >>>>>> starting >>>>>> a loop where the watchdog device is periodically pet, which can only >>>>>> be >>>>>> stopped by the user interrupting the script. >>>>>> >>>>>> This results in a timeout when running this test using the kselftest >>>>>> runner >>>>>> with no non-oneshot parameters (or no parameters at all): >>>> >>>> >>>> Sorry for the delay on this. >>>> >>>> This test isn't include in the default kselftest run? How are you >>>> running this? >>>> >>> >>> The goal of this series is to enable the test to be run using the >>> kselftest runner individually, not as part of the default run. So for >>> example w/out args: >>> >>> make -C tools/testing/selftests TARGETS=watchdog run_tests >>> >>> or with args: >>> >>> KSELFTEST_WATCHDOG_TEST_ARGS='-b -d -e -s -t 12 -T 3 -n 7 -N -L' make -C >>> tools/testing/selftests TARGETS=watchdog run_tests >>>>>> >>>>>> TAP version 13 >>>>>> 1..1 >>>>>> # timeout set to 45 >>>>>> # selftests: watchdog: watchdog-test >>>>>> # Watchdog Ticking Away! >>>>>> # .............................................# >>>>>> not ok 1 selftests: watchdog: watchdog-test # TIMEOUT 45 seconds >>>>>> >>>>>> To address this issue, the first patch in this series limits the >>>>>> loop >>>>>> to 5 >>>>>> iterations by default and adds support for a new '-c' option to >>>>>> customize >>>>>> the number of pings as required. >>>>>> >>>>>> The second patch conforms the test output to the KTAP format. >>>>>> >>>>> >>>>> Gentle ping - any thoughts on this series? It would simplify running >>>>> the >>>>> watchdog kselftest in CI environments by leveraging the runner. >>>>> >>>> >>>> This test isn't intended to be included in the default run. It >>>> requires >>>> loading a watchdog driver first. Do you load the driver from the >>>> runner? >>>> >>> >>> I get this test requires watchdog drivers to be loaded (which in this >>> case can't be added to a config fragment that goes with the selftest, as >>> they are platform-specific) and therefore cannot be included in the >>> default run. However, having ktap output support and limiting the ping >>> loop would allow the test to be run individually in the same way as >>> other selftests (so through the kselftest runner). >>> >>> Naturally, driver dependencies must be met for the test to run and >>> produce valid results. From my understanding the runner itself cannot >>> ensure this, so in this case it would be up to the user or CI to >>> enable/load the appropriate drivers before running the test. >>> If these dependencies are not satisfied, the test could just exit >>> with a skip code. >>> >>> Does this make sense to you? or is the kselftest runner intended to be >>> used to run exclusively a subset of tests in the selftests directory >>> (i.e. the ones that don't have platform-specific driver requirements)? >>> >> >> There are several tests that aren't included in the default run because >> they have dependencies and potentially damaging to the running system. >> These tests are not included for a reason. >> >> The first step would to be ensure writing shell script to load and >> unload the watchdog module and then pass in existing parameters such >> as the oneshot to run the test. >> >> There is no need to add a new parameter yet. Also there is no need to >> convert this to ktap yet. >> > > To clarify, I understand that this test is not suitable for the default > run, and I do not intend to change that. The patch series is meant to > make the test usable in a non-interactive environment, such as a CI, > when explicitly enabled and with the required modules already loaded. > The test can be with one shot timer - how is this test currently not usable and how are your changes making it usable? thanks, -- Shuah
On 6/21/24 23:08, Shuah Khan wrote: > On 6/18/24 07:40, Laura Nao wrote: >> Hi Shuah, >> >> On 6/7/24 23:07, Shuah Khan wrote: >>> On 6/7/24 03:53, Laura Nao wrote: >>>> Hi Shuah, >>>> >>>> On 6/7/24 01:03, Shuah Khan wrote: >>>>> On 6/6/24 03:57, Laura Nao wrote: >>>>>> Hi Shuah, >>>>>> >>>>>> On 5/6/24 13:13, Laura Nao wrote: >>>>>>> The watchdog selftest script supports various parameters for >>>>>>> testing >>>>>>> different IOCTLs. The watchdog ping functionality is validated >>>>>>> by >>>>>>> starting >>>>>>> a loop where the watchdog device is periodically pet, which can >>>>>>> only >>>>>>> be >>>>>>> stopped by the user interrupting the script. >>>>>>> >>>>>>> This results in a timeout when running this test using the >>>>>>> kselftest >>>>>>> runner >>>>>>> with no non-oneshot parameters (or no parameters at all): >>>>> >>>>> >>>>> Sorry for the delay on this. >>>>> >>>>> This test isn't include in the default kselftest run? How are you >>>>> running this? >>>>> >>>> >>>> The goal of this series is to enable the test to be run using the >>>> kselftest runner individually, not as part of the default run. So >>>> for >>>> example w/out args: >>>> >>>> make -C tools/testing/selftests TARGETS=watchdog run_tests >>>> >>>> or with args: >>>> >>>> KSELFTEST_WATCHDOG_TEST_ARGS='-b -d -e -s -t 12 -T 3 -n 7 -N -L' >>>> make -C >>>> tools/testing/selftests TARGETS=watchdog run_tests >>>>>>> >>>>>>> TAP version 13 >>>>>>> 1..1 >>>>>>> # timeout set to 45 >>>>>>> # selftests: watchdog: watchdog-test >>>>>>> # Watchdog Ticking Away! >>>>>>> # .............................................# >>>>>>> not ok 1 selftests: watchdog: watchdog-test # TIMEOUT 45 >>>>>>> seconds >>>>>>> >>>>>>> To address this issue, the first patch in this series limits the >>>>>>> loop >>>>>>> to 5 >>>>>>> iterations by default and adds support for a new '-c' option to >>>>>>> customize >>>>>>> the number of pings as required. >>>>>>> >>>>>>> The second patch conforms the test output to the KTAP format. >>>>>>> >>>>>> >>>>>> Gentle ping - any thoughts on this series? It would simplify >>>>>> running >>>>>> the >>>>>> watchdog kselftest in CI environments by leveraging the runner. >>>>>> >>>>> >>>>> This test isn't intended to be included in the default run. It >>>>> requires >>>>> loading a watchdog driver first. Do you load the driver from the >>>>> runner? >>>>> >>>> >>>> I get this test requires watchdog drivers to be loaded (which in >>>> this >>>> case can't be added to a config fragment that goes with the >>>> selftest, as >>>> they are platform-specific) and therefore cannot be included in the >>>> default run. However, having ktap output support and limiting the >>>> ping >>>> loop would allow the test to be run individually in the same way as >>>> other selftests (so through the kselftest runner). >>>> >>>> Naturally, driver dependencies must be met for the test to run and >>>> produce valid results. From my understanding the runner itself >>>> cannot >>>> ensure this, so in this case it would be up to the user or CI to >>>> enable/load the appropriate drivers before running the test. >>>> If these dependencies are not satisfied, the test could just exit >>>> with a skip code. >>>> >>>> Does this make sense to you? or is the kselftest runner intended to >>>> be >>>> used to run exclusively a subset of tests in the selftests >>>> directory >>>> (i.e. the ones that don't have platform-specific driver >>>> requirements)? >>>> >>> >>> There are several tests that aren't included in the default run >>> because >>> they have dependencies and potentially damaging to the running >>> system. >>> These tests are not included for a reason. >>> >>> The first step would to be ensure writing shell script to load and >>> unload the watchdog module and then pass in existing parameters such >>> as the oneshot to run the test. >>> >>> There is no need to add a new parameter yet. Also there is no need >>> to >>> convert this to ktap yet. >>> >> >> To clarify, I understand that this test is not suitable for the >> default >> run, and I do not intend to change that. The patch series is meant to >> make the test usable in a non-interactive environment, such as a CI, >> when explicitly enabled and with the required modules already loaded. >> > > The test can be with one shot timer - how is this test currently not > usable and how are your changes making it usable? > In a non-interactive environment, the current test can only be run through the oneshot parameters, making it impossible to test the WDIOC_KEEPALIVE ioctl (since the ping loop is only interrupted by SIGINT). So the first patch enables testing of the WDIOC_KEEPALIVE ioctl in such an environment, by making the ping loop finite. The second patch allows the reuse of the ktap result parser used for other kselftests, eliminating the need for a custom parser for this test. Thanks, Laura
The watchdog selftest script supports various parameters for testing different IOCTLs. The watchdog ping functionality is validated by starting a loop where the watchdog device is periodically pet, which can only be stopped by the user interrupting the script. This results in a timeout when running this test using the kselftest runner with no non-oneshot parameters (or no parameters at all): TAP version 13 1..1 # timeout set to 45 # selftests: watchdog: watchdog-test # Watchdog Ticking Away! # .............................................# not ok 1 selftests: watchdog: watchdog-test # TIMEOUT 45 seconds To address this issue, the first patch in this series limits the loop to 5 iterations by default and adds support for a new '-c' option to customize the number of pings as required. The second patch conforms the test output to the KTAP format. Laura Nao (2): selftests/watchdog: limit ping loop and allow configuring the number of pings selftests/watchdog: convert the test output to KTAP format .../selftests/watchdog/watchdog-test.c | 166 +++++++++++------- 1 file changed, 101 insertions(+), 65 deletions(-)