Francois Gouget fgouget@codeweavers.com writes:
diff --git a/winetest/dissect b/winetest/dissect index d9404fe2..8ef31b01 100755 --- a/winetest/dissect +++ b/winetest/dissect @@ -306,7 +306,7 @@ while (<IN>) { $dll = undef; mydie "too many tests skipped by user request\n" if ++$user_skips > $maxuserskips; }
- } elsif (/^(.*$unit.*: (\d+) tests executed ((\d+) marked as todo, (\d+) failures?), (\d+) skipped.)\r?$/) {
- } elsif (/^($unit:(?:0x[0-9a-f]+)? (\d+) tests executed ((\d+) marked as todo, (\d+) failures?), (\d+) skipped.)\r?$/) {
This may be bikeshedding, but I feel that it would be nicer to use the %04x format we use for pid in other places.
On Wed, 8 Feb 2017, Alexandre Julliard wrote: [...]
- } elsif (/^(.*$unit.*: (\d+) tests executed ((\d+) marked as todo, (\d+) failures?), (\d+) skipped.)\r?$/) {
- } elsif (/^($unit:(?:0x[0-9a-f]+)? (\d+) tests executed ((\d+) marked as todo, (\d+) failures?), (\d+) skipped.)\r?$/) {
This may be bikeshedding, but I feel that it would be nicer to use the %04x format we use for pid in other places.
Hmmm....
dlls/kernel32/console.c:1465: TRACE("Started wineconsole pid=%08x tid=%08x\n", dlls/kernel32/tests/console.c:1063: ok(list[0] == pid, "Expected %d, got %d\n", pid, list[0]); dlls/kernel32/tests/debugger.c:372: ok(dbg_blackbox.pid == crash_blackbox.pid, "the child and debugged pids don't match: %d != %d\n", crash_blackbox.pid, dbg_blackbox.pid); dlls/kernel32/tests/debugger.c:374: ok(dbg_blackbox.attach_rc, "DebugActiveProcess(%d) failed err=%d\n", dbg_blackbox.pid, dbg_blackbox.attach_err); dlls/kernel32/tests/debugger.c:376: ok(dbg_blackbox.detach_rc, "DebugActiveProcessStop(%d) failed err=%d\n", dbg_blackbox.pid, dbg_blackbox.detach_err); dlls/kernel32/tests/debugger.c:619: sprintf(cmd, "%s%s%08x "%s"", argv[0], arguments, pid, blackbox_file); dlls/msi/tests/automation.c:1627: ok(hr == S_OK, "SummaryInfo_Property (pid %d) failed, hresult 0x%08x\n", entry->property, hr); dlls/ole32/tests/marshal.c:3491: ok(info->dwServerPid == GetCurrentProcessId(), "dwServerPid was 0x%x instead of 0x%x\n", info->dwServerPid, GetCurrentProcessId()); dlls/ole32/tests/marshal.c:3502: ok(new_info->server_pid == GetCurrentProcessId(), "server pid was 0x%x instead of 0x%x\n", new_info->server_pid, dlls/ole32/tests/marshal.c:3531: ok(info->dwServerPid == GetCurrentProcessId(), "dwServerPid was 0x%x instead of 0x%x\n", info->dwServerPid, GetCurrentProcessId()); dlls/ole32/tests/marshal.c:3539: ok(new_info->server_pid == GetCurrentProcessId(), "server pid was 0x%x instead of 0x%x\n", new_info->server_pid, dlls/ole32/tests/marshal.c:3568: ok(info->dwServerPid == GetCurrentProcessId(), "dwServerPid was 0x%x instead of 0x%x\n", info->dwServerPid, GetCurrentProcessId()); dlls/ole32/tests/marshal.c:3578: ok(new_info->server_pid == GetCurrentProcessId(), "server pid was 0x%x instead of 0x%x\n", new_info->server_pid, dlls/ole32/tests/marshal.c:3603: ok(info->dwServerPid == GetCurrentProcessId(), "dwServerPid was 0x%x instead of 0x%x\n", info->dwServerPid, GetCurrentProcessId()); dlls/ole32/tests/marshal.c:3611: ok(new_info->server_pid == GetCurrentProcessId(), "server pid was 0x%x instead of 0x%x\n", new_info->server_pid, dlls/ole32/tests/marshal.c:3637: ok(info->dwServerPid == GetCurrentProcessId(), "dwServerPid was 0x%x instead of 0x%x\n", info->dwServerPid, GetCurrentProcessId()); dlls/ole32/tests/marshal.c:3645: ok(new_info->server_pid == GetCurrentProcessId(), "server pid was 0x%x instead of 0x%x\n", new_info->server_pid, dlls/iphlpapi/tests/iphlpapi.c:1614: trace( "%u: local %s:%u remote %s:%u state %u pid %u\n", i, ...
Oh! Those two:
dlls/kernel32/process.c:1044: TRACE( "started wineboot pid %04x tid %04x\n", pi.dwProcessId, pi.dwThreadId ); dlls/kernel32/process.c:2449: TRACE( "started process pid %04x tid %04x\n", info->dwProcessId, info->dwThreadId );
Sure no problem. More janitorial work?
So that would give us something like this:
drive.c:268: Test failed: This is a test failure drive:0012 42 tests executed (0 marked as todo, 1 failures), 0 skipped. drive:0010 0 tests executed (0 marked as todo, 0 failures), 0 skipped. kernel32:drive:0010 done (0)
The start of the 'Test failed' and 'tests executed' lines looks a bit similar but even so they should be easy to distinguish.
Do you have an opinion on the colons in the 'tests executed' lines? We used to have
drive: 0 tests executed...
And currently I've implemented
drive:0010 0 tests executed...
rather than
drive:0010: 0 tests executed...
The latter would actually be backward compatible with the TestBot's parsing (but not with test.winehq's dissect).
The 'Test failed' lines have the extra colon but the 'done' ones never did. While we're changing things maybe they should all have the trailing colon for consistency? Not sure if there's any impact?
Francois Gouget fgouget@codeweavers.com writes:
Oh! Those two:
dlls/kernel32/process.c:1044: TRACE( "started wineboot pid %04x tid %04x\n", pi.dwProcessId, pi.dwThreadId ); dlls/kernel32/process.c:2449: TRACE( "started process pid %04x tid %04x\n", info->dwProcessId, info->dwThreadId );
Sure no problem. More janitorial work?
I was thinking more of the server traces and the +pid debug channel. I don't think we need to cleanup the ok() traces, we should never see them anyway.
Do you have an opinion on the colons in the 'tests executed' lines? We used to have
drive: 0 tests executed...
And currently I've implemented
drive:0010 0 tests executed...
rather than
drive:0010: 0 tests executed...
More bikeshedding, but 0010:drive: would be more similar to the Wine traces, so maybe a little less confusing.
-
Alexandre Julliard -
Francois Gouget