ci: enforce pytest as required gate on all PRs, fix pre-existing test failures#56
Merged
Conversation
…st failures Workflows: - python-app.yml: add 'Run unit tests' step (pytest tests/ -v) that fails the job on any test failure; remove emojis from echo output; reorder steps so tests run before CLI smoke tests - coverage.yml: remove continue-on-error: true from pytest step so test failures now fail the coverage job; remove emojis from step summaries Test fixes (pre-existing failures that would have blocked CI): - docker_scanner.py _validate_file_path: check '..' in raw input string before Path.resolve() — resolved paths never contain '..' so the traversal guard was silently bypassed on Linux - docker_scanner.py _validate_image_name: replace blacklist with a whitelist regex so spaces and other unlisted characters are rejected - tests/test_utils.py: fix mock targets from 'utils.get_openai_api_key' (does not exist) to 'config_manager.get_config' which is what get_llm() actually calls - tests/test_docker_scanner.py: compare resolved paths on both sides to handle macOS /var -> /private/var symlink - tests/test_integration.py: same resolved-path fix Result: 16/17 tests pass locally (1 remaining failure is environment-specific — real OPENAI_API_KEY in shell env; passes in CI where no key is injected)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Makes tests a hard requirement before any PR can merge. Previously both workflows either skipped pytest entirely or used
continue-on-error: true, meaning broken tests never blocked a merge.Workflow changes
python-app.yml
Run unit testsstep:pytest tests/ -v— fails the job if any test failsOPENAI_API_KEYfrom the pytest step (unit tests use mocks, no real key needed)coverage.yml
continue-on-error: truefrom pytest step — test failures now fail the coverage jobTest fixes (pre-existing failures that would have broken CI)
docker_scanner.py_validate_file_path..in raw input beforePath.resolve()— resolved paths never contain..so the traversal guard was silently bypasseddocker_scanner.py_validate_image_nametests/test_utils.pyutils.get_openai_api_key(doesn't exist) toconfig_manager.get_configwhich is whatget_llm()actually callstests/test_docker_scanner.py/var→/private/varsymlinktests/test_integration.pyTest results
The 1 remaining local failure (
test_get_openai_api_key_missing) is environment-specific — triggered by a realOPENAI_API_KEYin the developer's shell. Passes in CI where no key is injected into the pytest step.After merging — enable branch protection
To fully enforce this, go to:
GitHub → Settings → Branches → Add rule → Branch name:
mainCheck:
build-and-test(from python-app.yml)This makes it impossible to merge a PR with failing tests.
Type of Change