[LNT][PATCH] Move submission code into test-suite code.

Chris Matthews chris.matthews at apple.com
Mon Dec 2 18:15:35 PST 2013


I like the suggestion to use lit to report the error.  In my case lit is not installed on my machine.  

Is is safe to assume lit as a dependency?

On Oct 10, 2013, at 12:15 PM, Daniel Dunbar <daniel at zuster.org> wrote:

> Looks pretty good, two nits inline:
> 
> > diff --git a/lnt/lnttool/main.py b/lnt/lnttool/main.py
> > index 3fb96e5a4eb4879fcccdf9fc9364c7feea22e620..f140f209951a520d3a6125b9ba54335f4384f287 100644
> > --- a/lnt/lnttool/main.py
> > +++ b/lnt/lnttool/main.py
> > @@ -130,28 +130,29 @@ def action_checkformat(name, args):
> >  def action_runtest(name, args):
> >      """run a builtin test application"""
> >  
> > +    # Runtest accepting options is deprecated, but lets not break the
> > +    # world, so collect them anyways and pass them on.
> >      parser = OptionParser("%s test-name [options]" % name)
> >      parser.disable_interspersed_args()
> > -    parser.add_option("", "--submit", dest="submit_url", metavar="URLORPATH",
> > -                      help=("autosubmit the test result to the given server "
> > -                            "(or local instance) [%default]"),
> > -                      type=str, default=None)
> > -    parser.add_option("", "--commit", dest="commit",
> > -                      help=("whether the autosubmit result should be committed "
> > -                            "[%default]"),
> > -                      type=int, default=True)
> > -    parser.add_option("", "--output", dest="output", metavar="PATH",
> > -                      help="write raw report data to PATH (or stdout if '-')",
> > -                      action="store", default=None)
> > -    parser.add_option("-v", "--verbose", dest="verbose",
> > -                      help="show verbose test results",
> > -                      action="store_true", default=False)
> > +    parser.add_option("", "--submit", dest="submit", type=str, default=None)
> > +    parser.add_option("", "--commit", dest="commit", type=str, default=None)
> > +    parser.add_option("", "--output", dest="output", type=str, default=None)
> > +    parser.add_option("-v", "--verbose", dest="verbose", default=None)
> >  
> > -    (opts, args) = parser.parse_args(args)
> > +    logger, _ = setup_logging()
> > +
> > +    (deprecated_opts, args) = parser.parse_args(args)
> >      if len(args) < 1:
> >          parser.error("incorrect number of argments")
> > -
> > -    test_name,args = args[0],args[1:]
> > +    
> > +    test_name, args = args[0], args[1:]
> > +    # Rebuild the deprecated arguments.
> > +    for key,val in vars(deprecated_opts).iteritems(): 
> > +        if val is not None:
> > +            args.append("--" + key)
> > +            if isinstance(val, str):
> > +                args.append(val)
> > +            logger.warning("--{} should be passed directly to the test suite.".format(key))
> 
> I don't think the logger should be used for UI level warnings, I'd just use
> lit.testing.util.commands.warning().
> 
> It probably makes sense to prepend the options to args instead of append, but
> its not super important.
> 
> >  
> >      import lnt.tests
> >      try:
> > @@ -159,50 +160,7 @@ def action_runtest(name, args):
> >      except KeyError:
> >          parser.error('invalid test name %r' % test_name)
> >  
> > -    report = test_instance.run_test('%s %s' % (name, test_name), args)
> > -
> > -    if opts.output is not None:
> > -        if opts.output == '-':
> > -            output_stream = sys.stdout
> > -        else:
> > -            output_stream = open(opts.output, 'w')
> > -        print >>output_stream, report.render()
> > -        if output_stream is not sys.stdout:
> > -            output_stream.close()
> > -
> > -    # Save the report to a temporary file.
> > -    #
> > -    # FIXME: This is silly, the underlying test probably wrote the report to a
> > -    # file itself. We need to clean this up and make it standard across all
> > -    # tests. That also has the nice side effect that writing into a local
> > -    # database records the correct imported_from path.
> > -    tmp = tempfile.NamedTemporaryFile(suffix='.json')
> > -    print >>tmp, report.render()
> > -    tmp.flush()
> > -
> > -    if opts.submit_url is not None:
> > -        if report is None:
> > -            raise SystemExit,"error: report generation failed"
> > -
> > -        from lnt.util import ServerUtil
> > -        test_instance.log("submitting result to %r" % (opts.submit_url,))
> > -        ServerUtil.submitFile(opts.submit_url, tmp.name, True, opts.verbose)
> > -    else:
> > -        # Simulate a submission to retrieve the results report.
> > -
> > -        # Construct a temporary database and import the result.
> > -        test_instance.log("submitting result to dummy instance")
> > -        
> > -        import lnt.server.db.v4db
> > -        import lnt.server.config
> > -        db = lnt.server.db.v4db.V4DB("sqlite:///:memory:",
> > -                                     lnt.server.config.Config.dummyInstance())
> > -        result = lnt.util.ImportData.import_and_report(
> > -            None, None, db, tmp.name, 'json', commit = True)
> > -        lnt.util.ImportData.print_report_result(result, sys.stdout, sys.stderr,
> > -                                                opts.verbose)
> > -
> > -    tmp.close()
> > +    test_instance.run_test('%s %s' % (name, test_name), args)
> >  
> >  def action_showtests(name, args):
> >      """show the available built-in tests"""
> > diff --git a/lnt/tests/builtintest.py b/lnt/tests/builtintest.py
> > index f937a87ede6ebeb86e92ddc888ba875fe5060b20..7bbd88390a9a22989bbdebf03011a4f99c446d49 100644
> > --- a/lnt/tests/builtintest.py
> > +++ b/lnt/tests/builtintest.py
> > @@ -3,9 +3,17 @@ Base class for builtin-in tests.
> >  """
> >  
> >  import sys
> > +import os
> >  
> >  from lnt.testing.util.misc import timestamp
> >  
> > +import lnt.util.ServerUtil as ServerUtil 
> > +import lnt.util.ImportData as ImportData
> > +
> > +import lnt.server.db.v4db
> > +import lnt.server.config
> > +
> > +
> >  class BuiltinTest(object):
> >      def __init__(self):
> >          pass
> > @@ -26,3 +34,41 @@ class BuiltinTest(object):
> >  
> >      def log(self, message, ts=timestamp()):
> >          print >>sys.stderr, '%s: %s' % (ts, message)
> > +
> > +    @staticmethod
> > +    def print_report(report, output):
> > +        """Print the report object to the output path."""
> > +        if output == '-':
> > +            output_stream = sys.stdout
> > +        else:
> > +            output_stream = open(output, 'w')
> > +        print >> output_stream, report.render()
> > +        if output_stream is not sys.stdout:
> > +            output_stream.close()
> > +
> > +    def submit(self, report_path, config):
> > +        """Submit the results file to the server.  If no server
> > +        was specified, use a local mock server.
> > +
> > +        """
> > +        assert os.path.exists(report_path), "Failed report should have never gotten here!"
> > +        result = None
> > +        if config.submit_url is not None:
> > +            self.log("submitting result to %r" % (config.submit_url,))
> > +            commit = True
> > +            result = ServerUtil.submitFile(config.submit_url, report_path, commit)
> > +        else:
> > +            # Simulate a submission to retrieve the results report.
> > +
> > +            # Construct a temporary database and import the result.
> > +            self.log("submitting result to dummy instance")
> > +
> > +            db = lnt.server.db.v4db.V4DB("sqlite:///:memory:",
> > +                                         lnt.server.config.Config.dummyInstance())
> > +            result = ImportData.import_and_report(
> > +                None, None, db, report_path, 'json', commit = True)
> > +        assert result is not None, "Results were not submitted."
> > +        ImportData.print_report_result(result, sys.stdout, sys.stderr,
> > +                                                    config.verbose)
> > +
> > +
> > diff --git a/lnt/tests/compile.py b/lnt/tests/compile.py
> > index fabf8ed64bd3a66ea7c710ddfcb25e5c696e18ff..545544056c01b2a8f490a1c05c5d1debe9632239 100644
> > --- a/lnt/tests/compile.py
> > +++ b/lnt/tests/compile.py
> > @@ -659,9 +659,6 @@ class CompileTest(builtintest.BuiltinTest):
> >          parser = OptionParser(
> >              ("%(name)s [options] [<output file>]\n" +
> >               usage_info) % locals())
> > -        parser.add_option("-v", "--verbose", dest="verbose",
> > -                          help="Show more test output",
> > -                          action="store_true", default=False)
> >          parser.add_option("-s", "--sandbox", dest="sandbox_path",
> >                            help="Parent directory to build and run tests in",
> >                            type=str, default=None, metavar="PATH")
> > @@ -743,6 +740,21 @@ class CompileTest(builtintest.BuiltinTest):
> >          group.add_option("", "--machine-name", dest="machine_name", type='str',
> >                           help="Machine name to use in submission [%default]",
> >                           action="store", default=platform.uname()[1])
> > +        group.add_option("", "--submit", dest="submit_url", metavar="URLORPATH",
> > +                          help=("autosubmit the test result to the given server "
> > +                                "(or local instance) [%default]"),
> > +                          type=str, default=None)
> > +        group.add_option("", "--commit", dest="commit",
> > +                          help=("whether the autosubmit result should be committed "
> > +                                "[%default]"),
> > +                          type=int, default=True)
> > +        group.add_option("", "--output", dest="output", metavar="PATH",
> > +                          help="write raw report data to PATH (or stdout if '-')",
> > +                          action="store", default=None)
> > +        group.add_option("-v", "--verbose", dest="verbose",
> > +                          help="show verbose test results",
> > +                          action="store_true", default=False)
> > +
> >          parser.add_option_group(group)
> >  
> >          opts,args = parser.parse_args(args)
> > @@ -999,9 +1011,15 @@ class CompileTest(builtintest.BuiltinTest):
> >          # Write out the report.
> >          lnt_report_path = os.path.join(g_output_dir, 'report.json')
> >          report = lnt.testing.Report(machine, run, testsamples)
> > -        lnt_report_file = open(lnt_report_path, 'w')
> > -        print >>lnt_report_file, report.render()
> > -        lnt_report_file.close()
> > +
> > +        # save report
> 
> Same nit as before about complete sentences and capitalization.
> 
> > +        self.print_report(report, lnt_report_path)
> > +
> > +        # and print to screen if requested
> > +        if opts.output is not None:
> > +            self.print_report(report, opts.output)
> > +
> > +        self.submit(lnt_report_path, opts)
> >  
> >          return report
> >  
> > diff --git a/lnt/tests/nt.py b/lnt/tests/nt.py
> > index 963a99e056782bec5dbf006946813b7c1c4f69e4..23820f892729a571cc3365ace4788e273cc0fa3f 100644
> > --- a/lnt/tests/nt.py
> > +++ b/lnt/tests/nt.py
> > @@ -7,6 +7,7 @@ import subprocess
> >  import sys
> >  import time
> >  import traceback
> > +import tempfile
> >  
> >  from datetime import datetime
> >  
> > @@ -1279,6 +1280,21 @@ class NTTest(builtintest.BuiltinTest):
> >                           metavar="NAME=VAL",
> >                           help="Add 'NAME' = 'VAL' to the run parameters",
> >                           type=str, action="append", default=[])
> > +        group.add_option("", "--submit", dest="submit_url", metavar="URLORPATH",
> > +                          help=("autosubmit the test result to the given server "
> > +                                "(or local instance) [%default]"),
> > +                          type=str, default=None)
> > +        group.add_option("", "--commit", dest="commit",
> > +                          help=("whether the autosubmit result should be committed "
> > +                                "[%default]"),
> > +                          type=int, default=True)
> > +        group.add_option("", "--output", dest="output", metavar="PATH",
> > +                          help="write raw report data to PATH (or stdout if '-')",
> > +                          action="store", default=None)
> > +        group.add_option("-v", "--verbose", dest="verbose",
> > +                          help="show verbose test results",
> > +                          action="store_true", default=False)
> > +
> >          parser.add_option_group(group)
> >  
> >          (opts, args) = parser.parse_args(args)
> > @@ -1432,10 +1448,9 @@ class NTTest(builtintest.BuiltinTest):
> >                  warning('expected --isysroot when executing with '
> >                          '--ios-simulator-sdk')
> >  
> > +        config = TestConfiguration(vars(opts), timestamp())
> >          # FIXME: We need to validate that there is no configured output in the
> >          # test-suite directory, that borks things. <rdar://problem/7876418>
> > -        options = dict(vars(opts).items())
> > -        config = TestConfiguration(options, timestamp())
> >          prepare_report_dir(config)
> >  
> >          # Multisample, if requested.
> > @@ -1461,15 +1476,20 @@ class NTTest(builtintest.BuiltinTest):
> >                                  for r in reports], [])
> >  
> >              # Write out the merged report.
> > -            lnt_report_path = os.path.join(config.report_dir, 'report.json')
> > +            lnt_report_path = config.report_path(None)
> >              report = lnt.testing.Report(machine, run, test_samples)
> >              lnt_report_file = open(lnt_report_path, 'w')
> >              print >>lnt_report_file,report.render()
> >              lnt_report_file.close()
> >  
> > -            return report
> > +        else:
> > +            report = run_test(nick, None, config)
> > +
> > +        if config.output is not None:
> > +            self.print_report(report, config.output)
> > +
> > +        self.submit(config.report_path(None), config)
> >  
> > -        report = run_test(nick, None, config)
> >          return report
> >  
> >  def create_instance():
> 
>  - Daniel
> 
> 
> On Fri, Oct 4, 2013 at 11:59 AM, Chris Matthews <chris.matthews at apple.com> wrote:
> Here is an updated patch.  Submit is moved to a superclass, and arguments are moved into the test suites themselves.  Arguments passed to runtest print deprecated warnings then are appended back on to the test suiteā€™s argument list.
> 
> 
> diff --git a/lnt/lnttool/main.py b/lnt/lnttool/main.py
> index 3fb96e5a4eb4879fcccdf9fc9364c7feea22e620..f140f209951a520d3a6125b9ba54335f4384f287 100644
> --- a/lnt/lnttool/main.py
> +++ b/lnt/lnttool/main.py
> @@ -130,28 +130,29 @@ def action_checkformat(name, args):
>  def action_runtest(name, args):
>      """run a builtin test application"""
>  
> +    # Runtest accepting options is deprecated, but lets not break the
> +    # world, so collect them anyways and pass them on.
>      parser = OptionParser("%s test-name [options]" % name)
>      parser.disable_interspersed_args()
> -    parser.add_option("", "--submit", dest="submit_url", metavar="URLORPATH",
> -                      help=("autosubmit the test result to the given server "
> -                            "(or local instance) [%default]"),
> -                      type=str, default=None)
> -    parser.add_option("", "--commit", dest="commit",
> -                      help=("whether the autosubmit result should be committed "
> -                            "[%default]"),
> -                      type=int, default=True)
> -    parser.add_option("", "--output", dest="output", metavar="PATH",
> -                      help="write raw report data to PATH (or stdout if '-')",
> -                      action="store", default=None)
> -    parser.add_option("-v", "--verbose", dest="verbose",
> -                      help="show verbose test results",
> -                      action="store_true", default=False)
> +    parser.add_option("", "--submit", dest="submit", type=str, default=None)
> +    parser.add_option("", "--commit", dest="commit", type=str, default=None)
> +    parser.add_option("", "--output", dest="output", type=str, default=None)
> +    parser.add_option("-v", "--verbose", dest="verbose", default=None)
>  
> -    (opts, args) = parser.parse_args(args)
> +    logger, _ = setup_logging()
> +
> +    (deprecated_opts, args) = parser.parse_args(args)
>      if len(args) < 1:
>          parser.error("incorrect number of argments")
> -
> -    test_name,args = args[0],args[1:]
> +    
> +    test_name, args = args[0], args[1:]
> +    # Rebuild the deprecated arguments.
> +    for key,val in vars(deprecated_opts).iteritems(): 
> +        if val is not None:
> +            args.append("--" + key)
> +            if isinstance(val, str):
> +                args.append(val)
> +            logger.warning("--{} should be passed directly to the test suite.".format(key))
>  
>      import lnt.tests
>      try:
> @@ -159,50 +160,7 @@ def action_runtest(name, args):
>      except KeyError:
>          parser.error('invalid test name %r' % test_name)
>  
> -    report = test_instance.run_test('%s %s' % (name, test_name), args)
> -
> -    if opts.output is not None:
> -        if opts.output == '-':
> -            output_stream = sys.stdout
> -        else:
> -            output_stream = open(opts.output, 'w')
> -        print >>output_stream, report.render()
> -        if output_stream is not sys.stdout:
> -            output_stream.close()
> -
> -    # Save the report to a temporary file.
> -    #
> -    # FIXME: This is silly, the underlying test probably wrote the report to a
> -    # file itself. We need to clean this up and make it standard across all
> -    # tests. That also has the nice side effect that writing into a local
> -    # database records the correct imported_from path.
> -    tmp = tempfile.NamedTemporaryFile(suffix='.json')
> -    print >>tmp, report.render()
> -    tmp.flush()
> -
> -    if opts.submit_url is not None:
> -        if report is None:
> -            raise SystemExit,"error: report generation failed"
> -
> -        from lnt.util import ServerUtil
> -        test_instance.log("submitting result to %r" % (opts.submit_url,))
> -        ServerUtil.submitFile(opts.submit_url, tmp.name, True, opts.verbose)
> -    else:
> -        # Simulate a submission to retrieve the results report.
> -
> -        # Construct a temporary database and import the result.
> -        test_instance.log("submitting result to dummy instance")
> -        
> -        import lnt.server.db.v4db
> -        import lnt.server.config
> -        db = lnt.server.db.v4db.V4DB("sqlite:///:memory:",
> -                                     lnt.server.config.Config.dummyInstance())
> -        result = lnt.util.ImportData.import_and_report(
> -            None, None, db, tmp.name, 'json', commit = True)
> -        lnt.util.ImportData.print_report_result(result, sys.stdout, sys.stderr,
> -                                                opts.verbose)
> -
> -    tmp.close()
> +    test_instance.run_test('%s %s' % (name, test_name), args)
>  
>  def action_showtests(name, args):
>      """show the available built-in tests"""
> diff --git a/lnt/tests/builtintest.py b/lnt/tests/builtintest.py
> index f937a87ede6ebeb86e92ddc888ba875fe5060b20..7bbd88390a9a22989bbdebf03011a4f99c446d49 100644
> --- a/lnt/tests/builtintest.py
> +++ b/lnt/tests/builtintest.py
> @@ -3,9 +3,17 @@ Base class for builtin-in tests.
>  """
>  
>  import sys
> +import os
>  
>  from lnt.testing.util.misc import timestamp
>  
> +import lnt.util.ServerUtil as ServerUtil 
> +import lnt.util.ImportData as ImportData
> +
> +import lnt.server.db.v4db
> +import lnt.server.config
> +
> +
>  class BuiltinTest(object):
>      def __init__(self):
>          pass
> @@ -26,3 +34,41 @@ class BuiltinTest(object):
>  
>      def log(self, message, ts=timestamp()):
>          print >>sys.stderr, '%s: %s' % (ts, message)
> +
> +    @staticmethod
> +    def print_report(report, output):
> +        """Print the report object to the output path."""
> +        if output == '-':
> +            output_stream = sys.stdout
> +        else:
> +            output_stream = open(output, 'w')
> +        print >> output_stream, report.render()
> +        if output_stream is not sys.stdout:
> +            output_stream.close()
> +
> +    def submit(self, report_path, config):
> +        """Submit the results file to the server.  If no server
> +        was specified, use a local mock server.
> +
> +        """
> +        assert os.path.exists(report_path), "Failed report should have never gotten here!"
> +        result = None
> +        if config.submit_url is not None:
> +            self.log("submitting result to %r" % (config.submit_url,))
> +            commit = True
> +            result = ServerUtil.submitFile(config.submit_url, report_path, commit)
> +        else:
> +            # Simulate a submission to retrieve the results report.
> +
> +            # Construct a temporary database and import the result.
> +            self.log("submitting result to dummy instance")
> +
> +            db = lnt.server.db.v4db.V4DB("sqlite:///:memory:",
> +                                         lnt.server.config.Config.dummyInstance())
> +            result = ImportData.import_and_report(
> +                None, None, db, report_path, 'json', commit = True)
> +        assert result is not None, "Results were not submitted."
> +        ImportData.print_report_result(result, sys.stdout, sys.stderr,
> +                                                    config.verbose)
> +
> +
> diff --git a/lnt/tests/compile.py b/lnt/tests/compile.py
> index fabf8ed64bd3a66ea7c710ddfcb25e5c696e18ff..545544056c01b2a8f490a1c05c5d1debe9632239 100644
> --- a/lnt/tests/compile.py
> +++ b/lnt/tests/compile.py
> @@ -659,9 +659,6 @@ class CompileTest(builtintest.BuiltinTest):
>          parser = OptionParser(
>              ("%(name)s [options] [<output file>]\n" +
>               usage_info) % locals())
> -        parser.add_option("-v", "--verbose", dest="verbose",
> -                          help="Show more test output",
> -                          action="store_true", default=False)
>          parser.add_option("-s", "--sandbox", dest="sandbox_path",
>                            help="Parent directory to build and run tests in",
>                            type=str, default=None, metavar="PATH")
> @@ -743,6 +740,21 @@ class CompileTest(builtintest.BuiltinTest):
>          group.add_option("", "--machine-name", dest="machine_name", type='str',
>                           help="Machine name to use in submission [%default]",
>                           action="store", default=platform.uname()[1])
> +        group.add_option("", "--submit", dest="submit_url", metavar="URLORPATH",
> +                          help=("autosubmit the test result to the given server "
> +                                "(or local instance) [%default]"),
> +                          type=str, default=None)
> +        group.add_option("", "--commit", dest="commit",
> +                          help=("whether the autosubmit result should be committed "
> +                                "[%default]"),
> +                          type=int, default=True)
> +        group.add_option("", "--output", dest="output", metavar="PATH",
> +                          help="write raw report data to PATH (or stdout if '-')",
> +                          action="store", default=None)
> +        group.add_option("-v", "--verbose", dest="verbose",
> +                          help="show verbose test results",
> +                          action="store_true", default=False)
> +
>          parser.add_option_group(group)
>  
>          opts,args = parser.parse_args(args)
> @@ -999,9 +1011,15 @@ class CompileTest(builtintest.BuiltinTest):
>          # Write out the report.
>          lnt_report_path = os.path.join(g_output_dir, 'report.json')
>          report = lnt.testing.Report(machine, run, testsamples)
> -        lnt_report_file = open(lnt_report_path, 'w')
> -        print >>lnt_report_file, report.render()
> -        lnt_report_file.close()
> +
> +        # save report
> +        self.print_report(report, lnt_report_path)
> +
> +        # and print to screen if requested
> +        if opts.output is not None:
> +            self.print_report(report, opts.output)
> +
> +        self.submit(lnt_report_path, opts)
>  
>          return report
>  
> diff --git a/lnt/tests/nt.py b/lnt/tests/nt.py
> index 963a99e056782bec5dbf006946813b7c1c4f69e4..23820f892729a571cc3365ace4788e273cc0fa3f 100644
> --- a/lnt/tests/nt.py
> +++ b/lnt/tests/nt.py
> @@ -7,6 +7,7 @@ import subprocess
>  import sys
>  import time
>  import traceback
> +import tempfile
>  
>  from datetime import datetime
>  
> @@ -1279,6 +1280,21 @@ class NTTest(builtintest.BuiltinTest):
>                           metavar="NAME=VAL",
>                           help="Add 'NAME' = 'VAL' to the run parameters",
>                           type=str, action="append", default=[])
> +        group.add_option("", "--submit", dest="submit_url", metavar="URLORPATH",
> +                          help=("autosubmit the test result to the given server "
> +                                "(or local instance) [%default]"),
> +                          type=str, default=None)
> +        group.add_option("", "--commit", dest="commit",
> +                          help=("whether the autosubmit result should be committed "
> +                                "[%default]"),
> +                          type=int, default=True)
> +        group.add_option("", "--output", dest="output", metavar="PATH",
> +                          help="write raw report data to PATH (or stdout if '-')",
> +                          action="store", default=None)
> +        group.add_option("-v", "--verbose", dest="verbose",
> +                          help="show verbose test results",
> +                          action="store_true", default=False)
> +
>          parser.add_option_group(group)
>  
>          (opts, args) = parser.parse_args(args)
> @@ -1432,10 +1448,9 @@ class NTTest(builtintest.BuiltinTest):
>                  warning('expected --isysroot when executing with '
>                          '--ios-simulator-sdk')
>  
> +        config = TestConfiguration(vars(opts), timestamp())
>          # FIXME: We need to validate that there is no configured output in the
>          # test-suite directory, that borks things. <rdar://problem/7876418>
> -        options = dict(vars(opts).items())
> -        config = TestConfiguration(options, timestamp())
>          prepare_report_dir(config)
>  
>          # Multisample, if requested.
> @@ -1461,15 +1476,20 @@ class NTTest(builtintest.BuiltinTest):
>                                  for r in reports], [])
>  
>              # Write out the merged report.
> -            lnt_report_path = os.path.join(config.report_dir, 'report.json')
> +            lnt_report_path = config.report_path(None)
>              report = lnt.testing.Report(machine, run, test_samples)
>              lnt_report_file = open(lnt_report_path, 'w')
>              print >>lnt_report_file,report.render()
>              lnt_report_file.close()
>  
> -            return report
> +        else:
> +            report = run_test(nick, None, config)
> +
> +        if config.output is not None:
> +            self.print_report(report, config.output)
> +
> +        self.submit(config.report_path(None), config)
>  
> -        report = run_test(nick, None, config)
>          return report
>  
>  def create_instance():
> 
> 
> 
> 
> 
> Chris Matthews
> chris.matthews at apple.com
> 
> On Sep 13, 2013, at 8:32 AM, Daniel Dunbar <daniel at zuster.org> wrote:
> 
>> On Mon, Sep 9, 2013 at 10:16 AM, Chris Matthews <chris.matthews at apple.com> wrote:
>> In an effort refactor the code to make way for run resampling, move the run submission code into the test suites. With resampling the submission logic will change in NT.
>> 
>> diff --git a/lnt/lnttool/main.py b/lnt/lnttool/main.py
>> index 2b667bfb97a67d9316f82908f91773073240cf5c..bc521d4baeec3cfadf72ff94cab114889dbf929d 100644
>> --- a/lnt/lnttool/main.py
>> +++ b/lnt/lnttool/main.py
>> @@ -139,11 +139,11 @@ def action_runtest(name, args):
>>                        help="show verbose test results",
>>                        action="store_true", default=False)
>>  
>> -    (opts, args) = parser.parse_args(args)
>> +    (global_opts, args) = parser.parse_args(args)
>>      if len(args) < 1:
>>          parser.error("incorrect number of argments")
>>  
>> -    test_name,args = args[0],args[1:]
>> +    test_name, args = args[0], args[1:]
>>  
>>      import lnt.tests
>>      try:
>> @@ -151,50 +151,7 @@ def action_runtest(name, args):
>>      except KeyError:
>>          parser.error('invalid test name %r' % test_name)
>>  
>> -    report = test_instance.run_test('%s %s' % (name, test_name), args)
>> -
>> -    if opts.output is not None:
>> -        if opts.output == '-':
>> -            output_stream = sys.stdout
>> -        else:
>> -            output_stream = open(opts.output, 'w')
>> -        print >>output_stream, report.render()
>> -        if output_stream is not sys.stdout:
>> -            output_stream.close()
>> -
>> -    # Save the report to a temporary file.
>> -    #
>> -    # FIXME: This is silly, the underlying test probably wrote the report to a
>> -    # file itself. We need to clean this up and make it standard across all
>> -    # tests. That also has the nice side effect that writing into a local
>> -    # database records the correct imported_from path.
>> -    tmp = tempfile.NamedTemporaryFile(suffix='.json')
>> -    print >>tmp, report.render()
>> -    tmp.flush()
>> -
>> -    if opts.submit_url is not None:
>> -        if report is None:
>> -            raise SystemExit,"error: report generation failed"
>> -
>> -        from lnt.util import ServerUtil
>> -        test_instance.log("submitting result to %r" % (opts.submit_url,))
>> -        ServerUtil.submitFile(opts.submit_url, tmp.name, True, opts.verbose)
>> -    else:
>> -        # Simulate a submission to retrieve the results report.
>> -
>> -        # Construct a temporary database and import the result.
>> -        test_instance.log("submitting result to dummy instance")
>> -        
>> -        import lnt.server.db.v4db
>> -        import lnt.server.config
>> -        db = lnt.server.db.v4db.V4DB("sqlite:///:memory:",
>> -                                     lnt.server.config.Config.dummyInstance())
>> -        result = lnt.util.ImportData.import_and_report(
>> -            None, None, db, tmp.name, 'json', commit = True)
>> -        lnt.util.ImportData.print_report_result(result, sys.stdout, sys.stderr,
>> -                                                opts.verbose)
>> -
>> -    tmp.close()
>> +    test_instance.run_test('%s %s' % (name, test_name), args, global_opts)
>> 
>> I would do this slightly differently, I would just deprecate the syntax of passing options to 'lnt runtest', and make the individual tests duplicate the submit options. Then for backwards compatibility you can just have 'lnt runtest' prepend synthesized command line arguments to the args array (maybe with a note about that being deprecated).
>>  
>>  
>>  def action_showtests(name, args):
>>      """show the available built-in tests"""
>> diff --git a/lnt/tests/builtintest.py b/lnt/tests/builtintest.py
>> index f937a87ede6ebeb86e92ddc888ba875fe5060b20..85a970f67070b3f4e03ee968b80f92264e797747 100644
>> --- a/lnt/tests/builtintest.py
>> +++ b/lnt/tests/builtintest.py
>> @@ -26,3 +26,14 @@ class BuiltinTest(object):
>>  
>>      def log(self, message, ts=timestamp()):
>>          print >>sys.stderr, '%s: %s' % (ts, message)
>> +
>> +    @staticmethod
>> +    def print_report(report, output):
>> +        """Print the report object to the output path."""
>> +        if output == '-':
>> +            output_stream = sys.stdout
>> +        else:
>> +            output_stream = open(output, 'w')
>> +        print >> output_stream, report.render()
>> +        if output_stream is not sys.stdout:
>> +            output_stream.close()
>> diff --git a/lnt/tests/compile.py b/lnt/tests/compile.py
>> index fabf8ed64bd3a66ea7c710ddfcb25e5c696e18ff..77a235da7fcfb55b558ef246ef71acf5d1531d94 100644
>> --- a/lnt/tests/compile.py
>> +++ b/lnt/tests/compile.py
>> @@ -654,7 +654,7 @@ class CompileTest(builtintest.BuiltinTest):
>>      def describe(self):
>>          return 'Single file compile-time performance testing'
>>  
>> -    def run_test(self, name, args):
>> +    def run_test(self, name, args, global_opts):
>>          global opts
>>          parser = OptionParser(
>>              ("%(name)s [options] [<output file>]\n" +
>> @@ -999,12 +999,46 @@ class CompileTest(builtintest.BuiltinTest):
>>          # Write out the report.
>>          lnt_report_path = os.path.join(g_output_dir, 'report.json')
>>          report = lnt.testing.Report(machine, run, testsamples)
>> -        lnt_report_file = open(lnt_report_path, 'w')
>> -        print >>lnt_report_file, report.render()
>> -        lnt_report_file.close()
>> +
>> +        # save report
>> +        self.print_report(report, lnt_report_path)
>> +
>> +        # and print to screen if requested
>> 
>> nit: Complete sentences in comments, please.
>> 
>> +        if global_opts.output is not None:
>> +            self.print_report(report, global_opts.output)
>> +
>> +        self.submit(lnt_report_path, global_opts)
>>  
>>          return report
>>  
>> +    def submit(self, report_path, config):
>> +        """Submit the results file to the server.  If no server
>> +        was specified, use a local mock server.
>> +
>> +        """
>> +        assert os.path.exists(report_path), "Failed report should have never gotten here!"
>> +
>> +        if config.submit_url is not None:
>> +
>> +            from lnt.util import ServerUtil
>> +            self.log("submitting result to %r" % (config.submit_url,))
>> +            ServerUtil.submitFile(config.submit_url, report_path, True, config.verbose)
>> +        else:
>> +            # Simulate a submission to retrieve the results report.
>> +
>> +            # Construct a temporary database and import the result.
>> +            self.log("submitting result to dummy instance")
>> +
>> +            import lnt.server.db.v4db
>> +            import lnt.server.config
>> +            db = lnt.server.db.v4db.V4DB("sqlite:///:memory:",
>> +                                         lnt.server.config.Config.dummyInstance())
>> +            result = lnt.util.ImportData.import_and_report(
>> +                None, None, db, report_path, 'json', commit = True)
>> +            lnt.util.ImportData.print_report_result(result, sys.stdout, sys.stderr,
>> +                                                    config.verbose)
>> +
>> +
>>  def create_instance():
>>      return CompileTest()
>>  
>> diff --git a/lnt/tests/nt.py b/lnt/tests/nt.py
>> index 963a99e056782bec5dbf006946813b7c1c4f69e4..cefb38c42c7c05cc24df76bc263e93cb50b7b09b 100644
>> --- a/lnt/tests/nt.py
>> +++ b/lnt/tests/nt.py
>> @@ -7,6 +7,7 @@ import subprocess
>>  import sys
>>  import time
>>  import traceback
>> +import tempfile
>>  
>>  from datetime import datetime
>>  
>> @@ -1070,7 +1071,7 @@ class NTTest(builtintest.BuiltinTest):
>>      def describe(self):
>>          return 'LLVM test-suite compile and execution tests'
>>  
>> -    def run_test(self, name, args):
>> +    def run_test(self, name, args, global_opts):
>>          parser = OptionParser(
>>              ("%(name)s [options] tester-name\n" + usage_info) % locals())
>>  
>> @@ -1432,10 +1433,10 @@ class NTTest(builtintest.BuiltinTest):
>>                  warning('expected --isysroot when executing with '
>>                          '--ios-simulator-sdk')
>>  
>> +        options = dict(vars(opts).items() + vars(global_opts).items())
>> +        config = TestConfiguration(options, timestamp())
>>          # FIXME: We need to validate that there is no configured output in the
>>          # test-suite directory, that borks things. <rdar://problem/7876418>
>> -        options = dict(vars(opts).items())
>> -        config = TestConfiguration(options, timestamp())
>>          prepare_report_dir(config)
>>  
>>          # Multisample, if requested.
>> @@ -1461,17 +1462,51 @@ class NTTest(builtintest.BuiltinTest):
>>                                  for r in reports], [])
>>  
>>              # Write out the merged report.
>> -            lnt_report_path = os.path.join(config.report_dir, 'report.json')
>> +            lnt_report_path = config.report_path(None)
>>              report = lnt.testing.Report(machine, run, test_samples)
>>              lnt_report_file = open(lnt_report_path, 'w')
>>              print >>lnt_report_file,report.render()
>>              lnt_report_file.close()
>>  
>> -            return report
>> +        else:
>> +            report = run_test(nick, None, config)
>> +
>> +        if config.output is not None:
>> +            self.print_report(report, config.output)
>> +
>> +        self.submit(config)
>>  
>> -        report = run_test(nick, None, config)
>>          return report
>>  
>> +
>> +    def submit(self, config):
>> +        """Submit the report to the server.  If no server
>> +        was specified, use a local mock server.
>> +
>> +        """
>> +        report_path = config.report_path(None)
>> +        assert os.path.exists(report_path), "Failed report should have never gotten here!"
>> +
>> +        if config.submit_url is not None:
>> +            from lnt.util import ServerUtil
>> +            self.log("submitting result to %r" % (config.submit_url,))
>> +            ServerUtil.submitFile(config.submit_url, report_path, True, config.verbose)
>> +        else:
>> +            # Simulate a submission to retrieve the results report.
>> +
>> +            # Construct a temporary database and import the result.
>> +            self.log("submitting result to dummy instance")
>> +
>> +            import lnt.server.db.v4db
>> +            import lnt.server.config
>> +            db = lnt.server.db.v4db.V4DB("sqlite:///:memory:",
>> +                                         lnt.server.config.Config.dummyInstance())
>> +            result = lnt.util.ImportData.import_and_report(
>> +                None, None, db, report_path, 'json', commit = True)
>> +            lnt.util.ImportData.print_report_result(result, sys.stdout, sys.stderr,
>> +                                                    config.verbose)
>> 
>> Isn't there a way to factor this to just be a method in the base class that both implementations use?
>> 
>>  - Daniel
>>  
>> +
>> +
>>  def create_instance():
>>      return NTTest()
>>  
>> 
>> 
>> 
>> 
>> 
>> 
>> Chris Matthews
>> chris.matthews at apple.com
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>> 
>> 
> 
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20131202/aff79c61/attachment.html>


More information about the llvm-commits mailing list