[PATCH v4] programs/winetest: don't create console when non connected to a console

[Jacek Caban]

Hi Eric,

On 4/1/22 13:55, Eric Pouech wrote:
> Le 31/03/2022 à 20:16, Alexandre Julliard a écrit :
>> Francois Gouget<fgouget at codeweavers.com>  writes:
>>
>>> Signed-off-by: Francois Gouget<fgouget at codeweavers.com>
>>>
>>> That's great. I can now reproduce these failures.
>>> The easiest way to do so from a terminal is something like this:
>>>
>>>    nohup ./wine ... 2>&1 | cat
>>>
>>> With this winetest patch user32:input succeeds... sometimes.
>>> I don't think this means the patch is wrong.
>>>
>>> Rather the issue is that user32:input itself 'forks' without passing
>>> DETACHED_PROCESS to CreateProcess(), causing consoles to appear in the
>>> middle of the test which randomly leads to failures.
>> Do these consoles also appear on Windows then?
>>
> basically we should have the following hierarchy of processes (for a 
> testbot invocation)
>
>   testagentd Unix/windows
>    \- TestLauncher (CUI)
>       \- unit test (CUI)
>          \- unit test can recurse upon itself (CUI)
>
> according to 
> https://testbot.winehq.org/JobDetails.pl?Key=111732&f201=exe64.report&f301=task.log#k301
>
> (it's a program to output various information about console (attached 
> / detached, other processes attached to console, user32 information 
> about console, info about std handles...)
>
> the info is written using various ways (prefix gives the way: w> using 
> write(1), W> using WriteFile(STD_OUTPUT_HANDLE), d> wine_dbg_output...)
>
>
> a) when the console is created
>
> under Windows, the console is created when TestLaunched is spawned. 
> Seems logical: first CUI process, testagentd is likely run detached.
>
> in wine env, the console is created at least for the unit test (test 
> as written cannot get information from testlauncher back)
>
> as testagentd (on unix) is likely detached (from unix tty), 
> TestLauncher is then created detached from unix but also windows 
> standpoint ; meaning the the w-console is only created for unit test.
>
> in both cases, the respawn from within the unit test should reuse the 
> same console
>
> b) it then means that the unit test (because it's launched with 
> console creation) does not (currently) honor the std handle 
> inheritance (hence providing different output information)
>
> that would explain the discrepancies on output prefix (windows with 
> wW> as those handles are inherited from TestLauncher), and wine (d> 
> using unix fd 1)
>
> (under the assumption that in both wine and windows the redirection 
> for running a tests are done in a similar way)
>

Nice, that explains the situation.


> we exchanged with Jacek what to do when wine foo is started as initial 
> wine process and foo is a CUI
>
> on one hand, it would be a bad idea to let CreateProcess create a 
> console. cases when script is run in a headless context, or detached 
> from any tty...
>
> on the other hand (as this case shows), not creating a console is not 
> always the best situation
>

Yeah, that solution is not perfect, but it seems to me that there is no 
solution that could satisfy both cases out of the box.


> from these considerations, I would recommand that we mimic windows' 
> behavior: when run under wine only, testagend should spawn 
> TestLauncher with wineconsole (we know we can force a user32/wine 
> console creation):
>
> ./wine wineconsole TestLauncher... (instead of ./wine TestLauncher)
>
>
> François: what would be the best way to do it? (tweaking 
> platform_unix/platform_run() seems to be more widely used than just 
> running wine?
>

One other solution that we may consider is to build tests as GUI 
applications and add AttachConsole(ATTACH_PARENT_PROCESS) to main() in 
test.h. That should preserve desired behaviour. We'd probably need an 
opt-out mechanism for console tests, but it should be fine otherwise, 
unless I'm missing something.


Thanks,

Jacek
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.winehq.org/pipermail/wine-devel/attachments/20220401/7e6c829b/attachment.htm>