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

Juan Lang juan.lang at gmail.com
Mon Mar 8 11:28:58 CST 2010


>>>> +    expected = 33; sprintf(fileA, testfile, tmpdir);
>>>> +    rc=shell_execute(NULL, fileA, NULL, NULL);
>>>> +    todo_wine {
>>>> +        ok(rc==expected || (rc>32&&  expected>32),
>>>> +            "expected %s (%d), got %s (%d), lpFile: %s \n",
>>>> +            expected==33 ? "success" : "failure", expected, rc>  32 ? "success" : "failure", rc, fileA
>>>> +            );
>>>> +    }
>>>
>>> In this case, expected is 33, so the "expected == 33" is unneeded.
>>> Just replace "%s (%d)" with "success (32)".  Same below with the
>>> remaining ok expressions:  make them match the actual value of
>>> expected, rather than any possible value.  Otherwise, looks good to
>>> me.
>
> PV> Yep, I like this one better as well. The only thing I see is that you
> PV> have "==expected" and "rc>32" (which is both true in the above case). Is
> PV> that to cover for OS differences? If so, please add some comments.
> No, that's because I use the same expression for both failure and
> success.
> Now Juan tells me to have unique format string for each case. Let's stop
> here already!

We both have gotten many patches committed, so I think our predictions
are better than most ;-)

Paul and I don't disagree, actually, we just spotted different things.
 I was talking about the format string, he's talking about the ok
expression.  Both have unnecessary expressions, and we're both telling
you to remove what's unnecessary.  So, going back to the example we
started with, here's what you have:

+    expected = 33; sprintf(fileA, testfile, tmpdir);
+    rc=shell_execute(NULL, fileA, NULL, NULL);
+    todo_wine {
+        ok(rc==expected || (rc>32&&  expected>32),
+            "expected %s (%d), got %s (%d), lpFile: %s \n",
+            expected==33 ? "success" : "failure", expected, rc>  32 ?
"success" : "failure", rc, fileA
+            );
+    }

and here's what it would look like, taking into account both Paul's
and my suggestions:

+    expected = 33; sprintf(fileA, testfile, tmpdir);
+    rc=shell_execute(NULL, fileA, NULL, NULL);
+    todo_wine {
+        ok(rc==expected,
+            "expected success (33), got %s (%d), lpFile: %s \n",
+            rc>  32 ? "success" : "failure", rc, fileA
+            );
+    }

Hope that helps.
--Juan



More information about the wine-devel mailing list