Add integration test covering multiple package managers

[slimreaper35]

Closes #857

Maintainers will complete the following section

• Commit messages are descriptive enough
• Code coverage from testing does not decrease and new code is covered
• Docs updated (if applicable)
• Docs links in the code are still valid (if docs were updated)

Note: If the contribution is external (not from an organization member), the CI
pipeline will not run automatically. After verifying that the CI is safe to run:

• approve GitHub Actions workflows by clicking a button
• approve the Red Hat Trusted App Pipeline container build by commenting /ok-to-test
  (as is the standard for Pipelines as Code)

[a-ovchinnikov]

LGTM with two small questions

[eskultety]

The test data and test case are fine but I can't say I'm a fan of how the utils code was bent to consider multiple in the test skipping logic.

[eskultety]

I'm afraid this and putting multiple to the EXTRA set is logically wrong and very confusing when looked at. If we merge this, while the underlying problem is only tangential to this PR, we'll forget about it and probably never address the real issue.
I think our main problem that f2e1314#r2069407087 touched in context of this PR is that we're trying to produce a set of tests to skip rather than a set of tests to run and then inverting the logic in pytest_collection_modifyitems, i.e. gather the set of tests to run and add the skip marker on the rest. It all boils down to
just_some_package_managers_were_affected_by which should IMO be refactored/dissected because the current design revolving around finding the affected set of package managers clearly doesn't suffice our needs. The code may end up being longer in the end, but arguably it could be more readable and less confusing when looked at. See below for a rough pseudocode snippet. If we adopt something similar, we don't need to fiddle with 'multiple' in this fairly opaque manner, IOW what's proposed in this PR pretty much just bends the code to comply with an existing design instead of amending/reworking the design itself if it lacks in any aspect.

#conftest.py
def pytest_collection_modifyitems(..., items: list):
  ...
  run_set = utils.determine_integration_tests_to_run(items)
  for item in items:
    # this is just a minor modification of the existing logic
    if utils.tested_object_name(item.path) in run_set:
      continue
    item.add_marker(skip_mark)

#utils.py
....
GLOBAL_TEST_IMPACT = (Path('Dockerfile'), Path('requirements.txt'), etc.)
LOCAL_TEST_IMPACT = namedtuple(Path('hermeto'), Path('tests/integration'))
PATHS_TO_CODE = frozenset(GLOBAL_TEST_IMPACT + LOCALT_TEST_IMPACT)
....
def determine_integration_tests_to_run(fullset: list):
  ret = set()
  changes = select_testable_changes()

  if must_test_all() or <changes in GLOBAL_TEST_IMPACT>:
    return fullset

  if len(changes) == 0:
    return ret

  if <changes in LOCAL_TEST_IMPACT.tests>:
    test_changes = [test for test in changes if test.name.startswith('test_')]
    if not test_changes:
      # global test impact change detected, e.g. utils.py or conftest.py
      return fullset
   
   # add specific test names directly to the set, this is a deliberate redundancy with what happens next
   ret.update(test_changes)

  # continue adding affected packages managers
  if <changes in LOCAL_TEST_IMPACT.hermeto>:
    ret.update(affected_package_managers(changes)

  return ret

[ben-alkov]

we're trying to produce a set of tests to skip rather than a set of tests to run and then inverting the logic in pytest_collection_modifyitems

Heartily agree that this is not The Way

[slimreaper35]

Dropped this part in favor of #950

[slimreaper35]

The test data and test case are fine but I can't say I'm a fan of how the utils code was bent to consider multiple in the test skipping logic.

Me neither, I would leave the adjustments in the utils code to you and @a-ovchinnikov as he has the best context around it. He also mentioned privately, that he would consider dropping the logic completely.

[a-ovchinnikov]

...mentioned ... he would consider dropping the logic completely

In this PR as well (see my comment above). Rewriting the rules to be less arcane than they appear now would also work. I am not entirely sure this is in the scope of the change though, maybe a follow-up issue would be a better place for that? Edit: formatting

[eskultety]

...mentioned ... he would consider dropping the logic completely

In this PR as well (see my comment above). Rewriting the rules to be less arcane than they appear now would also work. I am not entirely sure this is in the scope of the change though, maybe a follow-up issue would be a better place for that? Edit: formatting

I mean, we'd be deliberately merging a misbehaving code, yeah, in tests, but that shouldn't be a justification factor really if the logic is off. I don't mind tracking and tackling the problem in a different PR, but then this PR would have to wait and rebase to merge clean changes, i.e. just the test. It's not like this work is urgent or anything, effort has been invested, work has been done, we can just wait for some related polishing to happen first.

[eskultety]

Alternatively, we can incorporate the suggestion from #911 (comment) to eliminate the problem at hand and then we can tackle the refactoring or reverting the test filtering logic completely.