One of the hoped for TestBot improvements is to get rid of TestLauncher and replace it with WineTest on the theory that they perform the same role.
Indeed both add lines to the test report to identify the start and end of each test, and they check that the dlls needed for the test are present (which is buggy in TestLauncher).
However WineTest only deals with Wine's test executables and this leads to some hardcoded assumptions: 1) It expects to extract the test executables from its own resources. 2) It expects that the executables will be called something like <dllname>_test.exe and that it means it should check if <dllname> is usable. 3) It expects that when given the 'list' parameter the executable will spit out the list of test units to run. 4) Then it expects proceeds to run the test executables with the command line that it chose.
However the TestBot also lets Wine developers upload any executable they like and lets them specify the command line they want. This breaks all the above assumptions.
So, to replace TestLauncher, WineTest would need to be modified so it can run an arbitrary executable, not from its own resources, using an arbitrary command line. And it also means merging TestLauncher's code that dives into the executable's import table to figure out which dlls are needed, with WineTest's code that has support for activation contexts (the lack of which is what's causing the bug in TestLauncher if I understand correctly).
Finally one would also want to be able to build WineTest without any builtin test executable to get something small enough since it would get uploaded for each task. And that would still leave all the GUI as dead weight. As far as I can tell a WineTest with no builtin test still takes around 87 KB whereas TestLauncher can get under 12 KB.
Even in the patch case there are some issues though these have essentially been known from the start: * Currently it's only possible to build WineTest with all the tests, resulting in a huge executables. Fixing this requires either bypassing the build system to build WineTest, or tweaking the build system so one can specify which executables to embed.
* Even if we embed only one test executable we usually only want to run one test unit. Fortunately that's already possible by specifying it on the command line.
* However, even in the case of a patch the Wine developer can specify an arbitrary command line. This is probably not very command but do we want to drop it?
* Finally WineTest systematically outputs platform information. That would make every report a bit larger and would force developers to skip over it most of the time. But it may also be useful sometimes.
Given the above I feel like it would be simpler, cleaner and possibly more maintainable to keep the two separate after all.
Of course sticking with TestLauncher would require some work too but that seems quite tractable: * First not checking it into Git! * Making sure it gets updated automatically. * Fixing the dll checks. * Bringing it in line with the latest changes w.r.t. pids, run tmie data, etc.
On Mon, Apr 3, 2017 at 11:57 AM, André Hentschel nerv@dawncrow.de wrote:
Can't you just fork winetest, throw out everything unwanted, add some missing features and make that the new testlauncher?
Am 02.04.2017 11:30 nachm. schrieb Francois Gouget fgouget@codeweavers.com:
One of the hoped for TestBot improvements is to get rid of TestLauncher and replace it with WineTest on the theory that they perform the same role.
Indeed both add lines to the test report to identify the start and end of each test, and they check that the dlls needed for the test are present (which is buggy in TestLauncher).
However WineTest only deals with Wine's test executables and this leads to some hardcoded assumptions:
- It expects to extract the test executables from its own resources.
- It expects that the executables will be called something like <dllname>_test.exe and that it means it should check if <dllname> is usable.
- It expects that when given the 'list' parameter the executable will spit out the list of test units to run.
- Then it expects proceeds to run the test executables with the command line that it chose.
However the TestBot also lets Wine developers upload any executable they like and lets them specify the command line they want. This breaks all the above assumptions.
So, to replace TestLauncher, WineTest would need to be modified so it can run an arbitrary executable, not from its own resources, using an arbitrary command line. And it also means merging TestLauncher's code that dives into the executable's import table to figure out which dlls are needed, with WineTest's code that has support for activation contexts (the lack of which is what's causing the bug in TestLauncher if I understand correctly).
Finally one would also want to be able to build WineTest without any builtin test executable to get something small enough since it would get uploaded for each task. And that would still leave all the GUI as dead weight. As far as I can tell a WineTest with no builtin test still takes around 87 KB whereas TestLauncher can get under 12 KB.
Even in the patch case there are some issues though these have essentially been known from the start:
Currently it's only possible to build WineTest with all the tests, resulting in a huge executables. Fixing this requires either bypassing the build system to build WineTest, or tweaking the build system so one can specify which executables to embed.
Even if we embed only one test executable we usually only want to run one test unit. Fortunately that's already possible by specifying it on the command line.
However, even in the case of a patch the Wine developer can specify an arbitrary command line. This is probably not very command but do we want to drop it?
Finally WineTest systematically outputs platform information. That would make every report a bit larger and would force developers to skip over it most of the time. But it may also be useful sometimes.
Given the above I feel like it would be simpler, cleaner and possibly more maintainable to keep the two separate after all.
Of course sticking with TestLauncher would require some work too but that seems quite tractable:
- First not checking it into Git!
- Making sure it gets updated automatically.
- Fixing the dll checks.
- Bringing it in line with the latest changes w.r.t. pids, run tmie data, etc.
-- Francois Gouget fgouget@codeweavers.com
Or add those features to winetest, so there's one code base, not two..
On Mon, 3 Apr 2017, Austin English wrote: [...]
One of the hoped for TestBot improvements is to get rid of TestLauncher and replace it with WineTest on the theory that they perform the same role.
[...]
Or add those features to winetest, so there's one code base, not two..
That's what replacing TestLauncher with WineTest means. No?
On Tue, Apr 4, 2017 at 10:49 AM, Francois Gouget fgouget@codeweavers.com wrote:
On Mon, 3 Apr 2017, Austin English wrote: [...]
One of the hoped for TestBot improvements is to get rid of TestLauncher and replace it with WineTest on the theory that they perform the same role.
[...]
Or add those features to winetest, so there's one code base, not two..
That's what replacing TestLauncher with WineTest means. No?
-- Francois Gouget fgouget@codeweavers.com
Yes.
On Mon, 3 Apr 2017, André Hentschel wrote:
Can't you just fork winetest, throw out everything unwanted, add some missing features and make that the new testlauncher?
Well, TestLauncher is 461 lines whereas WineTest is 2827 lines.
I don't think there's all that much that needs to be brought over so trimming WineTest may not be much simpler. But moving the dll check / activation context to its own function (or even C file) would make it easier to copy / paste it and keep it in sync later.
-
André Hentschel -
Austin English -
Francois Gouget