workflow,pytest: add new --error-on-skip for pytest

[mvo5]

To ensure we do not skip tests in GH actions that we want to run add a new explicit way to allowlist tests that are okay to skip.

This is done via a small pytest plugin and the GH action will only set "no SELinux" initially.

Followup for osbuild/containers#72 and #1755

[draft as a bunch of tests will fail because our container is missing some more binaries]

[achilleas-k]

Interesting approach. We're getting really deep into pytest weirdnesses now :D

[mvo5]

@achilleas-k suggested to use --ok-to-skip to avoid the double negative, I like it so unless someone has a better suggestion I would say let's go with that

[supakeen]

This is starting to get really funky. Would it not be possible to remove skips we have and move SELinux tests into a separate module that is not selected on GitHub instead?

[mvo5]

This is starting to get really funky. Would it not be possible to remove skips we have and move SELinux tests into a separate module that is not selected on GitHub instead?

Maybe, something like this would work probably, it looks about the same to me, i have no strong opinion.

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 0bd04444..6501bf3d 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -46,7 +46,7 @@ jobs:
               TEST_CATEGORY="not test_stages.py and not test_assemblers.py"
           fi
           OSBUILD_TEST_STORE="${{ env.OSBUILD_TEST_STORE }}" \
-          tox -e "${{ matrix.environment }}" -- $TEST_WORKERS -k "$TEST_CATEGORY" --error-on-skip --ok-to-skip="no SELinux"
+          tox -e "${{ matrix.environment }}" -- $TEST_WORKERS -k "$TEST_CATEGORY" --error-on-skip -k "not test_selinux"
 
   v1_manifests:
     name: "Assembler test (legacy)"
diff --git a/conftest.py b/conftest.py
index 83603aca..8530e56d 100644
--- a/conftest.py
+++ b/conftest.py
@@ -7,10 +7,9 @@ def pytest_runtest_makereport(item, call):
     res = outcome.get_result()
     if res.skipped:
         error_on_skip = item.config.getoption('--error-on-skip')
-        allowlist_skip = item.config.getoption('--ok-to-skip')
 
         reason = str(call.excinfo.value)
-        if error_on_skip and reason not in allowlist_skip:
+        if error_on_skip:
             res.outcome = "failed"
             res.longrepr = f"skipping not allowed: {res.longrepr}"
 
@@ -19,6 +18,3 @@ def pytest_addoption(parser):
     parser.addoption(
         '--error-on-skip', action="store_store",
     )
-    parser.addoption(
-        "--ok-to-skip", type=lambda arg: arg.split(","),
-    )

[supakeen]

This is starting to get really funky. Would it not be possible to remove skips we have and move SELinux tests into a separate module that is not selected on GitHub instead?

Maybe, something like this would work probably, it looks about the same to me, i have no strong opinion.

diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml
index 0bd04444..6501bf3d 100644
--- a/.github/workflows/test.yml
+++ b/.github/workflows/test.yml
@@ -46,7 +46,7 @@ jobs:
               TEST_CATEGORY="not test_stages.py and not test_assemblers.py"
           fi
           OSBUILD_TEST_STORE="${{ env.OSBUILD_TEST_STORE }}" \
-          tox -e "${{ matrix.environment }}" -- $TEST_WORKERS -k "$TEST_CATEGORY" --error-on-skip --ok-to-skip="no SELinux"
+          tox -e "${{ matrix.environment }}" -- $TEST_WORKERS -k "$TEST_CATEGORY" --error-on-skip -k "not test_selinux"
 
   v1_manifests:
     name: "Assembler test (legacy)"
diff --git a/conftest.py b/conftest.py
index 83603aca..8530e56d 100644
--- a/conftest.py
+++ b/conftest.py
@@ -7,10 +7,9 @@ def pytest_runtest_makereport(item, call):
     res = outcome.get_result()
     if res.skipped:
         error_on_skip = item.config.getoption('--error-on-skip')
-        allowlist_skip = item.config.getoption('--ok-to-skip')
 
         reason = str(call.excinfo.value)
-        if error_on_skip and reason not in allowlist_skip:
+        if error_on_skip:
             res.outcome = "failed"
             res.longrepr = f"skipping not allowed: {res.longrepr}"
 
@@ -19,6 +18,3 @@ def pytest_addoption(parser):
     parser.addoption(
         '--error-on-skip', action="store_store",
     )
-    parser.addoption(
-        "--ok-to-skip", type=lambda arg: arg.split(","),
-    )

I like this more!

[github-actions]

This PR is stale because it has been open 30 days with no activity. Remove "Stale" label or comment or this will be closed in 7 days.

[github-actions]

This PR was closed because it has been stalled for 30+7 days with no activity.