Quoting Claudio Imbrenda (2022-12-02 13:56:47) > On Fri, 2 Dec 2022 13:48:17 +0100 > Thomas Huth <thuth@xxxxxxxxxx> wrote: > > > On 02/12/2022 12.56, Janosch Frank wrote: > > > On 12/2/22 12:32, Thomas Huth wrote: > > >> On 02/12/2022 11.44, Nico Boehr wrote: > > >>> Quoting Thomas Huth (2022-12-02 10:09:03) > > >>>> On 02/12/2022 10.03, Janosch Frank wrote: > > >>>>> On 12/1/22 09:46, Nico Boehr wrote: > > >>>>>> Upcoming changes will add a test which is very similar to the existing > > >>>>>> skey migration test. To reduce code duplication, move the common > > >>>>>> functions to a library which can be re-used by both tests. > > >>>>>> > > >>>>> > > >>>>> NACK > > >>>>> > > >>>>> We're not putting test specific code into the library. > > >>>> > > >>>> Do we need a new file (in the third patch) for the new test at all, or > > >>>> could > > >>>> the new test simply be added to s390x/migration-skey.c instead? > > >>> > > >>> Mh, not quite. One test wants to change storage keys *before* migrating, > > >>> the other *while* migrating. Since we can only migrate once, it is not > > >>> obvious to me how we could do that in one run. > > >>> > > >>> Speaking of one run, what we could do is add a command line argument > > >>> which decides which test to run and then call the same test with > > >>> different arguments in unittests.cfg. > > >> > > >> Yes, that's what I had in mind - use a command line argument to select the > > >> test ... should be OK as long as both variants are listed in unittests.cfg, > > >> shouldn't it? > > >> > > >> Thomas > > > > > > @Thomas @Claudio: > > > I see two possible solutions if we want a "testlib" at some point (which for > > > the record I don't have anything against): > > > > > > Putting the files into lib/s390x/testlib/* which will then be part of our > > > normal lib. > > > That's a minimal effort solution. It still puts those files into lib/* but > > > they are at least contained in a directory. > > > > > > Putting the files into s390x/testlib/* and creating a proper new lib. > > > Which means we'd need a few more lines of makefile changes. > > > > Though this is an excellent topic for a Friday afternoon bikeshedding ... I > > don't mind much either way. I maybe just got a small preference to not touch > > the main lib/ folder here. I guess you could even call it > > s390x/migration-skey-common.c and leave the lib logic out of the game ... > > but I don't really mind. Up to you to decide ;-) > > > > I really like the idea of having only one test and use a commandline > parameter to decide which variant to run > > this way no need to put things in external files OK, I also like this suggestion, then I'll refactor the tests.
