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 :
> 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