On 02/11/2010 03:37 PM, Ilya Basin wrote:
Hi Ilya,
As Wine doesn't do the right thing (according to your tests), all failing Wine tests should be marked as such. We have a todo_wine construct for that.
No C++ comments please.
We generally do not want any real version checking in the tests. Tests should decide based on behavior on what platform they are.
Why is this necessary?
Any particular reason for this indentation?
Thanks for review. Not sending to wine-patches this time. New patch is in the bottom. What's better, to attach a generated patch or to use it as a message body?
PV> On 02/11/2010 03:37 PM, Ilya Basin wrote:
PV> Hi Ilya,
PV> As Wine doesn't do the right thing (according to your tests), all PV> failing Wine tests should be marked as such. We have a todo_wine PV> construct for that. added 4 todo_wine's
PV> We generally do not want any real version checking in the tests. Tests PV> should decide based on behavior on what platform they are. added GetProcAddress(hkernel32, "AttachConsole") to detect XP/2k3 removed special case when XP emulates 9x because can't do it without using GetVersion().
PV> Why is this necessary? /* ensure tmpdir is in %TEMP%: GetTempPath() can succeed even if TEMP is undefined */
PV> Any particular reason for this indentation? OK
--- dlls/shell32/tests/shlexec.c | 87 ++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 87 insertions(+), 0 deletions(-)
diff --git a/dlls/shell32/tests/shlexec.c b/dlls/shell32/tests/shlexec.c index c673f0a..d575d22 100644 --- a/dlls/shell32/tests/shlexec.c +++ b/dlls/shell32/tests/shlexec.c @@ -797,6 +797,10 @@ static const char* testfiles[]= "%s\test file.sde", "%s\test file.exe", "%s\test2.exe", + "%s\simple.shlexec", + "%s\drawback_file.noassoc", + "%s\drawback_file.noassoc foo.shlexec", + "%s\drawback_nonexist.noassoc foo.shlexec", NULL };
@@ -852,6 +856,88 @@ static filename_tests_t noquotes_tests[]= {NULL, NULL, 0} };
+static void test_lpFile_parsed(void) +{ + /* basename tmpdir */ + const char* shorttmpdir; + + const char *testfile; + char fileA[MAX_PATH]; + + int rc; + int expected; + + HMODULE hkernel32 = GetModuleHandle("kernel32"); + BOOL isreallyXP2k3plus = NULL != GetProcAddress(hkernel32, "AttachConsole"); + + GetTempPathA(sizeof(fileA), fileA); + shorttmpdir = tmpdir + strlen(fileA); + + /* ensure tmpdir is in %TEMP%: GetTempPath() can succeed even if TEMP is undefined */ + SetEnvironmentVariableA("TEMP", fileA); + + #define TEST_LPFILE_PARSED_OK_() (rc==expected || rc>32 && expected>32) + #define TEST_LPFILE_PARSED_OK() ok(TEST_LPFILE_PARSED_OK_(), "expected %s (%d), got %s (%d), lpFile: %s \n", expected==33 ? "success" : "failure", expected, rc > 32 ? "success" : "failure", rc, fileA) + + /* existing "drawback_file.noassoc" 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); + todo_wine { TEST_LPFILE_PARSED_OK(); } + + /* 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(); + + /* error should be 2, not 31 */ + testfile = ""%s\drawback_file.noassoc" foo.shlexec"; + expected = 2; sprintf(fileA, testfile, tmpdir); + rc=shell_execute(NULL, fileA, NULL, NULL); + TEST_LPFILE_PARSED_OK(); + + /* ""command"" not works on wine (and real win9x and w2k) */ + if (isreallyXP2k3plus) { + testfile = """%s\simple.shlexec"""; + expected = 33; sprintf(fileA, testfile, tmpdir); + rc=shell_execute(NULL, fileA, NULL, NULL); + todo_wine { TEST_LPFILE_PARSED_OK(); } + } else { + win_skip("ShellExecute('""command""', ...) only works on XP/2k3 or newer\n"); + } + + /* nonexisting "drawback_nonexist.noassoc" not prevents finding "drawback_nonexist.noassoc foo.shlexec" on wine */ + testfile = "%s\drawback_nonexist.noassoc foo.shlexec"; + expected = 33; sprintf(fileA, testfile, tmpdir); + rc=shell_execute(NULL, fileA, NULL, NULL); + TEST_LPFILE_PARSED_OK(); + + /* is SEE_MASK_DOENVSUBST default flag? Should only be when XP emulates 9x (XP bug or real 95 or ME behavior ?) */ + testfile = "%%TEMP%%\%s\simple.shlexec"; + expected = 2; sprintf(fileA, testfile, shorttmpdir); + rc=shell_execute(NULL, fileA, NULL, NULL); + todo_wine { TEST_LPFILE_PARSED_OK(); } + + /* quoted */ + testfile = ""%%TEMP%%\%s\simple.shlexec""; + expected = 2; sprintf(fileA, testfile, shorttmpdir); + rc=shell_execute(NULL, fileA, NULL, NULL); + todo_wine { TEST_LPFILE_PARSED_OK(); } + + /* test SEE_MASK_DOENVSUBST works */ + testfile = "%%TEMP%%\%s\simple.shlexec"; + expected = 33; sprintf(fileA, testfile, shorttmpdir); + rc=shell_execute_ex(SEE_MASK_DOENVSUBST | SEE_MASK_FLAG_NO_UI, NULL, fileA, NULL, NULL); + TEST_LPFILE_PARSED_OK(); + + /* quoted */ + testfile = ""%%TEMP%%\%s\simple.shlexec""; + expected = 33; sprintf(fileA, testfile, shorttmpdir); + rc=shell_execute_ex(SEE_MASK_DOENVSUBST | SEE_MASK_FLAG_NO_UI, NULL, fileA, NULL, NULL); + TEST_LPFILE_PARSED_OK(); +} + static void test_filename(void) { char filename[MAX_PATH]; @@ -1938,6 +2024,7 @@ START_TEST(shlexec)
init_test();
+ test_lpFile_parsed(); test_filename(); test_find_executable(); test_lnks();
On 02/12/2010 12:39 PM, Ilya Basin wrote:
Depends on your mail client I guess. I usually attach but there are others who inline.
PV> Why is this necessary? /* ensure tmpdir is in %TEMP%: GetTempPath() can succeed even if TEMP is undefined */
But do your tests actually rely on %TEMP% being defined? Not having a TEMP (or TMP) will probably makes loads of tests fail and I doubt one has a valid config without those.
Also when you sent a newer patch that has changes you should mark it as 'try x' instead of 'resend'. 'Resend' is used when you think the patch has been missed by AJ for example.
-
Ilya Basin -
Paul Vriens