Hi Shuai,
On 04/21/14 03:45, Shuai Meng wrote:
> -
> - return return_bool(res, val);
> + V_VT(res) = VT_EMPTY;
> + return VariantChangeType(res, arg, VARIANT_LOCALBOOL, VT_BOOL);
You can't assume that res is not NULL. If result of the function is not
used, it will be NULL. A simple test case is like this:
call CBool(0)
Also, are you sure we want VARIANT_LOCALBOOL here? Did you test it
(probably by running tests on localized Windows with translated string
"False")?
Looking at CBool tests:
+Call ok(CBool(Empty) = False, "CBool(Empty) = " & CBool(Empty))
+Call ok(getVT(CBool(Empty)) = "VT_BOOL", "getVT(CBool(Empty)) = " & getVT(CBool(Empty)))
+Call ok(CBool(1) = True, "CBool(1) = " & CBool(1))
+Call ok(getVT(CBool(1)) = "VT_BOOL", "getVT(CBool(1)) = " & getVT(CBool(1)))
+Call ok(CBool(CLng(0)) = False, "CBool(CLng(0)) = " & CBool(CLng(0)))
+Call ok(getVT(CBool(CLng(0))) = "VT_BOOL", "getVT(CBool(CLng(0))) = " & getVT(CBool(CLng(0))))
+Call ok(CBool(CSng(0.5)) = True, "CBool(CSng(0.5)) = " & CBool(CSng(0.5)))
+Call ok(getVT(CBool(CSng(0.5))) = "VT_BOOL", "getVT(CBool(CSng(0.5))) = " & getVT(CBool(CSng(0.5))))
+Call ok(CBool(-0.56) = True, "CBool(-0.56) = " & CBool(-0.56))
+Call ok(getVT(CBool(-0.56)) = "VT_BOOL", "getVT(CBool(-0.56)) = " & getVT(CBool(-0.56)))
+Call ok(CBool(CCur(8.55)) = True, "CBool(CCur(8.55)) = " & CBool(CCur(8.55)))
+Call ok(getVT(CBool(CCur(8.55))) = "VT_BOOL", "getVT(CBool(CCur(8.55))) = " & getVT(CBool(CCur(8.55))))
+Call ok(CBool(CDate(0)) = False, "CBool(CDate(0)) = " & CBool(CDate(0)))
+Call ok(getVT(CBool(CDate(0))) = "VT_BOOL", "getVT(CBool(CDate(0))) = " & getVT(CBool(CDate(0))))
+Call ok(CBool(CDate(3.33)) = True, "CBool(CDate(3.33)) = " & CBool(CDate(3.33)))
+Call ok(getVT(CBool(CDate(3.33))) = "VT_BOOL", "getVT(CBool(CDate(3.33))) = " & getVT(CBool(CDate(3.33))))
+Call ok(CBool("6666.78") = True, "CBool(""6666.78"") = " & CBool("6666.78"))
+Call ok(getVT(CBool("6666.78")) = "VT_BOOL", "getVT(CBool(""6666.78"")) = " & getVT(CBool("6666.78")))
+Call ok(CBool(CByte(-0.1)) = False, "CBool(CByte(-0.1)) = " & CBool(CByte(-0.1)))
+Call ok(getVT(CBool(CByte(-0.1))) = "VT_BOOL", "getVT(CBool(CByte(-0.1))) = " & getVT(CBool(CByte(-0.1))))
Those look mostly good. However I'd like to see some more tests.
Interesting case is for example empty string and a "false", "False" and
"#FALSE#" string. Also I'd really like to see tests on object default
values. See the attached patch for an example.
Those comments apply also to other patches in the series. It may be
easier to proceed if you resent just the first patch fixed and include
tests that don't depending on other patches in the series in the patch
itself.
Cheers,
Jacek