Merge the Report._files/_chunks

[Swatinem]

Instead of maintaining two separate fields, this merges the previous _files which was a dict from filename to totals, and the _chunks which was a list of split chunks, themselves optionally a fully parsed ReportFile.

Maintaining only a single structure for the list of files makes a ton of internals of the Report simpler.
It also solves the problem of file_totals getting out of sync with the ReportFile.totals.

Instead of maintaining a FileSummary dataclass, and a separate ReportFile, this merges these two structures together, and makes sure that a Report always has ReportFiles.

Though the new ReportFile has a bit more logic around making parsing of its actual chunk contents on-demand only when needed, and avoid having to parse things when not strictly necessary.

---

I think the ~40% perf regression is reasonable, as it is doing more work now for a "shallow" parse. The slowness comes from the ReportFile constructor being slow. Part of that is because it has to support various arguments and argument types which are primarily only used within tests. In real usage, we either parse ReportFiles from json/chunks, or start with an empty file that has lines added/replaced into it.

[overwatch-beta]

✅ Sentry found no issues in your recent changes ✅

[codspeed-hq]

CodSpeed Performance Report

Merging #553 will degrade performances by 42.1%

_{Comparing swatinem/one-file (824b3bf) with main (ec1378b)}

Summary

⚡ 1 improvements
❌ 4 regressions
✅ 12 untouched benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

	Benchmark	BASE	HEAD	Change
❌	test_parse_shallow[ReadOnlyReport]	7.4 ms	12.5 ms	-41.2%
❌	test_parse_shallow[Report]	7.2 ms	12.3 ms	-41.82%
❌	test_parse_shallow[Rust ReadOnlyReport]	7.5 ms	12.9 ms	-42.1%
⚡	test_report_carryforward	6.1 ms	2.9 ms	×2.1
❌	test_report_serialize	6.1 ms	7.1 ms	-15.01%

[codecov]

Codecov Report

Attention: Patch coverage is 96.12403% with 5 lines in your changes missing coverage. Please review.

Project coverage is 88.83%. Comparing base (ec1378b) to head (824b3bf).

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
shared/reports/resources.py	93.42%	1 Missing and 4 partials ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #553      +/-   ##
==========================================
- Coverage   88.88%   88.83%   -0.05%     
==========================================
  Files         461      461              
  Lines       13192    13134      -58     
  Branches     1519     1505      -14     
==========================================
- Hits        11726    11668      -58     
- Misses       1142     1145       +3     
+ Partials      324      321       -3     

Flag	Coverage Δ
shared-docker-uploader	88.83% <96.12%> (-0.05%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[codecov-notifications]

Codecov Report

Attention: Patch coverage is 96.12403% with 5 lines in your changes missing coverage. Please review.

✅ All tests successful. No failed tests found.

Files with missing lines	Patch %	Lines
shared/reports/resources.py	93.42%	1 Missing and 4 partials ⚠️

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:

• ❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[giovanni-guidini]

Very good change!

[giovanni-guidini]

[very nit] now it returns Report, although they might be the same thing?

Docs also mention that it "creates a new report", but the code suggests all changes are done in-place, and the same report is returned.

[giovanni-guidini]

I wonder if that could pose a problem... say we use commit.parent report to build a "new" carryforward report for commit.

Would we save the modified commit.parent report back with the changes? Probably not, but if it's possible it might be a good idea to explicit that in the documentation

[Swatinem]

Thanks for reminding me about this. Indeed this is a behavior change, and I need to update the docs, and make sure that the calling code is aware of this.

[giovanni-guidini]

It seems that silently ignoring the bind argument (and the _else to some extent) could lead to unexpected behavior.

I'm sure this way it's easier to integrate with the rest of the code, but maybe it would be better to fail here if people are using those extra options

[Swatinem]

thats a good point. Python is also a bit annoying with *args vs **kwargs as well which can cause problems.
regardless of that, I believe the bind argument was unused before. I was fixing a bug with that recently as it was completely broken otherwise. So I’m pretty sure noone was using that argument.

I think this PR will need quite a bit of refactoring for downstream code as well, which I haven’t started yet, but will do so soonish.