tests: migrate to data-driven fixtures for feature presence - #2986
Merged
Conversation
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
williballenthin
commented
Apr 2, 2026
williballenthin
commented
Apr 2, 2026
williballenthin
force-pushed
the
data-fixtures-2
branch
from
April 2, 2026 14:06
71b4687 to
dc455d1
Compare
williballenthin
force-pushed
the
data-fixtures-2
branch
6 times, most recently
from
April 15, 2026 10:13
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
April 21, 2026 12:44
b68984e to
c74ae97
Compare
github-actions
Bot
dismissed
their stale review
April 21, 2026 12:46
CHANGELOG updated or no update needed, thanks! 😄
williballenthin
force-pushed
the
data-fixtures-2
branch
from
April 21, 2026 13:48
ac39fda to
0a35068
Compare
williballenthin
marked this pull request as ready for review
April 21, 2026 13:50
williballenthin
force-pushed
the
data-fixtures-2
branch
from
April 23, 2026 15:07
55e8721 to
e294e69
Compare
mr-tz
reviewed
Apr 24, 2026
mr-tz
left a comment
Collaborator
There was a problem hiding this comment.
nice work, great step into a data-driven test direction - a few comments to discuss inline
williballenthin
force-pushed
the
data-fixtures-2
branch
from
April 28, 2026 17:49
5eec048 to
ac69d73
Compare
williballenthin
force-pushed
the
data-fixtures-2
branch
2 times, most recently
from
May 11, 2026 07:06
e1fb1ff to
f0c8c3f
Compare
williballenthin
force-pushed
the
data-fixtures-2
branch
from
May 11, 2026 07:14
f0c8c3f to
e71731e
Compare
mr-tz
reviewed
May 11, 2026
mr-tz
left a comment
Collaborator
There was a problem hiding this comment.
looking like it's on a good track!
Collaborator
Author
|
awaiting CI outcome. i have no further pending TODOs, so recommend a full review and decision on whether to merge. |
Collaborator
|
Ha! Just went through it so assuming CI passes, we're good to go IMHO. |
mr-tz
approved these changes
May 11, 2026
williballenthin
force-pushed
the
data-fixtures-2
branch
from
May 11, 2026 08:36
d1c58da to
4fc986f
Compare
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
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.
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