Message ID | 20231016184408.879977-1-pvorel@suse.cz |
---|---|
Headers | show |
Series | Remove scsi testsuite + various testscripts | expand |
All explanations for the removals sound reasonable. so LGTM.
Reviewed-by: Marius Kittler <mkittler@suse.de>
Hi! > cleanup of 2 old scsi testsuites and some of legacy testscripts. > IMHO the testsuites are not worth of fixing. Agree. Reviewed-by: Cyril Hrubis <chrubis@suse.cz>
On Tue, Oct 17, 2023 at 2:44 AM Petr Vorel <pvorel@suse.cz> wrote: > Hi, > > cleanup of 2 old scsi testsuites and some of legacy testscripts. > IMHO the testsuites are not worth of fixing. > Thanks for the cleanup work! Reviewed-by: Li Wang <liwang@redhat.com>
Hello, Petr Vorel <pvorel@suse.cz> writes: > Hi, > > cleanup of 2 old scsi testsuites and some of legacy testscripts. > IMHO the testsuites are not worth of fixing. Very good. My only suggestion is to leave a tombstone in the documentation (or github issues) any time we delete something big and the thing it was supposed to test still should be tested. Something like "There was a testsuite called X, it appeared to do Y, but we had to remove it because of Z". It could be useful when answering questions about test feasability and for SEO. Reviwed-by: Richard Palethorpe <rpalethorpe@suse.com> > > Kind regards, > Petr > > Petr Vorel (7): > doc: Remove ltp-run-files.txt > fs: Remove scsi/ltpfs testsuite > fs: Remove scsi/ltpscsi testsuite > testscripts: Remove ltpdmmapper.sh > testscripts: Remove ltp-scsi_debug.sh > testscripts: Remove sysfs.sh > testcases: Remove autofs{1,4}.sh scripts > > doc/Test-Writing-Guidelines.asciidoc | 12 + > doc/ltp-run-files.txt | 96 - > testcases/kernel/fs/scsi/ltpfs/Ltpfs.h | 71 - > testcases/kernel/fs/scsi/ltpfs/LtpfsCmds.c | 315 - > testcases/kernel/fs/scsi/ltpfs/Makefile | 30 - > testcases/kernel/fs/scsi/ltpfs/ltpfs.part1 | 4 - > testcases/kernel/fs/scsi/ltpfs/ltpfs.part2 | 3 - > testcases/kernel/fs/scsi/ltpfs/ltpfs.part3 | 3 - > testcases/kernel/fs/scsi/ltpfs/ltpfs.part4 | 3 - > testcases/kernel/fs/scsi/ltpfs/ltpfs.part5 | 3 - > testcases/kernel/fs/scsi/ltpfs/ltpfs.part6 | 3 - > testcases/kernel/fs/scsi/ltpfs/ltpfs.part7 | 3 - > testcases/kernel/fs/scsi/ltpfs/ltpfsio.sh | 154 - > testcases/kernel/fs/scsi/ltpfs/main.c | 647 -- > testcases/kernel/fs/scsi/ltpscsi/Makefile | 47 - > testcases/kernel/fs/scsi/ltpscsi/llseek.c | 123 - > testcases/kernel/fs/scsi/ltpscsi/llseek.h | 10 - > testcases/kernel/fs/scsi/ltpscsi/ltpfsscsi.sh | 111 - > testcases/kernel/fs/scsi/ltpscsi/scsimain.c | 7651 ----------------- > testcases/kernel/fs/scsi/ltpscsi/sg_err.c | 1379 --- > testcases/kernel/fs/scsi/ltpscsi/sg_err.h | 162 - > testcases/kernel/fs/scsi/ltpscsi/sg_include.h | 42 - > testscripts/autofs1.sh | 273 - > testscripts/autofs4.sh | 259 - > testscripts/ltp-scsi_debug.sh | 218 - > testscripts/ltpdmmapper.sh | 204 - > testscripts/sysfs.sh | 121 - > 27 files changed, 12 insertions(+), 11935 deletions(-) > delete mode 100644 doc/ltp-run-files.txt > delete mode 100644 testcases/kernel/fs/scsi/ltpfs/Ltpfs.h > delete mode 100644 testcases/kernel/fs/scsi/ltpfs/LtpfsCmds.c > delete mode 100644 testcases/kernel/fs/scsi/ltpfs/Makefile > delete mode 100644 testcases/kernel/fs/scsi/ltpfs/ltpfs.part1 > delete mode 100644 testcases/kernel/fs/scsi/ltpfs/ltpfs.part2 > delete mode 100644 testcases/kernel/fs/scsi/ltpfs/ltpfs.part3 > delete mode 100644 testcases/kernel/fs/scsi/ltpfs/ltpfs.part4 > delete mode 100644 testcases/kernel/fs/scsi/ltpfs/ltpfs.part5 > delete mode 100644 testcases/kernel/fs/scsi/ltpfs/ltpfs.part6 > delete mode 100644 testcases/kernel/fs/scsi/ltpfs/ltpfs.part7 > delete mode 100755 testcases/kernel/fs/scsi/ltpfs/ltpfsio.sh > delete mode 100644 testcases/kernel/fs/scsi/ltpfs/main.c > delete mode 100644 testcases/kernel/fs/scsi/ltpscsi/Makefile > delete mode 100644 testcases/kernel/fs/scsi/ltpscsi/llseek.c > delete mode 100644 testcases/kernel/fs/scsi/ltpscsi/llseek.h > delete mode 100755 testcases/kernel/fs/scsi/ltpscsi/ltpfsscsi.sh > delete mode 100644 testcases/kernel/fs/scsi/ltpscsi/scsimain.c > delete mode 100644 testcases/kernel/fs/scsi/ltpscsi/sg_err.c > delete mode 100644 testcases/kernel/fs/scsi/ltpscsi/sg_err.h > delete mode 100644 testcases/kernel/fs/scsi/ltpscsi/sg_include.h > delete mode 100755 testscripts/autofs1.sh > delete mode 100755 testscripts/autofs4.sh > delete mode 100755 testscripts/ltp-scsi_debug.sh > delete mode 100755 testscripts/ltpdmmapper.sh > delete mode 100755 testscripts/sysfs.sh
Hi Richie, all, thanks for your review, merged. > Hello, > Petr Vorel <pvorel@suse.cz> writes: > > Hi, > > cleanup of 2 old scsi testsuites and some of legacy testscripts. > > IMHO the testsuites are not worth of fixing. > Very good. My only suggestion is to leave a tombstone in the > documentation (or github issues) any time we delete something big and > the thing it was supposed to test still should be tested. I understand the need of missing coverage, preferably over github issue (we document missing coverage over github issues already). I wonder what should be noted in this case. These test scripts attempted to test: * autofs (run other tests on autofs actually) * BIO (we still have testcases/kernel/device-drivers/tbio/) * sysfs (but we have at least some sysfs tests) * SCSI (I suppose these will be better handled elsewhere - xfstests have scsi_debug file, mention scsi in some generic and xfs specific tests) * device mapper tests (there is something ruby based: https://github.com/jthornber/device-mapper-test-suite from Joe Thornber from Red Hat) > Something like "There was a testsuite called X, it appeared to do > Y, but we had to remove it because of Z". > It could be useful when answering questions about test feasability and > for SEO. I'm not sure if this 20 years old code deserves this description (but feel free to write it if you think so). But identifying missing coverage is of course important. Maybe we could have a special wiki page which would link missing coverage issues [1], but also highlight the most important ones (big subsystem missing) and also point out what we consider being tested elsewhere or what would be hard to test with LTP thus should be tested elsewhere. Kind regards, Petr [1] https://github.com/linux-test-project/ltp/labels/missing%20coverage
Hello, Petr Vorel <pvorel@suse.cz> writes: > Hi Richie, all, > > thanks for your review, merged. > >> Hello, > >> Petr Vorel <pvorel@suse.cz> writes: > >> > Hi, > >> > cleanup of 2 old scsi testsuites and some of legacy testscripts. >> > IMHO the testsuites are not worth of fixing. > >> Very good. My only suggestion is to leave a tombstone in the >> documentation (or github issues) any time we delete something big and >> the thing it was supposed to test still should be tested. > > I understand the need of missing coverage, preferably over github issue > (we document missing coverage over github issues already). > > I wonder what should be noted in this case. These test scripts attempted to test: > * autofs (run other tests on autofs actually) > * BIO (we still have testcases/kernel/device-drivers/tbio/) > * sysfs (but we have at least some sysfs tests) > * SCSI (I suppose these will be better handled elsewhere - xfstests have > scsi_debug file, mention scsi in some generic and xfs specific tests) > * device mapper tests (there is something ruby based: > https://github.com/jthornber/device-mapper-test-suite from Joe Thornber from Red > Hat) > >> Something like "There was a testsuite called X, it appeared to do >> Y, but we had to remove it because of Z". > >> It could be useful when answering questions about test feasability and >> for SEO. > > I'm not sure if this 20 years old code deserves this description (but feel free > to write it if you think so). But identifying missing coverage is of course > important. Maybe we could have a special wiki page which would link missing > coverage issues [1], but also highlight the most important ones (big subsystem > missing) and also point out what we consider being tested elsewhere or what > would be hard to test with LTP thus should be tested elsewhere. I suppose if the test suite did not do anything interesting, then it's not useful. > > Kind regards, > Petr > > [1] https://github.com/linux-test-project/ltp/labels/missing%20coverage