Skip to content

Add pytest suite and CI workflow (finding #27) - #18

Open
elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/27-add-tests-and-ci
Open

Add pytest suite and CI workflow (finding #27)#18
elhoim wants to merge 1 commit into
MISP:mainfrom
elhoim:fix/27-add-tests-and-ci

Conversation

@elhoim

@elhoim elhoim commented Aug 31, 2026

Copy link
Copy Markdown
Member

Finding 27 (Medium) — .github/workflows/pypi-publish.yml:1

Problem

The repository contains NO tests and no CI job other than publishing - requirements-dev.txt holds only 'build'. Nothing verifies bin/cli.py before it is uploaded to PyPI.

Fix

Finding #27 (medium, add-tests-and-ci): the repo had no tests and its only CI job published to PyPI, so nothing verified bin/cli.py before release. Added tests/test_cli.py, a 30-test pytest suite covering the pure helper functions - attribute-type detection (is_ipv4/is_ipv6/looks_like_* family and guess_attribute_types ranking), module/type mapping helpers, markdown report rendering (format_markdown_output and its internal table rendering for dict/list/scalar responses), and cache logic (make_cache_key stability, get_cached_response hit/miss/TTL-expiry/malformed-entry handling, set_cached_response). The suite passes unmodified against the base commit (no other findings assumed fixed). Added .github/workflows/ci.yml (flake8 lint + pytest + CLI --help/import smoke test on Python 3.10/3.11/3.12, triggered on push/PR and reusable via workflow_call), and wired pypi-publish.yml's build job to needs: test calling that workflow, so a release can no longer be published without CI passing. Added requirements-dev.txt entries for pytest/flake8, a .flake8 config (max-line-length=130, matching two pre-existing long lines in cli.py which were left untouched as they're not part of this finding), and a .gitignore for pycache/.pytest_cache.

Verification

Reproduced against the unmodified code at 9b8c605, then re-checked after the change.

Before
Before the fix: repo had zero test files (find for test_*.py/conftest.py returned nothing) and requirements-dev.txt contained only '-r requirements.txt' plus 'build>=1.2.1'; .github/workflows/ only had pypi-publish.yml (build+publish, no lint/test step) - so a broken bin/cli.py could be released to PyPI with zero automated verification.
After
After the fix: 'source ~/.venv/bin/activate && python -m pytest tests/ -q' -> "30 passed in ~5.4s"; 'flake8 bin/ tests/' -> clean (no output); 'python bin/cli.py --help' exits 0; 'python -c "import importlib; importlib.import_module(\'bin.cli\')"' -> "import ok". pypi-publish.yml's build job now has 'needs: test' calling the reusable ./.github/workflows/ci.yml (workflow_call), so publishing depends on CI passing.

python bin/cli.py --help exits 0 and the module still imports cleanly. Verification was performed offline against the pure functions — no running misp-modules instance is required.

Branched from 9b8c605. This PR addresses only this finding; the other findings from the same review are in separate PRs, so they will need rebasing against each other as they merge.

🤖 Generated with Claude Code

https://claude.ai/code/session_01DYX4TKA5inzByJ4qGWKjqh

The repository had no tests and the only CI job was PyPI publishing, so nothing verified bin/cli.py before release.

Adds a pytest suite (tests/test_cli.py) covering the pure helper functions: attribute-type detection (guess_attribute_types and its is_/looks_like_* building blocks), module/type mapping helpers, markdown report rendering (format_markdown_output and its table rendering), and cache key/TTL logic (make_cache_key, get_cached_response, set_cached_response). The suite passes against the unmodified base commit.

Adds .github/workflows/ci.yml running flake8 and pytest plus a CLI --help/import smoke test on Python 3.10-3.12 for pushes and PRs, and wires it as a workflow_call dependency of the publish job in pypi-publish.yml so releases can no longer be published without CI passing first. Also adds requirements-dev.txt entries for pytest/flake8 and a .flake8 config.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant