pytest produces spurious error output in non-git directories/repos with git 2.14.2 when determining project name

[azaghal]

Environment

OS: Debian 9.3 Stretch (amd64), with backports repository enabled Python version: 2.7.13 pytest-benchmark version: 3.1.1 git version: 2.14.2

Requirements file (pip freeze):

attrs==17.4.0
docutils==0.14
funcsigs==1.0.2
pathlib==1.0.1
pkg-resources==0.0.0
pluggy==0.6.0
py==1.5.2
py-cpuinfo==3.3.0
pytest==3.3.2
pytest-benchmark==3.1.1
six==1.11.0
statistics==1.0.3.5

Reproduction steps

1. Bring-up a VM, and install the following prerequisites on it (subsequent steps should be performed on the VM itself):

   • virutalenvwrapper
   • git, version 2.14.2 (2.13.x and upwards may work as well, but I have not tested this)

   If using Vagrant, this should be as simple as:

   mkdir -p ~/tmp/pytest-benchmark-error-output
   cd ~/tmp/pytest-benchmark-error-output
   vagrant init debian/stretch64
   vagrant up
   vagrant ssh -c 'echo "deb http://ftp.debian.org/debian stretch-backports main" | sudo tee /etc/apt/sources.list.d/backports.list'
   vagrant ssh -c 'sudo apt-get update && sudo apt-get -t stretch-backports -y install virtualenvwrapper git'
   

2. Create an empty directory where pytest will be run, and switch to it:

   mkdir /tmp/pytest-benchmark-error-output
   cd /tmp/pytest-benchmark-error-output
   

3. Set-up Python virtual environment and activate it:

   mkvirtualenv pytest-benchmark-error-output && pip install pytest-benchmark==3.1.1
   

4. Run pytest:

   pytest
   

Expected results

While running command from (4), output is as follows:

======== test session starts ========
platform linux2 -- Python 2.7.13, pytest-3.3.2, py-1.5.2, pluggy-0.6.0
benchmark: 3.1.1 (defaults: timer=time.time disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /tmp/pytest-benchmark-error-output, inifile:
plugins: benchmark-3.1.1
collected 0 items                                                                                                                                                                                                 

======== no tests ran in 0.00 seconds ========

Actual results:

While running command from (4), output is as follows (note the fatal error at top):

fatal: --local can only be used inside a git repository
======== test session starts ========
platform linux2 -- Python 2.7.13, pytest-3.3.2, py-1.5.2, pluggy-0.6.0
benchmark: 3.1.1 (defaults: timer=time.time disable_gc=False min_rounds=5 min_time=0.000005 max_time=1.0 calibration_precision=10 warmup=False warmup_iterations=100000)
rootdir: /tmp/pytest-benchmark-error-output, inifile:
plugins: benchmark-3.1.1
collected 0 items                                                                                                                                                                                                 

======== no tests ran in 0.00 seconds ========

Additional notes

The error comes from the file src/pytest_benchmark/utils.py, function get_project_name_git. Technically speaking, the error can safely be ignored - it only seems that more recent git versions will be a bit more "whiny" about running the git config --local remote.origin.url command outside of a git repository.

I can see three possible solutions, and could provide a PR depending on what is more desirable:

1. Check if the directory we are in is a git repository or not. This is probably the safest thing to do long-term. Stack Overflow has a decent solution listed.

2. Mask the specific error output. This would have tendency to break in future.

3. Mask any error output. This would have tendency to hide more serious errors (like missing git binary or something similar).

[kiilerix]

It might be good optional high-level feature to determine project name from the VCS. But it seems a bit fragile and like a layering violation that a building block like pytest-benchmark talks to the VCS. Especially when it also is run when running pytest without acctually using pytest-benchmark.

Is there a way to disable it?

[ionelmc]

There is no way currently. I will make it so it's used on demand and fix the lack of stderr capturing (that produces your spurious output).

[kiilerix]

Thanks ... but I find it even more scary if it keeps doing the layering violation but just hide it even more.

[ionelmc]

Hehe, ok. So you would like this behavior to be opt-in? Like always have to specify a --benchmark-project-name=@auto or similar? Probably not.

So now I've read the bug report more - you have actually proposed error output masking. Exactly which fix will make you the most happy?

[kiilerix]

I do not consider output masking a good solution.

I'm not sure why pytest-benchmark needs a project name more than other parts of pytest ... and why the repository name is good for that name.

If there is a name, then I would expect to be able to set it in pytest.ini . That would be more reliable and correct than trying to guess it from other information.

[ionelmc]

So currently the project name is used for elasticsearch storage (as a namespace) and it's also part of the commit info.

So if getting project name is too sneaky then getting commit info is also sneaky. But commit info can't be put in a configuration file. I'd rather be consistent and be sneaky everywhere :)

So what I think would be best is: better output masking + avoid getting commit info/project name if no data is going to be saved.

You agree that commit info is a good thing yes?

[azaghal]

Well, I guess the quickest solution would be to simply mask-out stderr from git command. This would preserve existing behaviour, with the problem of any other error (say, missing git command) being masked-out as well.

A more comprehensive solution could be:

1. Introduce ability to specify project name within pytest.ini. This would allow for users to have better consistency in case of changing URLs and what-not.

2. Avoid getting info/project name if no data is going to be saved. Not sure if this is worth the effort, provided (3), (4), and (5) would get added.

3. Do an additional check to ensure git is present on the system (checking if git command is in the PATH).

4. Do an additional check to see if we are in a git repo prior to reading the remote.origin.url. This could be done with git rev-parse --git-dir where stderr is masked-out. This is as per answer from Stack Overflow.

5. Finally run ['git', 'config', '--local', 'remote.origin.url'] without masking the output so "valid" errors (such as key remote.origin.url not existing etc) would be visible.

[ml31415]

I don't see a good reason, why pytest-benchmark should interact with any versioning system at all, if I don't explicitly tell it to. Just run into this when I noticed that pytest-benchmark is calling for hg and producing error output, if I'm in a completely unversioned folder. I totally agree with @kiilerix that this is quite a violation from what I expect this tool to do. And a quite error prone and time waster when it's doing such stuff, whenever a test is run.

[ionelmc]

So I implemented (4). Only need to make this more lazy (don't muck with scms if no data is saved).

About making the whole scm thing optout or optin - @ml31415 why is getting scm info so bad for you? How does your usecase look?

[ml31415]

It's not a show stopper, but it slows down the whole pytest invocation quite a lot. I frequently use pytest selectively just for a couple of tests I'm working on, so wasting 1-2 seconds on every call is annoying.

[christopher-hesse]

pytest-benchmark is calling git, but git hangs in my local repo, which means pytest now hangs, how do I prevent pytest-benchmark from calling git?

[ionelmc]

Ok, we could have an config option for this.