stestr load doesn't handle empty subunit input well

[aspiers]

Issue description

Steps to reproduce the problem

• Clone openstack/nova

• Try to run a single test file:

  tox -e py27 -- -n nova/tests/unit/virt/libvirt/fakelibvirt.py
  

Expected behavior

I would expect that the tests in that file are run, as per https://stestr.readthedocs.io/en/latest/MANUAL.html#running-tests which claims:

you can also give it a file path and stestr will convert that to the proper python path under the covers. (assuming your project don’t manually mess with import paths)

or if for some reason the file could not be located or its tests not run (e.g. if the project does mess with import paths - I have no idea if that is true for nova), then I expect stestr would give a helpful warning or even error explaining what went wrong.

Actual behavior

stestr either crashes with a hopelessly cryptic error (which doesn't even look like a crash but more like a user-facing error, but I've submitted a separate issue #220 about that):

$ PATH=.tox/py27/bin:$PATH stestr run -n nova/tests/unit/virt/libvirt/fakelibvirt.py                                                                
min() arg is an empty sequence

or, if discovery is not suppressed, silently runs zero tests with no warning that something went wrong (the psycopg2 warning is obviously unrelated):

$ PATH=.tox/py27/bin:$PATH stestr run nova/tests/unit/virt/libvirt/fakelibvirt.py                                                                   
/home/adam/SUSE/cloud/OpenStack/git/nova/.tox/py27/lib/python2.7/site-packages/psycopg2/__init__.py:144: UserWarning: The psycopg2 wheel package will be renamed from release 2.8; in order to keep installing from binary please use "pip install psycopg2-binary" instead. For details see: <http://initd.org/psycopg/docs/install.html#binary-install-from-pypi>.
  """)

Specifications like the version of the project, operating system, or hardware

System information

stestr version (stestr --version):

2.2.0

Python release (python --version):

2.7.14

pip packages (pip freeze):

alembic==1.0.7
amqp==2.4.0
appdirs==1.4.3
asn1crypto==0.24.0
atomicwrites==1.2.1
attrs==18.2.0
automaton==1.15.0
Babel==2.6.0
bandit==1.5.1
bcrypt==3.1.6
cachetools==3.0.0
castellan==1.1.0
certifi==2018.11.29
cffi==1.11.5
chardet==3.0.4
cliff==2.14.0
cmd2==0.8.9
colorama==0.4.1
contextlib2==0.5.5
coverage==4.5.2
cryptography==2.5
cursive==0.2.2
ddt==1.2.0
debtcollector==1.20.0
decorator==4.3.2
dnspython==1.16.0
dogpile.cache==0.6.8
enum34==1.1.6
eventlet==0.24.1
extras==1.0.0
fasteners==0.14.1
fixtures==3.0.0
flake8==2.5.5
funcsigs==1.0.2
functools32==3.2.3.post2
future==0.17.1
futures==3.2.0
futurist==1.8.0
gabbi==1.44.0
gitdb2==2.0.5
GitPython==2.1.11
greenlet==0.4.15
grpcio==1.15.0
hacking==0.12.0
idna==2.8
ipaddress==1.0.22
iso8601==0.1.12
Jinja2==2.10
jmespath==0.9.3
jsonpatch==1.23
jsonpath-rw==1.4.0
jsonpath-rw-ext==1.2.0
jsonpointer==2.0
jsonschema==2.6.0
keystoneauth1==3.11.2
keystonemiddleware==5.3.0
kombu==4.2.2.post1
linecache2==1.0.0
lxml==4.3.0
Mako==1.0.7
MarkupSafe==1.1.0
mccabe==0.2.1
microversion-parse==0.2.1
mock==2.0.0
monotonic==1.5
more-itertools==5.0.0
mox3==0.26.0
msgpack==0.6.1
munch==2.3.2
netaddr==0.7.19
netifaces==0.10.9
networkx==2.2
-e git+https://github.com/openstack/nova.git@c134feda3d9527dbc9735e4ae9cd35c4782f1fb4#egg=nova
numpy==1.16.0
openstacksdk==0.23.0
os-brick==2.7.0
os-client-config==1.31.2
os-service-types==1.5.0
os-traits==0.10.0
os-vif==1.14.0
os-win==4.2.0
os-xenapi==0.3.4
osc-lib==1.12.0
oslo.cache==1.32.0
oslo.concurrency==3.29.0
oslo.config==6.8.0
oslo.context==2.22.0
oslo.db==4.43.0
oslo.i18n==3.23.0
oslo.log==3.42.2
oslo.messaging==9.3.1
oslo.middleware==3.37.0
oslo.policy==2.0.0
oslo.privsep==1.31.1
oslo.reports==1.29.1
oslo.rootwrap==5.15.1
oslo.serialization==2.28.1
oslo.service==1.34.0
oslo.upgradecheck==0.1.1
oslo.utils==3.39.0
oslo.versionedobjects==1.34.1
oslo.vmware==2.32.1
oslotest==3.7.0
osprofiler==2.5.2
ovs==2.10.0
ovsdbapp==0.14.0
paramiko==2.4.2
Paste==3.0.6
PasteDeploy==2.0.1
pathlib2==2.3.3
pbr==5.1.1
pep8==1.5.7
pluggy==0.8.1
ply==3.11
prettytable==0.7.2
psutil==5.5.0
psycopg2==2.7.7
py==1.7.0
pyasn1==0.4.5
pyasn1-modules==0.2.4
pycadf==2.8.0
pycparser==2.19
pydot==1.4.1
pyflakes==0.8.1
pyinotify==0.9.6
PyJWT==1.7.1
PyMySQL==0.9.3
PyNaCl==1.3.0
pyOpenSSL==19.0.0
pyparsing==2.3.1
pyperclip==1.7.0
pypowervm==1.1.19
pyroute2==0.5.3
pytest==4.1.1
python-barbicanclient==4.8.1
python-cinderclient==4.1.0
python-dateutil==2.7.5
python-editor==1.0.3
python-glanceclient==2.15.0
python-ironicclient==2.6.0
python-keystoneclient==3.18.0
python-mimeparse==1.6.0
python-neutronclient==6.11.0
python-subunit==1.3.0
pytz==2018.9
PyYAML==3.13
repoze.lru==0.7
requests==2.21.0
requests-mock==1.5.2
requestsexceptions==1.4.0
retrying==1.3.3
rfc3986==1.2.0
Routes==2.4.1
scandir==1.9.0
simplejson==3.16.0
six==1.12.0
smmap2==2.0.5
sortedcontainers==2.1.0
SQLAlchemy==1.2.17
sqlalchemy-migrate==0.12.0
sqlparse==0.2.4
statsd==3.3.0
stestr==2.2.0
stevedore==1.30.0
subprocess32==3.5.3
suds-jurko==0.6
taskflow==3.4.0
Tempita==0.5.2
tenacity==5.0.2
testresources==2.0.1
testscenarios==0.5.0
testtools==2.3.0
tooz==1.64.0
traceback2==1.4.0
unicodecsv==0.14.1
unittest2==1.1.0
urllib3==1.24.1
vine==1.2.0
voluptuous==0.11.5
warlock==1.3.0
wcwidth==0.1.7
WebOb==1.8.5
websockify==0.8.0
wrapt==1.11.1
wsgi-intercept==1.8.0
zVMCloudConnector==1.4.0

[mtreinish]

So the fundamental issue here is you're trying to run a file/module that's not actually tests. In the '-n' case it's definitely an edge condition we didn't think about and this shouldn't stack trace we should handle this gracefully and say you're specified test module didn't have any subunit output when run. This actually raises a larger issue with the load command (which is actually where the bug comes from) when you pass it input without subunit it doesn't handle this correctly at all, it either raises an exception from subunit-trace (which is what you're hitting) or if you turn off subunit-trace it just silently adds an empty file to the repository.

For the case without '-n' it should return an error like: "The specified regex doesn't match with anything" and then exit with a return code of 1. (which is the defined behavior when you pass in an invalid regex:

stestr/stestr/commands/run.py

Lines 471 to 472 in 7a1da4f

	stdout.write("The specified regex doesn't match with anything")
	return 1

) This is because you're passing in the path to the file as a regex for test selection. Obviously the path isn't a regex that's going to match any test ids returned from discovery. This is exactly what happened when I ran stestr run nova/tests/unit/virt/libvirt/fakelibvirt.py locally on the nova repo the output I got was:

/home/computertreker/git/nova/.tox/py37/lib/python3.7/site-packages/psycopg2/__init__.py:144: UserWarning: The psycopg2 wheel package will be renamed from release 2.8; in order to keep installing from binary please use "pip install psycopg2-binary" instead. For details see: <http://initd.org/psycopg/docs/install.html#binary-install-from-pypi>.
The specified regex doesn't match with anything

[aspiers]

@mtreinish commented on January 29, 2019 6:14 PM:

So the fundamental issue here is you're trying to run a file/module that's not actually tests.

Ah, good spot! I accidentally missed the test_ prefix before fakelibvirt.

In the '-n' case it's definitely an edge condition we didn't think about and this shouldn't stack trace we should handle this gracefully and say you're specified test module didn't have any subunit output when run.

Yes exactly.

This actually raises a larger issue with the load command (which is actually where the bug comes from) when you pass it input without subunit it doesn't handle this correctly at all, it either raises an exception from subunit-trace (which is what you're hitting) or if you turn off subunit-trace it just silently adds an empty file to the repository.

OK, so can this be fixed?

For the case without '-n' it should return an error like: "The specified regex doesn't match with anything" and then exit with a return code of 1. (which is the defined behavior when you pass in an invalid regex:

stestr/stestr/commands/run.py

Lines 471 to 472 in 7a1da4f

	stdout.write("The specified regex doesn't match with anything")
	return 1

)

This is because you're passing in the path to the file as a regex for test selection. Obviously the path isn't a regex that's going to match any test ids returned from discovery. This is exactly what happened when I ran stestr run nova/tests/unit/virt/libvirt/fakelibvirt.py locally on the nova repo the output I got was:

/home/computertreker/git/nova/.tox/py37/lib/python3.7/site-packages/psycopg2/__init__.py:144: UserWarning: The psycopg2 wheel package will be renamed from release 2.8; in order to keep installing from binary please use "pip install psycopg2-binary" instead. For details see: <[http://initd.org/psycopg/docs/install.html#binary-install-from-pypi](http://initd.org/psycopg/docs/install.html#binary-install-from-pypi)>.
The specified regex doesn't match with anything

That does not work for me - as already reported I do not get any such error. If I had, most likely I would have realised that it does not make sense to pass in a file path without -n. So this is another bug. I can even reproduce it with a nonsense argument:

$ PATH=.tox/py27/bin:$PATH stestr run foobar
/home/adam/SUSE/cloud/OpenStack/git/nova/.tox/py27/lib/python2.7/site-packages/psycopg2/__init__.py:144: UserWarning: The psycopg2 wheel package will be renamed from release 2.8; in order to keep installing from binary please use "pip install psycopg2-binary" instead. For details see: <http://initd.org/psycopg/docs/install.html#binary-install-from-pypi>.
  """)

The trailing """) noise also seems to point to a bug.

[mtreinish]

This actually raises a larger issue with the load command (which is actually where the bug comes from) when you pass it input without subunit it doesn't handle this correctly at all, it either raises an exception from subunit-trace (which is what you're hitting) or if you turn off subunit-trace it just silently adds an empty file to the repository.

OK, so can this be fixed?

Sorry I missed you're question here. Yes this should be fixable, it shouldn't be too bad we just need to figure out the best place in stestr load to figure out that the input is empty and return an appropriate error code and message in that case. The complexity is that the actual processing is all handled by setting up a stream processing pipeline and passing the input file-like object to it. So we have 2 approaches to fix it, either being a stage in the testtools/subunit processing pipeline that checks if the input was empty at test run stop time, or just check the output after the processing is done. We've typically done the latter approach because it's a bit simpler to implement and debug, and it doesn't require knowing the internals of testtools and subunit super well (which despite working with for years, I still feel that I really don't).

[masayukig]

It looks the issue was fixed in the stestr 2.5.0 release?

$ stestr --version
stestr 2.5.0
$ stestr run -n nova/tests/unit/virt/libvirt/fakelibvirt.py
No tests were successful during the run

$ stestr --version
stestr 2.4.0
$ stestr run -n nova/tests/unit/virt/libvirt/fakelibvirt.py
min() arg is an empty sequence
<snip>

[mtreinish]

Sort of but not for real. It's enough for the original issue in this bug both #263 and #244 fixed issues with the subunit trace handling in the load command so that broken input with subunit-trace enabled wouldn't be fatal. However, it doesn't change load's behavior it will still create an empty result in the repository. That empty result will then break anything else that uses the stored empty stream because nothing knows how to handle an empty stream. For example if you follow that command with stestr failing it will show the result as: PASSED (id=0) Or if you run stestr last you'll get a ValueError: min() arg is an empty sequence error. Or my favorite issue is if you run stestr run without subunit-trace for example with: stestr run --no-subunit-trace -n nova/tests/unit/virt/libvirt/fakelibvirt.py it will pass because there was no error in the empty subunit stream PASSED (id=1) (which is the same check stestr failing does).