tests: migrate to data-driven fixtures for feature presence#2986
tests: migrate to data-driven fixtures for feature presence#2986williballenthin wants to merge 16 commits intomasterfrom
Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Please add bug fixes, new features, breaking changes and anything else you think is worthwhile mentioning to the master (unreleased) section of CHANGELOG.md. If no CHANGELOG update is needed add the following to the PR description: [x] No CHANGELOG update needed
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
williballenthin
force-pushed
the
data-fixtures-2
branch
from
71b4687 to
dc455d1
Compare
williballenthin
force-pushed
the
data-fixtures-2
branch
6 times, most recently
from
f8d4c8b to
ad6aeb0
Compare
This comment was marked as resolved.
This comment was marked as resolved.
williballenthin
force-pushed
the
data-fixtures-2
branch
from
b68984e to
c74ae97
Compare
github-actions
Bot
dismissed
their stale review
CHANGELOG updated or no update needed, thanks! 😄
williballenthin
force-pushed
the
data-fixtures-2
branch
from
ac39fda to
0a35068
Compare
Moves FEATURE_PRESENCE_TESTS_DOTNET and FEATURE_COUNT_TESTS_DOTNET from hardcoded Python lists in tests/fixtures.py into the shared tests/fixtures/feature-presence.json data file, tagged with "dotnet".
williballenthin
force-pushed
the
data-fixtures-2
branch
from
55e8721 to
e294e69
Compare
mr-tz
left a comment
mr-tz
left a comment
There was a problem hiding this comment.
nice work, great step into a data-driven test direction - a few comments to discuss inline
There was a problem hiding this comment.
idea: can you show that the changes here work with the current tests so we have some reassurance there?
There was a problem hiding this comment.
good idea. i can probably do this on a one-off, local basis and screenshot my work.
- one-time regression check for rule parsing/handling
| if True: | ||
| import idapro # noqa: F401 [imported but unused] |
There was a problem hiding this comment.
idapro has to be imported first, because it messes with sys.path
but isort likes to reorder it
so i put it in a if True block to force it to show up first
There was a problem hiding this comment.
ack, maybe add a comment on this
Remove 24 file entries from fixture manifests that were defined in `files` but never referenced by any `features` entry (dead code). Add a test that catches unreferenced file entries going forward.
williballenthin
force-pushed
the
data-fixtures-2
branch
from
5eec048 to
ac69d73
Compare
2 participants
closes #2743
This PR refactors our test suite with the primary goal of making the feature tests data-driven; that is, we now have a collection of JSON files that describe where backends should find various features. This enables non-Python implementations of capa to demonstrate that they extract the features we'd expect.
Along the way, I also made some other cleanups. In particular, I tried to simplify
fixtures.pya bit. I removed a lot of the symbolic extractors (e.g.,pma01_03_extractor) since many were used only once or twice in other modules; I inlined this logic with more explicit code. I figure, more obvious code -> easier to mechanically port to another language. Pytest fixtures as magic test function parameter names is not that easy to reason about...To the reviewers, I request a thorough review of not just the code style, but of the underlying implementation, design, decisions, etc. I think you should set aside an hour for this.
Here's how I'd recommend that you review these changes:
test_viv_extractor.pyto see how a backend test now lookstests/fixtures/features/README.mdandstatic.jsonto see how the features are declaredfixtures.pyfor familiarity. don't read the diff, read the final.To be clear, there's moderate risk here: I've translated the existing feature test cases to a new format, and I might have missed or corrupted some of them. In some scenarios, I tried to consolidate feature test cases (like when IDA only worked on some samples, and Ghidra on a different set) and it's possible that I mangled the underlying intent. However, I did my best and the test suite probably high quality. I believe we should accept this risk, but you should think about this, too.
Checklist