Skip to content

Conversation

@nflexfo
Copy link
Contributor

@nflexfo nflexfo commented Nov 25, 2025

One line description of pull request

Factorizes duplicated code in Pinfo.

Description:

In plaso/cli/pinfo_tool.py, both methods _CalculateStorageCounters() and _CompareStores() have duplicated codes that only varies in data but not in behavior. Like any duplicated code, modifying one path implies to also modify all the other paths. This is error prone and it hurts extensibility.

This merge request, factorize those code and provides a test basis for said methods. Also, it cuts the code footprint by half. The behavior should be identical and printing differences in storage preserves the same order.

Additional Context

I'm working on adding a new "data type counter" into the storage file. I already have a working patch but it hurts me to add another duplicated code in those paths. So, this is kinda of a "pre-requisite" patch.

Concerning this MR itself, I have a few questions (mostly regarding tests):

  • I've named the test function for _CompareStores, testPrivateCompareStores since testCompareStores already exists. Is this name fine?
  • I'm not entirely convinced by those "mocks" in testPrivateCompareStores. Is this how you would approach it or do you prefer checking it from an actual file instead?
  • The counters list in _CompareStores sounds like something that should goes into class attribute, so we have a "single source of truth" (both in code and tests). What do you think about it?

Regards,

@nflexfo nflexfo marked this pull request as draft November 25, 2025 10:18
@nflexfo nflexfo mentioned this pull request Nov 26, 2025
4 tasks
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