Re: [PATCH] vbscript: Fixed CBool(try 2)

[Piotr Caban]

On 05/01/14 10:16, Shuai Meng wrote:
> Changelog:
>    added tests on object subtype.
>    improved CBool.
>
> https://newtestbot.winehq.org/JobDetails.pl?Key=6689
>
> ---
>   dlls/vbscript/global.c      | 41
> ++++++++++++-----------------------------
>   dlls/vbscript/tests/api.vbs | 42
> ++++++++++++++++++++++++++++++++++++++++++
>   2 files changed, 54 insertions(+), 29 deletions(-)
> The tests are not passing on wine:
> run.c:1013: Test failed: api.vbs: L"Err.number = 0"
> run.c:1013: Test failed: api.vbs: L"Err.number = 0"
> run.c:1013: Test failed: api.vbs: L"Err.number = 0"

These are the failing tests:
Call testCBoolError("#TRUE#", 458)
Call testCBoolError("#FALSE#", 458)
Call testCBoolError(MyObject, 458)

Could you please also add following tests:
MyObject.myval = 1
Call ok(CBool(MyObject) = True, "CBool(MyObject) = " & CBool(MyObject))
MyObject.myval = 0
Call ok(CBool(MyObject) = False, "CBool(MyObject) = " & CBool(MyObject))

[PATCH] vbscript: Fixed CBool(try 2)

[Shuai Meng]

Thank you for commenting.


2014-05-01 17:59 GMT+08:00 Piotr Caban <piotr.caban(a)gmail.com>:
> On 05/01/14 10:16, Shuai Meng wrote:
> > Changelog:
> >    added tests on object subtype.
> >    improved CBool.
> >
> > https://newtestbot.winehq.org/JobDetails.pl?Key=6689
> >
> > ---
> >   dlls/vbscript/global.c      | 41
> > ++++++++++++-----------------------------
> >   dlls/vbscript/tests/api.vbs | 42
> > ++++++++++++++++++++++++++++++++++++++++++
> >   2 files changed, 54 insertions(+), 29 deletions(-)
>
> The tests are not passing on wine:
> run.c:1013: Test failed: api.vbs: L"Err.number = 0"
> run.c:1013: Test failed: api.vbs: L"Err.number = 0"
> run.c:1013: Test failed: api.vbs: L"Err.number = 0"
>
> I have changed the tests like this and they can pass on wine with no
> failure.
> 481 Sub testCBoolError
> 482     on error resume next
> 483
> 484     Call Err.clear()
> 485     Call CBool("")
> 486     Call ok(Err.number = 13, "Err.number = " & Err.number)
> 487
> 488     Call Err.clear()
> 489     Call CBool("#False#")
> 490     Call ok(Err.number = 13, "Err.number = " & Err.number)
> 491
> 492     Call Err.clear()
> 493     Call CBool("#True#")
> 494     Call ok(Err.number = 13, "Err.number = " & Err.number)
> 495
> 496     Call Err.clear()
> 497     Call CBool("MyObject")
> 498     Call ok(Err.number = 13, "Err.number = " & Err.number)
> 499 End Sub

These are the failing tests:
> Call testCBoolError("#TRUE#", 458)
> Call testCBoolError("#FALSE#", 458)
> Call testCBoolError(MyObject, 458)
>
> As showed above, I have changed 458 into 13, and can pass on winxp.
> However there two failures come nowhere like this:
> run.c:1013: Test failed: api.vbs: L"Err.number = 0"
> run.c:1013: Test failed: api.vbs: L"Err.number = 0"
> I can't understand it: if Err.number = 0 then everything is right, why does
> it come out?
> Could you please also add following tests:
> MyObject.myval = 1
> Call ok(CBool(MyObject) = True, "CBool(MyObject) = " & CBool(MyObject))
> MyObject.myval = 0
> Call ok(CBool(MyObject) = False, "CBool(MyObject) = " & CBool(MyObject))

Ok, but does this mean I am testing object in the proper way?

[PATCH] vbscript: Fixed CBool(try 2)

[Piotr Caban]

On 05/01/14 15:22, Shuai Meng wrote:
> I have changed the tests like this and they can pass on wine with no
> failure.
> 481 Sub testCBoolError
> 482     on error resume next
> 483
> 484     Call Err.clear()
> 485     Call CBool("")
> 486     Call ok(Err.number = 13, "Err.number = " & Err.number)
> 487
> 488     Call Err.clear()
> 489     Call CBool("#False#")
> 490     Call ok(Err.number = 13, "Err.number = " & Err.number)
> 491
> 492     Call Err.clear()
> 493     Call CBool("#True#")
> 494     Call ok(Err.number = 13, "Err.number = " & Err.number)
> 495
> 496     Call Err.clear()
> 497     Call CBool("MyObject")
> 498     Call ok(Err.number = 13, "Err.number = " & Err.number)
> 499 End Sub
> I think that previous way of implementing this tests was nicer.  The 
> results are different because you're testing different things (e.g. 
> "MyObject" instead of MyObject, "#True#" instead of "#TRUE#").
> These are the failing tests:
>     Call testCBoolError("#TRUE#", 458)
>     Call testCBoolError("#FALSE#", 458)
>     Call testCBoolError(MyObject, 458)

In order to fix it you need to change the implementation. Probably you 
need to check if string equals to "#TRUE#" and return early without 
calling VariantChangeType. I'm not sure what needs to be done in 
MyObject case.
> As showed above, I have changed 458 into 13, and can pass on winxp.
> However there two failures come nowhere like this:
> run.c:1013: Test failed: api.vbs: L"Err.number = 0"
> run.c:1013: Test failed: api.vbs: L"Err.number = 0"
> I can't understand it: if Err.number = 0 then everything is right, why
> does it come out?
> It's happening because on wine the test is succeeding while it's 
> supposed to fail.
> Could you please also add following tests:
>     MyObject.myval = 1
>     Call ok(CBool(MyObject) = True, "CBool(MyObject) = " & CBool(MyObject))
>     MyObject.myval = 0
>     Call ok(CBool(MyObject) = False, "CBool(MyObject) = " & CBool(MyObject))
>
> Ok, but does this mean I am testing object in the proper way?

Currently you're only testing what happens if default value is unset 
(variant VT_EMPTY). This is just testing what happens if other values 
are returned.

[PATCH] vbscript: Fixed CBool(try 2)

[Jacek Caban]

On 05/01/14 16:38, Piotr Caban wrote:
> > These are the failing tests:
> >     Call testCBoolError("#TRUE#", 458)
> >     Call testCBoolError("#FALSE#", 458)
> >     Call testCBoolError(MyObject, 458)
>
> In order to fix it you need to change the implementation. Probably you
> need to check if string equals to "#TRUE#" and return early without
> calling VariantChangeType. I'm not sure what needs to be done in
> MyObject case.

Actually, I'd like to see a cleaner solution than special casing
specific strings.

Jacek