bloomberg / pytest-memray

pytest plugin for easy integration of memray memory profiler
https://pytest-memray.readthedocs.io/en/latest/
Apache License 2.0
338 stars 25 forks source link

Report memory limit and allocated memory in longrepr #5

Closed petr-tik closed 2 years ago

petr-tik commented 2 years ago

Make the longrepr more descriptive and enrich the xml output with this information.

Refactor limit_memory to return a _MemoryInfo type, which can be presented as a PytestSection and a longrepr string.

Remove item.nodeid from longrepr since longrepr is only used by junit-xml parser and the structure of the xml document makes the item.nodeid in the error message redundant.

Testing performed Add a test that parses the resulting xml file to make sure the expected string was written.

Issue number of the reported bug or feature request: #3

gaborbernat commented 2 years ago

@petr-tik can you try to resolve the merge conflicts?

petr-tik commented 2 years ago

@gaborbernat i have. please take a look when you can

petr-tik commented 2 years ago

Thanks for the detailed review with code style and refactor suggestions.

Addressed all of them. Only have 1 REVIEW comment about the name of the dataclass, happy to remove the comment, if you find the type name descriptive enough.

petr-tik commented 2 years ago

do you want me to edit the PR message before merging?

gaborbernat commented 2 years ago

If you have any amendments you think makes sense, sure 👍

petr-tik commented 2 years ago

i wanted to tidy it up and leave a descriptive commit message in main

gaborbernat commented 2 years ago

Go ahead with that then 👍

petr-tik commented 2 years ago

Done. I am guessing you are the one with squash and merge rights?

pablogsal commented 2 years ago

Thanks a lot @petr-tik for the fantastic work and for persuing getting this fixed ❤️

And thanks a lot @gaborbernat for the review! 💪

petr-tik commented 2 years ago

oh, my PR message wasn't included in the commit.

Thanks for the detailed review. Absolute pleasure