Chore: refactor state export

[NicolasMassart]

What is this about?

State export feature in app/util/logs/index.ts and app/util/logs/index.test.ts are not following good practices and it makes it hard and complex to test.
Long term this will increase the risk of tests being hard to understand and lead to untested parts and possible data being included in export when they should not.

Scenario

No response

Design

No response

Technical Details

• Two exported functions downloadStateLogs and generateStateLogs
• downloadStateLogs calls generateStateLogs internally
• Test is testing generateStateLogs output
• Test is testing downloadStateLogs output (and so testing also generateStateLogs again)
• tests can’t be isolated as both functions are in the same module
• No spying possible on generateStateLogs because the spy would be on the module export ref when downloadStateLogs uses the module internal ref. So your spy behave like it’s never called.
• We test generateStateLogs that should be private!
• generateStateLogs is exported only for test purpose! (avoid doing things only for test purpose, and here it's really not hard to prevent)
• the only way to properly test this without refactoring is a complex mock that increases risks of misunderstanding and not properly testing things (risk of testing the mock instead of the real code)
• The real solution is a refactoring of the original code either to only test public code (but requires to catch the downloaded base 64 encoded data and decode it, see feat: add metametricsid in state export #12621, not the best) or to export generateStateLogs as a util function and test is separately and be able to spy on it in the downloadStateLogs run.

The refactoring should make each part under test testable in an isolated way. This export feature is not complex enough to say it can't be refactored, so we should refactor it. All the code base will benefit from it and this provides a good example of how to write testable code.

Threat Modeling Framework

• with this refactoring and making it better tested, we lower the risk of data being added in the export when it should not.

Acceptance Criteria

• all code is properly tested without complicated workaround
• no artificial export for test use
• unit tests properly scoped to the code under test
• all tests pass
• not change on the feature behaviour

Stakeholder review needed before the work gets merged

• Engineering (needed in most cases)
• Design
• Product
• QA (automation tests are required to pass before merging PRs but not all changes are covered by automation tests - please review if QA is needed beyond automation tests)
• Security
• Legal
• Marketing
• Management (please specify)
• Other (please specify)

References

See also #12621