Use AST or "engine" provided discovery instead of "simple"'s import?

[yarikoptic]

ATM I am trying to test drive #29 on https://github.com/datalad/datalad/ code base. Output is filled with

[995826] 2020-10-27 12:11:12,684 - datalad.utils - DEBUG - Maximal length of cmdline string (adjusted for safety margin): 1048576
[995826] 2020-10-27 12:11:12,700 - pycrunch.discovery.simple - ERROR - Failed to load `datalad/cmdline/tests/test_formatters.py`
[995826] 2020-10-27 12:11:12,700 - pycrunch.discovery.simple - ERROR - importing datalad.cmdline.tests.test_formatters failed with exception: Cannot run the event loop while another loop is running
Traceback (most recent call last):
  File "/home/yoh/proj/misc/pycrunch-engine/pycrunch/discovery/simple.py", line 103, in find_tests_in_folder
    module = importlib.import_module(module_name)
  File "/usr/lib/python3.8/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 961, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 961, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 961, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "<frozen importlib._bootstrap>", line 1014, in _gcd_import
  File "<frozen importlib._bootstrap>", line 991, in _find_and_load
  File "<frozen importlib._bootstrap>", line 975, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 671, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 783, in exec_module
  File "<frozen importlib._bootstrap>", line 219, in _call_with_frames_removed
  File "/home/yoh/proj/datalad/datalad-master/datalad/__init__.py", line 47, in <module>
    cfg = ConfigManager()
  File "/home/yoh/proj/datalad/datalad-master/datalad/config.py", line 247, in __init__
    self.reload(force=True)
  File "/home/yoh/proj/datalad/datalad-master/datalad/config.py", line 300, in reload
    self._stores[store_id] = self._reload(runargs)
  File "/home/yoh/proj/datalad/datalad-master/datalad/config.py", line 333, in _reload
    stdout, stderr = self._run(
  File "/home/yoh/proj/datalad/datalad-master/datalad/config.py", line 684, in _run
    out = self._runner.run(self._config_cmd + args, **kwargs)
  File "/home/yoh/proj/datalad/datalad-master/datalad/cmd.py", line 489, in run
    results = event_loop.run_until_complete(
  File "/usr/lib/python3.8/asyncio/base_events.py", line 592, in run_until_complete
    self._check_running()
  File "/usr/lib/python3.8/asyncio/base_events.py", line 554, in _check_running
    raise RuntimeError(
RuntimeError: Cannot run the event loop while another loop is running
/home/yoh/proj/misc/pycrunch-engine/pycrunch/discovery/simple.py:108: RuntimeWarning: coroutine 'run_async_cmd' was never awaited

which might be something we might want to address on datalad end (to not reuse main loop) but in any case -- I think that pycrunch should not import modules to discover tests. It should either just load those files and inspect via AST or even better -- rely on the underlying engine to discover tests. E.g. with nose it should be smth like

$> DATALAD_LOG_TARGET=/dev/null python -m nose --collect-only -v datalad
Failure: SkipTest () ... ok
datalad.cmdline.tests.test_helpers.test_strip_arg_from_argv ... ok
datalad.cmdline.tests.test_helpers.test_get_repo_instance_annex ... ok
datalad.cmdline.tests.test_helpers.test_get_repo_instance_git ... ok
datalad.cmdline.tests.test_main.test_version ... ok
datalad.cmdline.tests.test_main.test_help_np ... ok
datalad.cmdline.tests.test_main.test_usage_on_insufficient_args ... ok
...

may be there is a better way - since in this invocation nose also runs all "tests generators" which causes some work to be done in fixtures etc (that is why I explicitly redirected our logging target to /dev/null to not pollute). IMHO for pycrunch purpose "running generators" is not needed probably and those could be considered a single "test".

[gleb-britecore]

Parsing command-line output can be fragile, it may pick up some unrelated log lines, and in the case of pytest it is also slow (360ms to run pytest.main, and more time to compile bytecode). For each [engine+version], we will need separate parser, and expect that pytest will never change the output format. And, if we have 1000+ tests, it would be overkill to run full discovery every time single file changes

I'm was considering AST as an option, but at the moment of the first release, I didn't have much expertise with it. This in on a list, but a bit lower than async support

If deciding what to do next, I would say - async tests, because it is easy to adjust code for simple discovery (by removing some top-level import/object instantiation), and not possible at all to run async tests

[yarikoptic]

Re AST, right -- found https://github.com/gleb-sevruk/pycrunch-engine/pull/7/files#diff-950f810bbc146172e4397584f2146599edb7c059f25f2614ad1ccf4166409b05R102 ;-)

re parsing: I bet there should be some way to invoke pytest straight from python to discover tests without running them, and without parsing any stdout.

[gleb-sevruk]

Will be fixed in https://github.com/gleb-sevruk/pycrunch-engine/pull/57

[gleb-sevruk]

Fixed! Please give it a shot 🎉

pip install pycrunch-engine==1.3