CFMTech / pytest-monitor

Pytest plugin for analyzing resource usage during test sessions
MIT License
175 stars 16 forks source link

NotImplementedError: can't find current frequency file #60

Closed Lucas-C closed 2 years ago

Lucas-C commented 2 years ago

Describe the bug After installing pytest-monitor to plug it inside https://github.com/PyFPDF/fpdf2 a stacktrace was raised when calling pytest

To Reproduce Steps to reproduce the behavior:

  1. Clone https://github.com/PyFPDF/fpdf2
  2. Run pip install --upgrade . pytest-monitor -r test/requirements.txt in the cloned repo directory
  3. Run pytest

Expected behavior No error

Stacktrace

INTERNALERROR> Traceback (most recent call last):
INTERNALERROR>   File "/home/user/.local/share/virtualenvs/fpdf2/lib/python3.8/site-packages/_pytest/main.py", line 266, in wrap_session
INTERNALERROR>     config.hook.pytest_sessionstart(session=session)
INTERNALERROR>   File "/home/user/.local/share/virtualenvs/fpdf2/lib/python3.8/site-packages/pluggy/_hooks.py", line 265, in __call__
INTERNALERROR>     return self._hookexec(self.name, self.get_hookimpls(), kwargs, firstresult)
INTERNALERROR>   File "/home/user/.local/share/virtualenvs/fpdf2/lib/python3.8/site-packages/pluggy/_manager.py", line 80, in _hookexec
INTERNALERROR>     return self._inner_hookexec(hook_name, methods, kwargs, firstresult)
INTERNALERROR>   File "/home/user/.local/share/virtualenvs/fpdf2/lib/python3.8/site-packages/pluggy/_callers.py", line 60, in _multicall
INTERNALERROR>     return outcome.get_result()
INTERNALERROR>   File "/home/user/.local/share/virtualenvs/fpdf2/lib/python3.8/site-packages/pluggy/_result.py", line 60, in get_result
INTERNALERROR>     raise ex[1].with_traceback(ex[2])
INTERNALERROR>   File "/home/user/.local/share/virtualenvs/fpdf2/lib/python3.8/site-packages/pluggy/_callers.py", line 34, in _multicall
INTERNALERROR>     next(gen)  # first yield
INTERNALERROR>   File "/home/user/.local/share/virtualenvs/fpdf2/lib/python3.8/site-packages/pytest_monitor/pytest_monitor.py", line 194, in pytest_sessionstart
INTERNALERROR>     session.pytest_monitor.compute_info(session.config.option.mtr_description,
INTERNALERROR>   File "/home/user/.local/share/virtualenvs/fpdf2/lib/python3.8/site-packages/pytest_monitor/session.py", line 85, in compute_info
INTERNALERROR>     self.set_environment_info(ExecutionContext())
INTERNALERROR>   File "/home/user/.local/share/virtualenvs/fpdf2/lib/python3.8/site-packages/pytest_monitor/sys_utils.py", line 72, in __init__
INTERNALERROR>     self.__cpu_freq_base = psutil.cpu_freq().current
INTERNALERROR>   File "/home/user/.local/share/virtualenvs/fpdf2/lib/python3.8/site-packages/psutil/__init__.py", line 1857, in cpu_freq
INTERNALERROR>     ret = _psplatform.cpu_freq()
INTERNALERROR>   File "/home/user/.local/share/virtualenvs/fpdf2/lib/python3.8/site-packages/psutil/_pslinux.py", line 701, in cpu_freq
INTERNALERROR>     raise NotImplementedError(
INTERNALERROR> NotImplementedError: can't find current frequency file

Desktop (please complete the following information):

js-dieu commented 2 years ago

Hi @Lucas-C

Apologies for the delay and thanks for reporting! It seems psutil is confused with your system. I think it will look for /proc/cpuinfo but such files do not seems to exists.

I suggest to report that point to psutil directly.

Lucas-C commented 2 years ago

This is indeed an issue with psutil & WSL: https://github.com/giampaolo/psutil/issues/1251

While support for this is added, could you consider having a fallback? Like other libs did: https://github.com/google/deepvariant/issues/191#issuecomment-504492772

js-dieu commented 2 years ago

You are absolutely right. I think that 2 issues is enough for implementing it especially with the psutil team having difficulties in solving these issues. I'll do my best to do it quickly. I am thinking about a two times fallback similar to:

try:
   freq = psutil.cpu_freq()
   return freq.current if freq is not None else 0.0
except NotImplementedError:
    return float(os.environ.get('PYTEST_MONITOR_CPU_FREQ', '0')

I think that would put your exception away and match your use case.

Lucas-C commented 2 years ago

That look great! Thank you for taking a look at this @js-dieu 😊