Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use subunit-trace #72

Merged
merged 8 commits into from
Jul 28, 2017
Merged

Use subunit-trace #72

merged 8 commits into from
Jul 28, 2017

Conversation

mtreinish
Copy link
Owner

This adds subunit-trace output filtering to stestr. The code is copied directly from the os-testr project [1]. This enables subunit-trace by default on the run and last commands, and enables it as an option on the load command. Right now most of the subunit-trace output options are hard coded, with the only exception being colorized output which there is a config flag for. In the future we might add support for more customized output.

One thing to note is that to avoid a potential namespace collision this does not add the console_script entrypoint for the subunit-trace command. If/when os-testr switches to using stestr we might be able to figure out a deprecation path in os-testr and remove the forked copy of subunit-trace.

Closes Issue #22

[1] http://git.openstack.org/cgit/openstack/os-testr

@mtreinish mtreinish requested a review from masayukig July 27, 2017 17:15
@mtreinish mtreinish changed the title Use subunit trace Use subunit-trace Jul 27, 2017
This commit copies over the subunit-trace and colorizer (it's only in
repo dependency) from the os-testr project. [1] These modules will be
used as the new default output filter from stestr, but to avoid a
potential circular dependency (the long term plan is to switch os-testr
to use stestr underneath) we need the subunit-trace modules to live
locally.

[1] http://git.openstack.org/cgit/openstack/os-testr
Copy link
Collaborator

@masayukig masayukig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM, but I found a bug

output_result = testtools.StreamResultRouter(output_result)
cat = subunit.test_results.CatFiles(sys.stdout)
output_result.add_rule(cat, 'test_id', test_id=None)
start_time = datetime.datetime.utcnow()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This causes an error[1] when pretty_out is True(--no-subunit-trace specified).
[1] UnboundLocalError: local variable 'start_time' referenced before assignment

Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, start time isn't defined in the case pretty_out is false. So when L157 is run there is no start time defined. I should have put start time definition out one indent level and to L144. I'll respin it right now

This commit switches the default output filter to be subunit trace. This
is configurable and if people desire they can either use the old output
format, or straight subunit. But, the subunit-trace output is a lot more
useful by default.
We had to port the colorizer output modules when we migrated
subunit-trace from os-testr. This commit puts it to use by adding cli
options to stestr run and stestr load, the only 2 commands which
currently use subunit-trace, to enable colorized output.
This commit adds the subunit-trace output filter to the stestr last
command. This is enabled by default, the last command is used to see the
results of the last command and the subunit-trace filter makes viewing
test results much easier.
This commit adds docs for subunit-trace and other output filters, as
well as providing an end user guide to the stestr manual.
@mtreinish
Copy link
Owner Author

ok, the latest revision should have fixed the issue you caught. Thanks for testing and reviewing this and all the other patches this week.

Copy link
Collaborator

@masayukig masayukig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM :)

@masayukig
Copy link
Collaborator

(I really don't know the github workflow... :-p)

@mtreinish mtreinish merged commit c182d70 into master Jul 28, 2017
@mtreinish mtreinish deleted the use-subunit-trace branch July 28, 2017 15:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants