search

Danp2 / au3WebDriver

Web Driver UDF for AutoIt
MIT License
108 stars 23 forks source link

_WD_DebugSwitch fix + refactoring #423

Closed mlipok closed 1 year ago

mlipok commented 1 year ago

Discussion is in: https://github.com/Danp2/au3WebDriver/pull/422

Danp2 commented 1 year ago

@mlipok Not sure if you saw this post in the original PR -- https://github.com/Danp2/au3WebDriver/pull/422#discussion_r1117065685. If the change in commit https://github.com/Danp2/au3WebDriver/pull/422/commits/40c0e73503864c4f9cfeca66c149f2348391baf0 is correct, then I believe that your code in this PR needs further revisions.

mlipok commented 1 year ago

@mlipok Not sure if you saw this post in the original PR -- #422 (comment). If the change in commit 40c0e73 is correct, then I believe that your code in this PR needs further revisions.

I change concept of code (refactoring) image

Did you check how this code works ? Currently I get:

+ wd_demo.au3: Running: UserTesting
1090 START:                     $_WD_DEBUG=2
! 
1095 RESULT=2     After $_WD_DEBUG_None:    $_WD_DEBUG=0
_WD_DebugSwitch ==> Success [0 / 0]
1097 RESULT=1     Restoring:            $_WD_DEBUG=2
! 
1101 RESULT=2     After $_WD_DEBUG_Error:   $_WD_DEBUG=1
_WD_DebugSwitch ==> Success [0 / 0]
1103 RESULT=1     Restoring:            $_WD_DEBUG=2
! 
_WD_DebugSwitch ==> Success [0 / 0]
1107 RESULT=2     After $_WD_DEBUG_Info:    $_WD_DEBUG=2
_WD_DebugSwitch ==> Success [0 / 0]
1109 RESULT=1     Restoring:            $_WD_DEBUG=2
! 
_WD_DebugSwitch ==> Success [0 / 0]
1113 RESULT=2     After $_WD_DEBUG_Full:    $_WD_DEBUG=3
_WD_DebugSwitch ==> Success [0 / 0]
1115 RESULT=1     Restoring:            $_WD_DEBUG=2
+ wd_demo.au3: Finished: UserTesting
mlipok commented 1 year ago

Of course I can refactor it a little more

Danp2 commented 1 year ago

Did you check how this code works ?

No, because I wanted you to respond to the correctness of the change in my PR first since you are proposing to merge your PR into mine.

The current functionality stores $vMode (the new value for $_WD_Debug) onto the stack. I believe this is wrong, since the stack should only contain the value of prior $_WD_Debug settings, not the new proposed setting.

mlipok commented 1 year ago

I believe this is wrong, since the stack should only contain the value of prior $_WD_Debug settings, not the new proposed setting.

Give some time to think about.

mlipok commented 1 year ago

I believe this is wrong, since the stack should only contain the value of prior $_WD_Debug settings, not the new proposed setting.

Give some time to think about.

I can agree with your aproach but this changes a little more than a single line. Will prepare new commit ASAP

mlipok commented 1 year ago

Please take a look on: refactoring - first usage - empty stack array

Func UserTesting() ; here you can replace the code to test your stuff before you ask on the forum

    ConsoleWrite('! ' & @ScriptLineNumber & ' START 1:              $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    Local $iResult

    ConsoleWrite("! " & @CRLF)
    $iResult = _WD_DebugSwitch($_WD_DEBUG_None)
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     After $_WD_DEBUG_None:   $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     Restoring:           $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)

    ConsoleWrite("! " & @CRLF)
    $iResult = _WD_DebugSwitch($_WD_DEBUG_Error)
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     After $_WD_DEBUG_Error:  $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     Restoring:           $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)

    ConsoleWrite("! " & @CRLF)
    $iResult = _WD_DebugSwitch($_WD_DEBUG_Info)
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     After $_WD_DEBUG_Info:   $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     Restoring:           $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)

    ConsoleWrite("! " & @CRLF)
    $iResult = _WD_DebugSwitch($_WD_DEBUG_Full)
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     After $_WD_DEBUG_Full:   $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     Restoring:           $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)

    ConsoleWrite('! ' & @ScriptLineNumber & ' START 2:              $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)

    $iResult = _WD_DebugSwitch($_WD_DEBUG_None)
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     After $_WD_DEBUG_None:   $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch($_WD_DEBUG_Error)
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     After $_WD_DEBUG_Error:  $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch($_WD_DEBUG_Info)
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     After $_WD_DEBUG_Info:   $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch($_WD_DEBUG_Full)
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     After $_WD_DEBUG_Full:   $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     Restoring:           $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     Restoring:           $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     Restoring:           $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     Restoring:           $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite(@ScriptLineNumber & ' RESULT=' & $iResult & '     Restoring:           $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)

EndFunc   ;==>UserTesting

my results are:

+ wd_demo.au3: Running: UserTesting
! 1090 START 1:                 $_WD_DEBUG=2
! 
1095 RESULT=1     After $_WD_DEBUG_None:    $_WD_DEBUG=0
_WD_DebugSwitch ==> Success [0 / 0]
1097 RESULT=0     Restoring:            $_WD_DEBUG=2
! 
1101 RESULT=1     After $_WD_DEBUG_Error:   $_WD_DEBUG=1
_WD_DebugSwitch ==> Success [0 / 0]
1103 RESULT=0     Restoring:            $_WD_DEBUG=2
! 
_WD_DebugSwitch ==> Success [0 / 0]
1107 RESULT=1     After $_WD_DEBUG_Info:    $_WD_DEBUG=2
_WD_DebugSwitch ==> Success [0 / 0]
1109 RESULT=0     Restoring:            $_WD_DEBUG=2
! 
_WD_DebugSwitch ==> Success [0 / 0]
1113 RESULT=1     After $_WD_DEBUG_Full:    $_WD_DEBUG=3
_WD_DebugSwitch ==> Success [0 / 0]
1115 RESULT=0     Restoring:            $_WD_DEBUG=2
! 1118 START 2:                 $_WD_DEBUG=2
1121 RESULT=1     After $_WD_DEBUG_None:    $_WD_DEBUG=0
1123 RESULT=2     After $_WD_DEBUG_Error:   $_WD_DEBUG=1
_WD_DebugSwitch ==> Success [0 / 0]
1125 RESULT=3     After $_WD_DEBUG_Info:    $_WD_DEBUG=2
_WD_DebugSwitch ==> Success [0 / 0]
1127 RESULT=4     After $_WD_DEBUG_Full:    $_WD_DEBUG=3
_WD_DebugSwitch ==> Success [0 / 0]
1129 RESULT=3     Restoring:            $_WD_DEBUG=2
1131 RESULT=2     Restoring:            $_WD_DEBUG=1
1133 RESULT=1     Restoring:            $_WD_DEBUG=0
_WD_DebugSwitch ==> Success [0 / 0]
1135 RESULT=0     Restoring:            $_WD_DEBUG=2
_WD_DebugSwitch ==> Success [0 / 0] : There are no saved debug levels
1137 RESULT=0     Restoring:            $_WD_DEBUG=2
+ wd_demo.au3: Finished: UserTesting
Danp2 commented 1 year ago

After seeing your latest changes, I now realize that I was misunderstanding your prior implementation where the "current" value would have already been on the stack and the "new" value was being added.

mlipok commented 1 year ago

Ok. But the present version is good for you ? because for me it is good.

Danp2 commented 1 year ago

I just tested it. One concern I have is in regards to these results --

    _WD_DebugSwitch ==> Success [0 / 0] : Invalid argument in function-call
    124 RESULT=-1     Restoring:            $_WD_DEBUG=3```
    _WD_DebugSwitch ==> Success [0 / 0] : There are no saved debug levels
    124 RESULT=0     Restoring:         $_WD_DEBUG=3

These are misleading to me because of the _WD_DebugSwitch ==> Success [0 / 0]. Also, there's no way to test if the restore failed in the 2nd example.

mlipok commented 1 year ago

Also, there's no way to test if the restore failed in the 2nd example.

So we should set return to -1

        Else
            $iStackSize = -1
            $sMessage = 'There are no saved debug levels'
        EndIf

because this is the only one situation when restore can fail.

EDIT: Introduced here: return -1 when 'There are no saved debug levels'

mlipok commented 1 year ago

These are misleading to me because of the _WD_DebugSwitch ==> Success [0 / 0].

take a look on this change: Return SetError($iErr, $iExt, $iStackSize)

mlipok commented 1 year ago

current testing script

Func UserTesting() ; here you can replace the code to test your stuff before you ask on the forum

    ConsoleWrite('! ' & @ScriptLineNumber & ' START 1:              $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    Local $iResult

    ConsoleWrite("! " & @CRLF)
    $iResult = _WD_DebugSwitch($_WD_DEBUG_None)
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   $_WD_DEBUG_None:    $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   restoring:      $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)

    ConsoleWrite("! " & @CRLF)
    $iResult = _WD_DebugSwitch($_WD_DEBUG_Error)
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   $_WD_DEBUG_Error:   $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   restoring:      $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)

    ConsoleWrite("! " & @CRLF)
    $iResult = _WD_DebugSwitch($_WD_DEBUG_Info)
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   $_WD_DEBUG_Info:    $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   restoring:      $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)

    ConsoleWrite("! " & @CRLF)
    $iResult = _WD_DebugSwitch($_WD_DEBUG_Full)
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   $_WD_DEBUG_Full:    $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   restoring:      $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)

    ConsoleWrite('! ' & @ScriptLineNumber & ' START 2:              $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)

    $iResult = _WD_DebugSwitch($_WD_DEBUG_None)
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   $_WD_DEBUG_None:    $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch($_WD_DEBUG_Error)
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   $_WD_DEBUG_Error:   $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch($_WD_DEBUG_Info)
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   $_WD_DEBUG_Info:    $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch($_WD_DEBUG_Full)
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   $_WD_DEBUG_Full:    $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   restoring:      $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   restoring:      $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   restoring:      $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   restoring:      $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)
    $iResult = _WD_DebugSwitch()
    ConsoleWrite('> ' & @ScriptLineNumber & ' RESULT=' & $iResult & '   After   restoring:      $_WD_DEBUG=' & $_WD_DEBUG & @CRLF)

EndFunc   ;==>UserTesting

and current results:

+ wd_demo.au3: Running: UserTesting
! 1090 START 1:                 $_WD_DEBUG=2
! 
> 1095 RESULT=1 After   $_WD_DEBUG_None:    $_WD_DEBUG=0
_WD_DebugSwitch:  stack size: 0
> 1097 RESULT=0 After   restoring:      $_WD_DEBUG=2
! 
> 1101 RESULT=1 After   $_WD_DEBUG_Error:   $_WD_DEBUG=1
_WD_DebugSwitch:  stack size: 0
> 1103 RESULT=0 After   restoring:      $_WD_DEBUG=2
! 
_WD_DebugSwitch:  stack size: 1
> 1107 RESULT=1 After   $_WD_DEBUG_Info:    $_WD_DEBUG=2
_WD_DebugSwitch:  stack size: 0
> 1109 RESULT=0 After   restoring:      $_WD_DEBUG=2
! 
_WD_DebugSwitch:  stack size: 1
> 1113 RESULT=1 After   $_WD_DEBUG_Full:    $_WD_DEBUG=3
_WD_DebugSwitch:  stack size: 0
> 1115 RESULT=0 After   restoring:      $_WD_DEBUG=2
! 1118 START 2:                 $_WD_DEBUG=2
> 1121 RESULT=1 After   $_WD_DEBUG_None:    $_WD_DEBUG=0
> 1123 RESULT=2 After   $_WD_DEBUG_Error:   $_WD_DEBUG=1
_WD_DebugSwitch:  stack size: 3
> 1125 RESULT=3 After   $_WD_DEBUG_Info:    $_WD_DEBUG=2
_WD_DebugSwitch:  stack size: 4
> 1127 RESULT=4 After   $_WD_DEBUG_Full:    $_WD_DEBUG=3
_WD_DebugSwitch:  stack size: 3
> 1129 RESULT=3 After   restoring:      $_WD_DEBUG=2
> 1131 RESULT=2 After   restoring:      $_WD_DEBUG=1
> 1133 RESULT=1 After   restoring:      $_WD_DEBUG=0
_WD_DebugSwitch:  stack size: 0
> 1135 RESULT=0 After   restoring:      $_WD_DEBUG=2
_WD_DebugSwitch: There are no saved debug levels stack size: -1
> 1137 RESULT=-1    After   restoring:      $_WD_DEBUG=2
+ wd_demo.au3: Finished: UserTesting
Danp2 commented 1 year ago

_WD_DebugSwitch: There are no saved debug levels stack size: -1

This could be improved --

mlipok commented 1 year ago
  • Stack size is actually 0, not -1

negative values indicate an error.

  • Provide separation between There are no saved debug levels and stack size: -1

commit is comming soon.

mlipok commented 1 year ago

This could be improved

please take a look: negative values indicate an error

Danp2 commented 1 year ago
  • Stack size is actually 0, not -1

negative values indicate an error.

That's fine, but it shouldn't display stack size IMO when the value shown is an error code

Provide separation between There are no saved debug levels and stack size: -1

I don't see where you addressed this.

Putting these two together, I'm expecting to see something like this -

_WD_DebugSwitch: There are no saved debug levels / error code: -1

mlipok commented 1 year ago

using: stack size / error code

I get:

+ wd_demo.au3: Running: UserTesting
! 1090 START 1:                 $_WD_DEBUG=2
! 
> 1095 RESULT=1 After   $_WD_DEBUG_None:    $_WD_DEBUG=0
_WD_DebugSwitch:  stack size: 0
> 1097 RESULT=0 After   restoring:      $_WD_DEBUG=2
! 
> 1101 RESULT=1 After   $_WD_DEBUG_Error:   $_WD_DEBUG=1
_WD_DebugSwitch:  stack size: 0
> 1103 RESULT=0 After   restoring:      $_WD_DEBUG=2
! 
_WD_DebugSwitch:  stack size: 1
> 1107 RESULT=1 After   $_WD_DEBUG_Info:    $_WD_DEBUG=2
_WD_DebugSwitch:  stack size: 0
> 1109 RESULT=0 After   restoring:      $_WD_DEBUG=2
! 
_WD_DebugSwitch:  stack size: 1
> 1113 RESULT=1 After   $_WD_DEBUG_Full:    $_WD_DEBUG=3
_WD_DebugSwitch:  stack size: 0
> 1115 RESULT=0 After   restoring:      $_WD_DEBUG=2
! 1118 START 2:                 $_WD_DEBUG=2
> 1121 RESULT=1 After   $_WD_DEBUG_None:    $_WD_DEBUG=0
> 1123 RESULT=2 After   $_WD_DEBUG_Error:   $_WD_DEBUG=1
_WD_DebugSwitch:  stack size: 3
> 1125 RESULT=3 After   $_WD_DEBUG_Info:    $_WD_DEBUG=2
_WD_DebugSwitch:  stack size: 4
> 1127 RESULT=4 After   $_WD_DEBUG_Full:    $_WD_DEBUG=3
_WD_DebugSwitch:  stack size: 3
> 1129 RESULT=3 After   restoring:      $_WD_DEBUG=2
> 1131 RESULT=2 After   restoring:      $_WD_DEBUG=1
> 1133 RESULT=1 After   restoring:      $_WD_DEBUG=0
_WD_DebugSwitch:  stack size: 0
> 1135 RESULT=0 After   restoring:      $_WD_DEBUG=2
_WD_DebugSwitch: There are no saved debug levels error code: -1
> 1137 RESULT=-1    After   restoring:      $_WD_DEBUG=2
+ wd_demo.au3: Finished: UserTesting
mlipok commented 1 year ago

and using this: $sMessage formating

_WD_DebugSwitch: There are no saved debug levels / error code: -1
> 1137 RESULT=-1    After   restoring:      $_WD_DEBUG=2
+ wd_demo.au3: Finished: UserTesting