[PATCH] add testcases for variables scope

[PATCH] add testcases for variables scope

[Zhou Wenjian]

	* testsuite/systemtap.base/var_scope.exp: New test case.
	* testsuite/systemtap.base/var_scope.stp: New test file.
---
 testsuite/systemtap.base/var_scope.exp | 14 ++++++++++++
 testsuite/systemtap.base/var_scope.stp | 41 ++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+)
 create mode 100755 testsuite/systemtap.base/var_scope.exp
 create mode 100755 testsuite/systemtap.base/var_scope.stp

diff --git a/testsuite/systemtap.base/var_scope.exp b/testsuite/systemtap.base/var_scope.exp
new file mode 100755
index 0000000..70a25a9
--- /dev/null
+++ b/testsuite/systemtap.base/var_scope.exp
@@ -0,0 +1,14 @@
+# Check variables scope
+
+set test "var_scope"
+if {![installtest_p]} { untested "$test"; return }
+
+foreach runtime [get_runtime_list] {
+    if {$runtime != ""} {
+	stap_run $srcdir/$subdir/$test.stp no_load $all_pass_string \
+	    --runtime=$runtime -w
+    } else {
+	stap_run $srcdir/$subdir/$test.stp no_load $all_pass_string \
+	    -w
+    }
+}
diff --git a/testsuite/systemtap.base/var_scope.stp b/testsuite/systemtap.base/var_scope.stp
new file mode 100755
index 0000000..78b6f46
--- /dev/null
+++ b/testsuite/systemtap.base/var_scope.stp
@@ -0,0 +1,41 @@
+/*
+ * var_scope.stp
+ *
+ * Check variables scope
+ */
+global var_global=2015;
+
+probe begin {
+    println("systemtap starting probe")
+    var_probe=2015
+}
+probe end   {  println("systemtap ending probe")    }
+
+
+function changevar()
+{
+    var_local=2014
+}
+
+probe end {
+    ret=0
+    if (var_global != 2015) {
+        ret=1
+        printf("systemtap test failure - var_global:%d != 2015\n", var_global)
+    }
+
+    var_local=2015
+    changevar()
+    if (var_local != 2015) {
+        ret=1
+        printf("systemtap test failure - var_local:%d != 2015\n", var_local)
+    }
+
+    if (var_probe == 2015) {
+        ret=1
+        printf("systemtap test failure - var_probe:%d == 2015\n", var_probe)
+    }
+
+    if (ret == 0)
+        println("systemtap test success")
+}
-- 
1.8.3.1

Re: [PATCH] add testcases for variables scope

["Zhou, Wenjian/周文剑"]

Hello David,

During this work, I found some logic in the testsuite is not so correct.
For example, in the testcase of "if":
probe end
{
         println("systemtap ending probe")
         if (1) {
                 println("systemtap test success");
         } else {
                 println("systemtap test failure");
         }
         if (0) {
                 println("systemtap test failure");
         } else {
                 println("systemtap test success");
         }
}
The output is:
systemtap ending probe
systemtap test success
systemtap test success


The testsuite only focus on the first message after "systemtap ending probe".
If the message is "systemtap test success", then pass the case.
It don't care whether the following message is success or fail.
It leads to the result that even if the output message is the following,
there also won't be any error message in the result.

systemtap ending probe
systemtap test success
systemtap test failure


I think there are two ways to fix it, if it needs to be fixed.
1,
adjust the function stap_run

2,
change the way of printing "systemtap test success" (likes the implementation in this patch)

I prefer the second, for it won't introduce some other errors.

-- 
Thanks
Zhou

On 11/05/2015 04:27 PM, Zhou Wenjian wrote:
> 	* testsuite/systemtap.base/var_scope.exp: New test case.
> 	* testsuite/systemtap.base/var_scope.stp: New test file.
> ---
>   testsuite/systemtap.base/var_scope.exp | 14 ++++++++++++
>   testsuite/systemtap.base/var_scope.stp | 41 ++++++++++++++++++++++++++++++++++
>   2 files changed, 55 insertions(+)
>   create mode 100755 testsuite/systemtap.base/var_scope.exp
>   create mode 100755 testsuite/systemtap.base/var_scope.stp
>
> diff --git a/testsuite/systemtap.base/var_scope.exp b/testsuite/systemtap.base/var_scope.exp
> new file mode 100755
> index 0000000..70a25a9
> --- /dev/null
> +++ b/testsuite/systemtap.base/var_scope.exp
> @@ -0,0 +1,14 @@
> +# Check variables scope
> +
> +set test "var_scope"
> +if {![installtest_p]} { untested "$test"; return }
> +
> +foreach runtime [get_runtime_list] {
> +    if {$runtime != ""} {
> +	stap_run $srcdir/$subdir/$test.stp no_load $all_pass_string \
> +	    --runtime=$runtime -w
> +    } else {
> +	stap_run $srcdir/$subdir/$test.stp no_load $all_pass_string \
> +	    -w
> +    }
> +}
> diff --git a/testsuite/systemtap.base/var_scope.stp b/testsuite/systemtap.base/var_scope.stp
> new file mode 100755
> index 0000000..78b6f46
> --- /dev/null
> +++ b/testsuite/systemtap.base/var_scope.stp
> @@ -0,0 +1,41 @@
> +/*
> + * var_scope.stp
> + *
> + * Check variables scope
> + */
> +global var_global=2015;
> +
> +probe begin {
> +    println("systemtap starting probe")
> +    var_probe=2015
> +}
> +probe end   {  println("systemtap ending probe")    }
> +
> +
> +function changevar()
> +{
> +    var_local=2014
> +}
> +
> +probe end {
> +    ret=0
> +    if (var_global != 2015) {
> +        ret=1
> +        printf("systemtap test failure - var_global:%d != 2015\n", var_global)
> +    }
> +
> +    var_local=2015
> +    changevar()
> +    if (var_local != 2015) {
> +        ret=1
> +        printf("systemtap test failure - var_local:%d != 2015\n", var_local)
> +    }
> +
> +    if (var_probe == 2015) {
> +        ret=1
> +        printf("systemtap test failure - var_probe:%d == 2015\n", var_probe)
> +    }
> +
> +    if (ret == 0)
> +        println("systemtap test success")
> +}
>

Re: [PATCH] add testcases for variables scope

[David Smith]

On 11/05/2015 02:27 AM, Zhou Wenjian wrote:
> 	* testsuite/systemtap.base/var_scope.exp: New test case.
> 	* testsuite/systemtap.base/var_scope.stp: New test file.

Thanks for the new test case, I've checked it in:

<https://sourceware.org/git/gitweb.cgi?p=systemtap.git;a=commitdiff;h=09b9b8b1d782630f4ccf7c129c6526cc45ed731a;hp=f104f853a637ee3a197d6094f362e45ed64f4d27>

Note that I made one small tweak. You had 2 end probes, the first
printing "systemtap ending probe" and the second printing the
success/fail message. For end probes without any sequence numbers, I
don't believe we guarantee any execution order. So, I combined the two
probes. The other option would have been to add sequence numbers, i.e.:

 probe end(0) ...
 probe end(1) ...

Thanks again.

-- 
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

Re: [PATCH] add testcases for variables scope

[David Smith]

On 11/05/2015 05:02 AM, "Zhou, Wenjian/周文剑" wrote:
> Hello David,
> 
> During this work, I found some logic in the testsuite is not so correct.
> For example, in the testcase of "if":
> probe end
> {
>         println("systemtap ending probe")
>         if (1) {
>                 println("systemtap test success");
>         } else {
>                 println("systemtap test failure");
>         }
>         if (0) {
>                 println("systemtap test failure");
>         } else {
>                 println("systemtap test success");
>         }
> }
> The output is:
> systemtap ending probe
> systemtap test success
> systemtap test success
> 
> 
> The testsuite only focus on the first message after "systemtap ending
> probe".

... stuff deleted ...

> I think there are two ways to fix it, if it needs to be fixed.
> 1,
> adjust the function stap_run
> 
> 2,
> change the way of printing "systemtap test success" (likes the
> implementation in this patch)
> 
> I prefer the second, for it won't introduce some other errors.

You way #2 above would have worked fine. I fixed it with a 3rd solution,
just changing the expected result string to be 2 copies of the
'all_pass_string':

<https://sourceware.org/git/gitweb.cgi?p=systemtap.git;a=commit;h=0d7d53dd805f636278858651a649d57fb87ba6c8>

Thanks for finding this.

-- 
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)

Re: [PATCH] add testcases for variables scope

["Zhou, Wenjian/周文剑"]

On 11/06/2015 12:55 AM, David Smith wrote:
> On 11/05/2015 05:02 AM, "Zhou, Wenjian/周文剑" wrote:
>> Hello David,
>>
>> During this work, I found some logic in the testsuite is not so correct.
>> For example, in the testcase of "if":
>> probe end
>> {
>>          println("systemtap ending probe")
>>          if (1) {
>>                  println("systemtap test success");
>>          } else {
>>                  println("systemtap test failure");
>>          }
>>          if (0) {
>>                  println("systemtap test failure");
>>          } else {
>>                  println("systemtap test success");
>>          }
>> }
>> The output is:
>> systemtap ending probe
>> systemtap test success
>> systemtap test success
>>
>>
>> The testsuite only focus on the first message after "systemtap ending
>> probe".
>
> ... stuff deleted ...
>
>> I think there are two ways to fix it, if it needs to be fixed.
>> 1,
>> adjust the function stap_run
>>
>> 2,
>> change the way of printing "systemtap test success" (likes the
>> implementation in this patch)
>>
>> I prefer the second, for it won't introduce some other errors.
>
> You way #2 above would have worked fine. I fixed it with a 3rd solution,
> just changing the expected result string to be 2 copies of the
> 'all_pass_string':
>
> <https://sourceware.org/git/gitweb.cgi?p=systemtap.git;a=commit;h=0d7d53dd805f636278858651a649d57fb87ba6c8>
>
> Thanks for finding this.
>

The testcase "if" is just an example.
There are still many other testcases having this problem...
Should I fix it?

-- 
Thanks
Zhou

Re: [PATCH] add testcases for variables scope

[David Smith]

On 11/05/2015 07:09 PM, "Zhou, Wenjian/周文剑" wrote:
> On 11/06/2015 12:55 AM, David Smith wrote:

>> ... stuff deleted ...
>>
>>> I think there are two ways to fix it, if it needs to be fixed.
>>> 1,
>>> adjust the function stap_run
>>>
>>> 2,
>>> change the way of printing "systemtap test success" (likes the
>>> implementation in this patch)
>>>
>>> I prefer the second, for it won't introduce some other errors.
>>
>> You way #2 above would have worked fine. I fixed it with a 3rd solution,
>> just changing the expected result string to be 2 copies of the
>> 'all_pass_string':
>>
>> <https://sourceware.org/git/gitweb.cgi?p=systemtap.git;a=commit;h=0d7d53dd805f636278858651a649d57fb87ba6c8>
>>
>>
>> Thanks for finding this.
> 
> The testcase "if" is just an example.
> There are still many other testcases having this problem...
> Should I fix it?

Yes, it would be great if this was fixed. The first step would be to get
a list of all the testcases with the problem. Then we'd fix it using
either method #2 or #3.

-- 
David Smith
dsmith@redhat.com
Red Hat
http://www.redhat.com
256.217.0141 (direct)
256.837.0057 (fax)