[PATCH] winetest: Add GUI to display results of individual tests

Zebediah Figura z.figura12 at gmail.com
Sat Dec 15 13:41:15 CST 2018


Sorry for the late review; I'm a little busy at the moment.

On 12/11/2018 07:52 PM, Isira Seneviratne wrote:
 > From 5902ce10274900a04a740aa9a11ab91070d6e7b0 Mon Sep 17 00:00:00 2001
 > From: Isira Seneviratne <isirasen96 at gmail.com>
 > Date: Wed, 5 Dec 2018 19:32:31 +0530
 > Subject: [PATCH] winetest: Add GUI to display results of individual tests
 >
 > This patch adds an interface that displays the result of each test
 > as soon as it completes, with the status value if the test failed.
 > The dialog appears when the tests start running.
 >
 > Signed-off-by: Isira Seneviratne <isirasen96 at gmail.com>
 > ---
 >  programs/winetest/main.c      | 78 +++++++++++++++++++++++++++++++++++
 >  programs/winetest/resource.h  |  4 ++
 >  programs/winetest/winetest.rc | 10 +++++
 >  3 files changed, 92 insertions(+)
 >
 > diff --git a/programs/winetest/main.c b/programs/winetest/main.c
 > index 56044095f3..9a5a069c26 100644
 > --- a/programs/winetest/main.c
 > +++ b/programs/winetest/main.c
 > @@ -4,6 +4,7 @@
 >   * Copyright 2003, 2004 Jakob Eriksson   (for Solid Form Sweden AB)
 >   * Copyright 2003 Dimitrie O. Paun
 >   * Copyright 2003 Ferenc Wagner
 > + * Copyright 2018 Isira Seneviratne
 >   *
 >   * This library is free software; you can redistribute it and/or
 >   * modify it under the terms of the GNU Lesser General Public
 > @@ -34,6 +35,7 @@
 >  #include <windows.h>
 >  #include <winternl.h>
 >  #include <mshtml.h>
 > +#include <commctrl.h>
 >
 >  #include "winetest.h"
 >  #include "resource.h"
 > @@ -85,6 +87,11 @@ static void (WINAPI *pReleaseActCtx)(HANDLE);
 >  /* To store the current PATH setting (related to .NET only provided 
dlls) */
 >  static char *curpath;
 >
 > +/* To store the test results separately */
 > +static HWND hResDlg;
 > +static HWND hListView;

Please try to avoid Hungarian notation in new code.

 > +static int i = 0;

'i' shouldn't be global.

 > +
 >  /* check if test is being filtered out */
 >  static BOOL test_filtered_out( LPCSTR module, LPCSTR testname )
 >  {
 > @@ -789,13 +796,45 @@ run_test (struct wine_test* test, const char* 
subtest, HANDLE out_file, const ch
 >      else
 >      {
 >          int status;
 > +        char result[20];
 >          DWORD pid, start = GetTickCount();
 > +        LVITEMA lvItem;

Similarly here.

 > +
 >          char *cmd = strmake (NULL, "%s %s", test->exename, subtest);
 >          report (R_STEP, "Running: %s:%s", test->name, subtest);
 >          xprintf ("%s:%s start %s -\n", test->name, subtest, file);
 >          status = run_ex (cmd, out_file, tempdir, 120000, &pid);
 >          heap_free (cmd);
 >          xprintf ("%s:%s:%04x done (%d) in %ds\n", test->name, 
subtest, pid, status, (GetTickCount()-start)/1000);
 > +
 > +        /* Stores the results separately */
 > +        if (status)
 > +            strcpy(result, "Success");
 > +        else
 > +            sprintf(result, "Failure (%d)", status);
 > +

This is duplicated below.

 > +        /* Adds a single row to the list view control (test name, 
subtest name and result). */
 > +        lvItem.mask = LVIF_TEXT;
 > +        lvItem.state = 0;
 > +        lvItem.stateMask = 0;
 > +        lvItem.iSubItem = 0;
 > +        lvItem.iItem = i;
 > +        lvItem.pszText = test->name;
 > +        ListView_InsertItemA(hListView, &lvItem);
 > +
 > +        lvItem.iSubItem = 1;
 > +        lvItem.pszText = (char *) subtest;
 > +        ListView_SetItemA(hListView, &lvItem);
 > +
 > +        lvItem.iSubItem = 2;
 > +        if (status == 0)
 > +            strcpy(result, "Success");
 > +        else
 > +            sprintf(result, "Failure (%d)", status);

'status' here is the return value of MsgWaitForMultipleObjects(), which 
isn't very interesting to report. I'd suggest instead giving a count of 
the number of failures, similar to what test.winehq.org shows (e.g. 
[1].) Note that to do this you'll need to parse failures from the test 
output, similar to how TestLauncher does it.

[1] 
http://test.winehq.org/data/4397d9497608a3df45a5c519a37f5dcde5cc2301/index_Linux.html

 > +        lvItem.pszText = result;
 > +        ListView_SetItemA(hListView, &lvItem);
 > +        i++;
 > +
 >          if (status) failures++;
 >      }
 >      if (failures) report (R_STATUS, "Running tests - %u failures", 
failures);
 > @@ -996,6 +1035,24 @@ extract_test_proc (HMODULE hModule, LPCSTR 
lpszType, LPSTR lpszName, LONG_PTR lP
 >      return TRUE;
 >  }
 >
 > +INT_PTR CALLBACK
 > +ResultDlgProc(HWND hwnd, UINT msg, WPARAM wParam, LPARAM lParam)
 > +{
 > +    switch(msg)
 > +    {
 > +        case WM_INITDIALOG:
 > +            return TRUE;
 > +
 > +        case WM_CLOSE:
 > +            EndDialog(hwnd, 0);
 > +            break;
 > +
 > +        default:
 > +            return FALSE;
 > +    }
 > +    return TRUE;
 > +}
 > +
 >  static char *
 >  run_tests (char *logname, char *outdir)
 >  {
 > @@ -1006,6 +1063,9 @@ run_tests (char *logname, char *outdir)
 >      char tmppath[MAX_PATH], tempdir[MAX_PATH+4];
 >      DWORD needed;
 >      HMODULE kernel32;
 > +    /* Variables needed to display the results of the tests */
 > +    LVCOLUMNA lvCol;
 > +    HINSTANCE hInstance = GetModuleHandleA(NULL);
 >
 >      /* Get the current PATH only once */
 >      needed = GetEnvironmentVariableA("PATH", NULL, 0);
 > @@ -1124,6 +1184,24 @@ run_tests (char *logname, char *outdir)
 >      if (nr_native_dlls)
 >          report( R_WARNING, "Some dlls are configured as native, you 
won't be able to submit results." );
 >
 > +    /* Initializes the result dialog window and the list view 
control it contains */
 > +    hResDlg = CreateDialogParamA(hInstance, 
MAKEINTRESOURCEA(IDD_RES_DLG), 0, ResultDlgProc, 0);
 > +
 > +    hListView = GetDlgItem(hResDlg, IDD_RES_LIST);
 > +
 > +    lvCol.mask = LVCF_FMT | LVCF_WIDTH | LVCF_TEXT | LVCF_SUBITEM;
 > +    lvCol.cx = 100;
 > +    lvCol.fmt = LVCFMT_LEFT;
 > +
 > +    lvCol.pszText = (char *) "Test";
 > +    ListView_InsertColumnA(hListView, 0, &lvCol);
 > +
 > +    lvCol.pszText = (char *) "Subtest";
 > +    ListView_InsertColumnA(hListView, 1, &lvCol);
 > +
 > +    lvCol.pszText = (char *) "Result";
 > +    ListView_InsertColumnA(hListView, 2, &lvCol);
 > +
 >      report (R_STATUS, "Running tests");
 >      report (R_PROGRESS, 1, nr_of_tests);
 >      for (i = 0; i < nr_of_files; i++) {
 > diff --git a/programs/winetest/resource.h b/programs/winetest/resource.h
 > index be71274f58..46525afad5 100644
 > --- a/programs/winetest/resource.h
 > +++ b/programs/winetest/resource.h
 > @@ -2,6 +2,7 @@
 >   * Resource definitions
 >   *
 >   * Copyright 2004 Ferenc Wagner
 > + * Copyright 2018 Isira Seneviratne
 >   *
 >   * This library is free software; you can redistribute it and/or
 >   * modify it under the terms of the GNU Lesser General Public
 > @@ -25,6 +26,9 @@
 >  #define IDD_TAG    102
 >  #define IDD_EMAIL  103
 >
 > +#define IDD_RES_DLG 110
 > +#define IDD_RES_LIST 111
 > +
 >  #define IDC_STATIC -1
 >
 >  #define IDC_ST0 1000
 > diff --git a/programs/winetest/winetest.rc 
b/programs/winetest/winetest.rc
 > index a3898b6ce0..673ee57843 100644
 > --- a/programs/winetest/winetest.rc
 > +++ b/programs/winetest/winetest.rc
 > @@ -2,6 +2,7 @@
 >   * Winetest resources
 >   *
 >   * Copyright 2004 Ferenc Wagner
 > + * Copyright 2018 Isira Seneviratne
 >   *
 >   * This library is free software; you can redistribute it and/or
 >   * modify it under the terms of the GNU Lesser General Public
 > @@ -88,6 +89,15 @@ BEGIN
 >      DEFPUSHBUTTON "Close", IDCANCEL, 55, 40, 40, 14
 >  END
 >
 > +IDD_RES_DLG DIALOG DISCARDABLE 160, 0, 240, 230
 > +STYLE WS_VISIBLE | WS_POPUP | WS_CAPTION | WS_SYSMENU
 > +CAPTION "Winetest Results"
 > +FONT 8, "MS Shell Dlg"
 > +BEGIN
 > +    CONTROL "", IDD_RES_LIST, "SysListView32", WS_CHILD | WS_VISIBLE 
| WS_BORDER | LVS_REPORT, 10,
 > +    10, 220, 210
 > +END
 > +

Again, I think this would make more sense as part of the existing dialog 
rather than a new one. I'm open to arguments against this, but I'd like 
to hear them first.

 >  /* @makedep: winetest.ico */
 >  IDI_WINE ICON "winetest.ico"
 >
 > --
 > 2.20.0



More information about the wine-devel mailing list