Problem using dynamic values for wait step

[brianredbeard]

At some point the parsing of dynamic wait values has become broken. Introspection into what is occurring in tagui_parse.php is pretty hard to follow and print/debug statements haven't been much help.

Related to #122

[Aussiroth]

Hi,
I tested out a simple code for the wait with variable input

for i from 1 to 10
{
	wait '+i+'
	echo "i have waited " + i + " seconds"
}

This code worked as expected, i.e. waiting 1 second, then 2 seconds, and so on to 10 seconds.
The automation took a total of 56 seconds, which is expected since it would have had a total of 55 seconds of wait steps.

Can you provide some further context/code which caused the wait step to break intended behavior with dynamic input? Perhaps this may be caused by a casperjs runtime behavior on mac/linux platform.

[kensoh]

Adding on Alvin, I tried on macOS and Linux (CentOS), seems ok, even with new variable syntax -

(btw nice test case 👍)

wait `i`

Hey Brian, thanks for raising this! You've probably come across some edge case which was missed.

[brianredbeard]

I think it's actually a rather interesting variable scoping issue. I was playing a bit with the rendered JavaScript code, executing it directly so as to minimize variables.

This then lead me to see that your examples do indeed work while mine did not. This was my sample code. First, in tagui_local.js I have the following functions:

function getRandomInt(max) {
        max = typeof max !== 'undefined' ? max : 10;
        return Math.ceil(Math.random() * Math.floor(max));
}

function randomArray(max, count) {
        count = typeof count !== 'undefined' ? count : 10;
        max = typeof max !== 'undefined' ? max : 10;
        var array = [];
        for (c = 1; c <= count; c++) {
                array.push(getRandomInt(max));
        }
        return array;
}

Which I then utilize in my example:

for n from 1 to 5
{
        x = getRandomInt()
        wait '+x+'
}

This renders to the following javascript:

casper.then(function() {for (n=1; n<=5; n++)
{casper.then(function() {for_loop_signal = '[CONTINUE_SIGNAL][n]';});
(function (n) { // start of IIFE pattern
{ // start of code block

casper.then(function() { // start of JS code
x = getRandomInt()
}); // end of JS code

casper.then(function() {techo('wait '+x+'');});
casper.wait((parseFloat(''+x+'')*1000), function() {});

} // end of code block
})(n); // end of IIFE pattern, with dummy marker for break step

The trouble is that the definition of x = getRandomInt() seems to be in a different scope than x provided in the casper.wait() call. Due to the scoping issue, it then renders to zero, leaving no delay.

After realizing the scoping issue I got a bit creative and defined an array with values outside of the code block, then use a for loop to iterate over the array and reference the index of the values:

count = getRandomInt()
echo count
arr = randomArray(5,count)
echo arr

for n from 1 to count
{
        x = arr[n - 1]
        wait `arr[n-1]`
}

Which then results in:

./tagui randjs

START - automation started - Fri Oct 12 2018 11:36:59 GMT-0700 (PDT)

3
4,1,3
wait 4
wait 1
wait 3

FINISH - automation finished - 8.4s

This solution works as I would expect, though it was a bit of mental gymnastics to come to this conclusion.

[kensoh]

Hi Brian, I was expecting something cool to come out from your response and indeed - nice catch and thanks for sharing your findings & workaround!

Yeah, it's kinda convoluted, the way JS code blocks are generated to work with how CasperJS framework works. Can you try if below file (first remove the .txt extension, as GitHub doesn't allow attaching .php file here) works for your original code and perhaps other automation scripts that you've developed?

tagui_parse.php.txt

Hi Alvin,

In above tagui_parse.php I made changes to 2 lines in wait_intent() function. I encapsulated the casper.wait calls within a casper.then. This seems to fix the scoping issue. But because this is going to be a change that could potentially break other existing user scripts, can you have a look at some of your scripts to see if execution behavior still works as expected?

This is a bug to fix, need to figure out if the above change breaks existing scripts. I tried with some of the sample flows seems ok and the generated positive_test.js seems alright. But will be cautious to make this change without first testing exhaustively on current user scripts.

Below is amended version of wait_intent() -

function wait_intent($raw_intent) {
$params = trim(substr($raw_intent." ",1+strpos($raw_intent." "," "))); if ($params == "") $params = "5";
if (strpos($params,"'+")!==false and strpos($params,"+'")!==false) // handling for dynamic time
return "casper.then(function() {".
"techo('".$raw_intent."');});\ncasper.then(function() {casper.wait((parseFloat('".$params."')*1000), function() {"."});});"."\n\n";
else return "casper.then(function() {".
"techo('".$raw_intent."');});\ncasper.then(function() {casper.wait(" . (floatval($params)*1000) . ", function() {"."});});"."\n\n";}

Adding on a minimum replication case which involves interacting with web elements. For example, below script will return ReferenceError: Can't find variable: b error if above change is not made -

http://tebel.org/index.php
print footer
b = 5
wait `b`
print footer

[Aussiroth]

I see, I remember this scoping issue with the casper.then back from working with the for loop issues. I'll try to put it in and test out if behavior remains the same on the scripts I have.

[kensoh]

Adding on, I've sent an email to CK copying you. Since he originally raised the dynamic wait enhancement request, and he probably has a wide range of scripts to do good sanity tests.

Also, want to note here that popup step and frame step may have similar issues in the future. Their calls are not encapsulated within casper.then(). popup step by default takes in regex string so it probably not relevant here to accept dynamic inputs.

frame step would probably not work with a dynamic variable but i do think that is an unlikely use case (calling a frame name which the developer don't know in advance), perhaps can leave it as it is now with just this note for future review. Otherwise, will be adding complexity to the already very complicated frame step implementation.

[Aussiroth]

So far tested and for the scripts (5 of them) I had, the wait step triggers at the right time and for the right duration. Would be good to wait for CK to provide further confirmation though.

[Aussiroth]

#304 fixes this issue, so I will close this now.