Reviewed 1 of 3 files at r5.
Reviewable status: 1 of 3 files reviewed, 13 unresolved discussions (waiting on @asomsiko and @just-sparta)

tests/conditional_compilation/conftest.py, line 23 at r5 (raw file):

# add current dir to imports
sys.path.insert(0, str((Path(getsourcefile(lambda: 0))).resolve(strict=True)))

pytest already adds current dir to sys path https://docs.pytest.org/en/stable/pythonpath.html#invoking-pytest-versus-python-m-pytest

Also, notice the bug in the code - it adds a current file to the sys.path, not the current directory

tests/conditional_compilation/conftest.py, line 25 at r5 (raw file):

# pylint: disable=import-error

is not needed then

tests/conditional_compilation/conftest.py, line 88 at r5 (raw file):

@pytest.fixture(scope="function")
def test_info(request, pytestconfig):

Appreciate if you could put a short comment on what this and the following fixtures do

tests/conditional_compilation/conftest.py, line 89 at r5 (raw file):

@pytest.fixture(scope="function")
def test_info(request, pytestconfig):
    setattr(request.node._request, "test_info", {"test_id": {}})

Should we keep test_info empty so we can catch errors if the test does not fill it in?

tests/conditional_compilation/test_collect.py, line 17 at r5 (raw file):

def test_cc_collect(test_id, model, sea_runtool, benchmark_app, collector_dir, artifacts, test_info):
    """ Test conditional compilation statistics collection
    :param test_info: custom `test_info` field of built-in `request` pytest fixture

better explain what it contains e.g. :param test_info: A dictionary to store test metadata

tests/conditional_compilation/tests_utils.py, line 1 at r5 (raw file):

import json

file missing copyright and a comment.

Please ensure pylint has zero issues on it.

tests/conditional_compilation/tests_utils.py, line 6 at r5 (raw file):

def open_cc_json(path: Path = Path(__file__).parent / "cc_tests.json"):

Better name might be read_test_info and write_test_info. Because your reading, not opening. Also the json is an implementation detail and should not be in the name.

tests/conditional_compilation/tests_utils.py, line 12 at r5 (raw file):

cc_tests.json

Please make a constant for this TEST_INFO_NAME='cc_tests.json'

tests/conditional_compilation/tests_utils.py, line 12 at r5 (raw file):

Path(__file__)

In the other places we use getsourcefile(lambda: 0) instead __file__. Better to align to common approach.

tests/conditional_compilation/tests_utils.py, line 16 at r5 (raw file):

ensure_ascii=False

Could you give an example of non-ascii content?

tests/conditional_compilation/tests_utils.py, line 22 at r5 (raw file):

    for test_dict in cc_tests_ids:
        used_csv = []
        for cc_csv in workspace.glob("**/*.csv"):

Can we pass csv path via test_info from the test case here?

I'm thinking the test which calls sea_runtool does own .csv generation and it may not always have all CSVs in a single folder.

Reviewed 2 of 3 files at r5.
Reviewable status: 0 of 3 files reviewed, 4 unresolved discussions (waiting on @asomsiko, @just-sparta, and @vurusovs)

tests/conditional_compilation/conftest.py, line 87 at r4 (raw file):

in case of failures there is no need to continue test

Disagree with that. I'd argue we want to see a failures and that requires to run the followup test

tests/conditional_compilation/conftest.py, line 89 at r5 (raw file):

setattr(request.node._request, "test_info", {"test_id": {}, "out_csv": {}})

It is still not clear to me why would we need to define empty test_id and out_csv. Why can't we define an empty test_infosetattr(request.node._request, "test_info", {})?

tests/conditional_compilation/test_collect.py, line 18 at r6 (raw file):

    """ Test conditional compilation statistics collection
    :param test_info: A dictionary to store test metadata
    :param test_info: custom `test_info` field of built-in `request` pytest fixture.

test_info comment seems duplicated

Reviewed 1 of 3 files at r6, 2 of 2 files at r7.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @just-sparta and @vurusovs)