shell32/tests: add tests for the parser of SHELLEXECUTEINFO.lpFile [try 4]

[Juan Lang]

Hi Ilya,

I think you're still confused by the prohibition against using
GetVersion.  Your changes follow the letter of that edict, but not the
spirit:

+    HMODULE hkernel32 = GetModuleHandle("kernel32");
+    BOOL isreallyXP_2k3_plus = NULL != GetProcAddress(hkernel32,
"AttachConsole");
+    BOOL isreally98_2k_plus = NULL != GetProcAddress(hkernel32,
"GetLongPathNameA");

That is, you're looking for other markers of particular versions of
Windows.  That's not the reason for avoiding the use of GetVersion.
We avoid GetVersion because version checks in general are fragile.
Sometimes a behavior will change due to the installation of a service
pack, for example, and the tests will begin to fail if they expect a
particular behavior on a particular version of Windows.

What we prefer is for tests to check the behavior of the function
being tested on every version of Windows where that's possible.  In
some cases, it's impossible, because the function doesn't exist on old
versions of Windows, or because the function crashes when called in a
particular way.  In such cases, we try to avoid crashes, and use
win_skip to indicate that certain tests couldn't be performed.

In other cases, some older versions behave in ways that aren't really
sensible.  We mark these as broken(), indicating that it's acceptable
for Windows to behave this way, but not for Wine to do so.  I think
this is what you want, e.g. here:

+    /* quoted lpFile not works on real win95 and nt4 */
+    if (isreally98_2k_plus) {
+        /* if quoted, existing "drawback_file.noassoc" not prevents
finding "drawback_file.noassoc foo.shlexec" on wine */
+        testfile = "\"%s\\drawback_file.noassoc foo.shlexec\"";
+        expected = 33; sprintf(fileA, testfile, tmpdir);
+        rc=shell_execute(NULL, fileA, NULL, NULL);
+        TEST_LPFILE_PARSED_OK();
+    } else {
+        win_skip("ShellExecute('\"command\"', ...) only works on
98/2k or newer\n");
+    }

What you want to do instead is to call shell_execute in any case
(unless doing so would crash.)  You'll want to check rc on succeeding
versions of Windows/Wine with ok, and mark it as broken elsewhere.
E.g.,
  rc = shell_execute(NULL, fileA, NULL, NULL);
  ok((rc==expected || rc>32 && expected>32) || broken(rc == whatever
busted versions of Windows return), "expected ...");

Also, those TEST_LPFILE_PARSED_OK macros are ugly, and hard to fix if
one place has a certain behavior, while another has different
behavior.  Just use ok directly wherever you're using the macro.

Thanks,
--Juan