Merge lp://staging/~veebers/autopilot/texttest-run-uprefresh into lp://staging/autopilot

Proposed by Christopher Lee
Status: Needs review
Proposed branch: lp://staging/~veebers/autopilot/texttest-run-uprefresh
Merge into: lp://staging/autopilot
Diff against target: 848 lines (+351/-130)
7 files modified
autopilot/__init__.py (+1/-1)
autopilot/run.py (+21/-19)
autopilot/testresult.py (+85/-22)
autopilot/tests/functional/test_autopilot_functional.py (+87/-54)
autopilot/tests/unit/test_command_line_args.py (+10/-2)
autopilot/tests/unit/test_run.py (+127/-30)
autopilot/tests/unit/test_testresults.py (+20/-2)
To merge this branch: bzr merge lp://staging/~veebers/autopilot/texttest-run-uprefresh
Reviewer Review Type Date Requested Status
platform-qa-bot continuous-integration Approve
PS Jenkins bot continuous-integration Needs Fixing
prod-platform-qa continuous-integration Pending
Christopher Lee Pending
Thomi Richards Pending
Corey Goldberg Pending
Review via email: mp+225247@code.staging.launchpad.net

This proposal supersedes a proposal from 2014-03-03.

Commit message

Improve non-verbose output to console during tests

Description of the change

(Resubmitted w/ merged trunk + conflict fixes.)

This branch improves the non-verbose output, to get similar output as unittest runner.
It prints the dots/flags as tests run. so it's not just silent when running in normal (non-verbose) mode.

this branch also bumps the version of Python used in tox.ini configuration for testing to: Python3.4

To post a comment you must log in.
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Corey Goldberg (coreygoldberg) wrote : Posted in a previous version of this proposal

from irc with thomi:

<thomi> you should flush stdout afdter startTest, since we want to see that output in the case where a test takes a long time to run
<cgoldberg> right
<thomi> also, I think the skip line should have the optional message in '( )'
 so like:
 test.id ... SKIP (Not runnable on the device)
 for example
<cgoldberg> yup.. agree
<thomi> also, I think the status messages can be a bit more verbose.
 particularly, please change:
 XFAIL => EXPECTED FAIL
 and
 NOTOK => UNEXPECTED SUCCESS
<cgoldberg> right. makes sense. I copied the unittest runner's output, but there's no reason not to be a little more explicit
<thomi> also, you should generalise the code (diff lines 82-92) to go in a separate function, something like _wrap_result_with_output_decorator
 which accepts a result object, and wraps it in either a LoggingResultDecorator or your new decorator
 that way, it's easier to test, and we keep the complexity of the construct_XXX functions down
<thomi> cgoldberg: in your tests, I think you can make your 'assertOutput' lines more readable, by doing:
<cgoldberg> thomi, ok.. i can wrap it in a function
<thomi> self.assertOutput('{id} ... OK\n', 'pass')
 one line is better than 3, since we read left->right
 other than those minor quibbles above ^^, this looks great, but you're missing some tests still
<cgoldberg> thomi, hah.. will do
<thomi> I'd like to see an integration test that shows that when you specify the various result formats without the verbose flag, we get the correct result object & wrapper
 similarly for when you do specify the verbose flag
<cgoldberg> gotcha.. yea i can add that too
<thomi> Finally, I think you should add 6 functional tests to the functional test suite
 1 test fro each format, * 1 with verbose, and one without verbose
 actually, the verbose case is almost certainly covered already (but please do check)
 so maybe it's only 3 new functional tests :)

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Corey Goldberg (coreygoldberg) wrote : Posted in a previous version of this proposal

from thomi:

OK, so we need to change things. Seems like there's several things we're trying to control independantly of each other here:

1) The level of verbosity autopilot uses when logging. Ideally we'd make sure that all our log levels were sensible, and then this would control the logging framework verbosity level. 0 * '-v' => normal and higher. '-v' => info logs and higher. '-vv' => debug logs and higher. The log *always* gets attached to the test result for every test, regardless of the output file format.

2) The format we store run reports in. We have 'text', 'xml', 'subunit'. I think we need one more, which is 'verbosetext' or something similar. This causes the test log to get printed to stdoud as the test is runing. This is equivilent to the '-v' flag before this change.

3) The location of the output report. If the -o option is specified, the report is written to that file, in the format specified by the '-f' parameter.

Finally, if, after configuring all this, stdout is not used, the 'status' format should be printed to stdout as well.

review: Needs Fixing
Revision history for this message
Corey Goldberg (coreygoldberg) wrote : Posted in a previous version of this proposal

Four output formats will be:
* status (new format)
* xml
* subunit
* text
Every format can be sent to stdout or a file.
-v / -vv flag ONLY controls the verbosity of the python log.
make subclass of TextTestResult to print to stream

Revision history for this message
Corey Goldberg (coreygoldberg) wrote : Posted in a previous version of this proposal

make above fixes

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Christopher Lee (veebers) wrote : Posted in a previous version of this proposal

Just a couple of minor things.

Initially I thought I misunderstood the verbose/output changes until I read the comment re: verbose output only going to a log file. I think that the -h output needs to be updated to state as such.
(actually I think I'm confused on this point now).

The help stats that the default format for -f is text where it is now 'status' (I was initially a little confused why my -v wasn't outputting the tapping details etc.).

line 488-491 has some odd indentation. I would put the opening [ on a new line i.e.
  self.run_autopilot(
      [ "run", "--failfast", "-f", "text", "tests"]
  )
# You could put the list args on new lines too if you wanted, but not needed here.

That's all for now, I'll try give a more functional review later on :-)

review: Needs Fixing
Revision history for this message
Corey Goldberg (coreygoldberg) wrote : Posted in a previous version of this proposal

thanks veebers!

> I think that the -h output needs to be updated to state as such.
> (actually I think I'm confused on this point now).

I will update -h commandline help, and /docs today with clear explanation of log and output handling.

> line 488-491 has some odd indentation.

I just pushed a fix for indentation in functional tests.

Revision history for this message
Corey Goldberg (coreygoldberg) wrote : Posted in a previous version of this proposal

updated help strings.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Approve (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Thomi Richards (thomir-deactivatedaccount) wrote : Posted in a previous version of this proposal

Hi,

A few things:

40 - '-v', '--verbose', required=False, default=False, action='count',
41 - help="Show autopilot log messages. Set twice to also log data "
42 - "useful for debugging autopilot itself.")
43 + "-v", "--verbosity", action='count', default=0, required=False,
44 + help="Increase verbosity of test details and 'text' mode output. "
45 + "Set twice to also log data useful for debugging autopilot "
46 + "itself.")

This is incorrect, since this is for the vis tool. The original message was correct, I don't think you need to change it at all.

54 - '-v', '--verbose', required=False, default=False, action='count',
55 - help="Show autopilot log messages. Set twice to also log data useful "
56 - "for debugging autopilot itself.")
57 + "-v", "--verbosity", action='count', default=0, required=False,
58 + help="Increase verbosity of test details and 'text' mode output. "
59 + "Set twice to also log data useful for debugging autopilot "
60 + "itself.")

Same here - this is for the launch command.

A small thing, but this:

85 -# Copyright (C) 2012-2013 Canonical
86 +# Copyright (C) 2012,2013,2014 Canonical

Should be "2012-2014", not "2012,2013,2014", as per legal advice.

102 from autopilot import get_version_string, parse_arguments
103 -import autopilot.globals
104 from autopilot._debug import get_all_debug_profiles
105 -from autopilot.testresult import get_output_formats
106 -from autopilot.utilities import DebugLogFilter, LogFormatter
107 from autopilot.application._launcher import (
108 _get_app_env_from_string_hint,
109 get_application_launcher_wrapper,
110 launch_process,
111 )
112 +import autopilot.globals
113 +from autopilot.testresult import get_output_formats
114 +from autopilot.utilities import DebugLogFilter, LogFormatter

This isn't the standard we've set in the AP codebase. We mix 'import FOO' and 'from FOO import BAR' statements in one block, and alphabeticise the module names.

565 + def test_failfast_text_mode(self):

Please move the test_failfast test to a new class, and use scenarios to make sure that *all* test result formats support failfast, not just text and status.

Here's the big change I think we need to make:

Currently, the logger is set up to log to the same stream as the result object. This works for the text format, but not the others. Instead, we should make it so the test log is added to the test result as a detail, for every format. This will help us with the subunit format, for example. TBH, I thought this already happend, but I can't see the code now. Feel free to point it out to me :)

Cheers

review: Needs Fixing
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote : Posted in a previous version of this proposal
review: Needs Fixing (continuous-integration)
Revision history for this message
Corey Goldberg (coreygoldberg) wrote : Posted in a previous version of this proposal

fixed:

- fixed copyright date format.
- reverted help text for -v arg in launch and vis modes
- added scenaro tests for failfast to cover all output formatsbz

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
483. By Christopher Lee

Fix introduced flake8 issues

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :

FAILED: Continuous integration, rev:483
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~veebers/autopilot/texttest-run-uprefresh/+merge/225247/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/autopilot-ci/751/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-utopic-amd64-ci/25
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-utopic-amd64-ci/25/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-utopic-armhf-ci/25
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-utopic-armhf-ci/25/artifact/work/output/*zip*/output.zip
    SUCCESS: http://jenkins.qa.ubuntu.com/job/autopilot-utopic-i386-ci/25
        deb: http://jenkins.qa.ubuntu.com/job/autopilot-utopic-i386-ci/25/artifact/work/output/*zip*/output.zip
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-utopic-autopilot/151
    UNSTABLE: http://jenkins.qa.ubuntu.com/job/autopilot-testrunner-otto-utopic-autopilot/238
    SUCCESS: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1459
        deb: http://jenkins.qa.ubuntu.com/job/generic-mediumtests-builder-utopic-amd64/1459/artifact/work/output/*zip*/output.zip

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/autopilot-ci/751/rebuild

review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
484. By Christopher Lee

Fix 3 failing tests with typo change

485. By Christopher Lee

Fix sp error in test (verbose vs verbosity)

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
486. By Christopher Lee

Merge trunk + fix conflicts

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Needs Fixing (continuous-integration)
Revision history for this message
platform-qa-bot (platform-qa-bot) wrote :
review: Approve (continuous-integration)

Unmerged revisions

486. By Christopher Lee

Merge trunk + fix conflicts

485. By Christopher Lee

Fix sp error in test (verbose vs verbosity)

484. By Christopher Lee

Fix 3 failing tests with typo change

483. By Christopher Lee

Fix introduced flake8 issues

482. By Christopher Lee

Merge trunk + fix conflicts

481. By Corey Goldberg

fixed comments

480. By Corey Goldberg

flake8 fixes

479. By Corey Goldberg

details unit tests

478. By Corey Goldberg

fixes

477. By Corey Goldberg

fixes per latest review

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches