On Tue, 2020-12-22 at 12:39 +0100, Martin Liška wrote:
> Hello.
> 
> The patch adds a new test for an existing GCOV test-case. Newly
> added run-gcov-pytest parses JSON format produced by GCOV and
> runs pytest on it.
> 
> Patch can bootstrap on x86_64-linux-gnu and survives regression
> tests.

At a high level, this patch calls out to Python 3, allowing for test
logic to be written in Python, rather than Tcl.  Are we doing this
anywhere else in our test suite?  I'm in favor of this (I'm much more
comfortable in Python than in Tcl, I dread anytime I have to touch the
Tcl code).

The test implicitly requires python3, and the 3rd party pytest module
installed within it.  What happens if these aren't installed?  (ideally
an UNSUPPORTED at the DejaGnu level, I think).

Some further comments inline below...

> Ready to be installed?
> Thanks,
> Martin
> 
> gcc/testsuite/ChangeLog:
> 
>       PR gcov-profile/98273
>       * lib/gcov.exp: Add run-gcov-pytest function which runs pytest.
>       * g++.dg/gcov/pr98273.C: New test.
>       * g++.dg/gcov/gcov.py: New test.
>       * g++.dg/gcov/test-pr98273.py: New test.
> ---
>   gcc/testsuite/g++.dg/gcov/gcov.py         | 10 ++++++++
>   gcc/testsuite/g++.dg/gcov/pr98273.C       | 24 +++++++++++++++++++
>   gcc/testsuite/g++.dg/gcov/test-pr98273.py | 27
> ++++++++++++++++++++++
>   gcc/testsuite/lib/gcov.exp                | 28
> +++++++++++++++++++++++
>   4 files changed, 89 insertions(+)
>   create mode 100644 gcc/testsuite/g++.dg/gcov/gcov.py
>   create mode 100644 gcc/testsuite/g++.dg/gcov/pr98273.C
>   create mode 100644 gcc/testsuite/g++.dg/gcov/test-pr98273.py
> 
> diff --git a/gcc/testsuite/g++.dg/gcov/gcov.py
> b/gcc/testsuite/g++.dg/gcov/gcov.py
> new file mode 100644
> index 00000000000..a8c4ea9ae71
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/gcov/gcov.py
> @@ -0,0 +1,10 @@
> +import gzip
> +import json
> +import os
> +
> +
> +def gcov_from_env():
> +    # return parsed JSON content a GCOV_PATH file
> +    json_filename = os.environ['GCOV_PATH'] + '.gcov.json.gz'
> +    json_data = gzip.open(json_filename).read()
> +    return json.loads(json_data)
> diff --git a/gcc/testsuite/g++.dg/gcov/pr98273.C
> b/gcc/testsuite/g++.dg/gcov/pr98273.C
> new file mode 100644
> index 00000000000..bfa83cbe4d0
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/gcov/pr98273.C
> @@ -0,0 +1,24 @@
> +/* PR gcov-profile/98273 */
> +
> +/* { dg-options "--coverage -std=c++11" } */
> +/* { dg-do run { target native } } */
> +
> +int
> +main ()
> +{
> +  int i = 42;
> +  {
> +    auto f = [] () {
> +      auto g = [] () {};
> +      g ();
> +      g ();
> +    };
> +    f ();
> +  }
> +  ++i;
> +  ++i;
> +  ++i;
> +  return 45 - i;
> +}
> +
> +/* { dg-final { run-gcov-pytest pr98273.C "test-pr98273.py" } } */
> diff --git a/gcc/testsuite/g++.dg/gcov/test-pr98273.py
> b/gcc/testsuite/g++.dg/gcov/test-pr98273.py
> new file mode 100644
> index 00000000000..6cb39d10c1e
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/gcov/test-pr98273.py

I had an idea, not sure if a good one: if all of the test logic is
moved from the .C file to a python script, then perhaps the script
should be the name of the .C file with a .py suffix i.e. here it could
be "pr98273.C.py"

Or would there be cases where different scripts could be called for the
same .C file?  Would scripts get reused between different source files?

(not sure)

> @@ -0,0 +1,27 @@
> +from gcov import gcov_from_env
> +
> +import pytest
> +
> +
> +@pytest.fixture(scope='function', autouse=True)
> +def gcov():
> +    return gcov_from_env()
> +
> +
> +def test_basics(gcov):
> +    files = gcov['files']
> +    assert len(files) == 1
> +    functions = files[0]['functions']
> +    assert len(functions) == 3
> +
> +
> +def test_lines(gcov):
> +    lines = gcov['files'][0]['lines']
> +    linesdict = {}
> +    for line in lines:
> +        linesdict[int(line['line_number'])] = line
> +
> +    assert linesdict[21]['function_name'] == 'main'
> +    assert linesdict[15]['function_name'] == '_ZZ4mainENKUlvE_clEv'
> +    assert (linesdict[12]['function_name']
> +            == '_ZZZ4mainENKUlvE_clEvENKUlvE_clEv')
> diff --git a/gcc/testsuite/lib/gcov.exp b/gcc/testsuite/lib/gcov.exp
> index 9276aead06b..dd589d4dd8a 100644
> --- a/gcc/testsuite/lib/gcov.exp
> +++ b/gcc/testsuite/lib/gcov.exp
> @@ -247,6 +247,34 @@ proc verify-calls { testname testcase file } {
>       return $failed
>   }
>   
> +proc run-gcov-pytest { args } {

I think this function needs a leading comment, talking about what is
invoked, and the kinds of lines that are parsed.

What happens if an exception is raised by the script?  e.g. if there's
a SyntaxError in the script, ideally it should "bubble up" through the
Tcl layer into a DejaGnu "ERROR" I think, rather than being silently
dropped.

> +    global GCOV
> +    global srcdir subdir
> +    # Extract the test file name from the arguments.
> +    set testcase [lindex $args 0]
> +
> +    verbose "Running $GCOV $testcase in $srcdir/$subdir" 2
> +    set testcase [remote_download host $testcase]
> +    set result [remote_exec host $GCOV "$testcase -i"]
> +
> +    set pytest_script [lindex $args 1]
> +    setenv GCOV_PATH $testcase
> +    verbose "pytest_script: $pytest_script" 2
> +    spawn -noecho python3 -m pytest --color=no -rA -s --tb=no
> $srcdir/$subdir/$pytest_script
> +
> +    set prefix "\[^\r\n\]*"
> +    expect {
> +      -re "FAILED($prefix)\[^\r\n\]+\r\n" {
> +       fail "$expect_out(1,string)"
> +       exp_continue
> +      }
> +      -re "PASSED($prefix)\[^\r\n\]+\r\n" {
> +       pass "$expect_out(1,string)"
> +       exp_continue
> +      }
> +    }
> +}
> +
>   # Called by dg-final to run gcov and analyze the results.
>   #
>   # ARGS consists of the optional strings "branches" and/or "calls",

Hope this is constructive
Dave

Reply via email to