Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Hide details when report exceeds size limit #278

Merged
merged 3 commits into from Jun 2, 2022

Conversation

maciejtutak
Copy link
Contributor

@maciejtutak maciejtutak commented Jun 2, 2022

This PR is based on the discussion in #262
Fixes #262

Hello,
Thank you for the detailed explanations and possible solutions. In the end, I decided to implement v1 of the discussed solutions in #262. The action will now omit coverage details spoilers below the summary if the report size exceeds the GitHub limit, like in the picture:

picture

All in all, I think this is a pretty edge-casey issue, as usually PRs shouldn't grow that big in size, except when one does do a significant refactor. This solution is not perfect - it only strips the details, and doesn't cover for the case where the failed test results are a majority of the coverage report.

I'm not familiar enough with the tool to implement a smart truncation mechanism properly.

In my opinion, that would require either a bigger refactor or just drilling a param to truncate the details through various formatCoverage helper functions all the way to the bottom. We could think of perhaps, always truncating more than 50 rows of the tables, as I feel like that's too much already for a PR. Doing it that way, we could avoid drilling parameters. But that may not solve the issue of reports being too long that we set out to fix in the first place :P

@ArtiomTr ArtiomTr added the enhancement New feature or request label Jun 2, 2022
Copy link
Owner

@ArtiomTr ArtiomTr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect 👍

@ArtiomTr
Copy link
Owner

ArtiomTr commented Jun 2, 2022

@maciejtutak, thank you for your contribution.

Looks like you accidentally deleted tests/format/details/__snapshots__/getFileCoverageDetailRow.test.ts.snap file. Because of that, some tests are failing. Could you please run npm test -- -u to update all snapshots?

@maciejtutak
Copy link
Contributor Author

I brought it back, my bad :)

@ArtiomTr ArtiomTr merged commit 47b8df8 into ArtiomTr:main Jun 2, 2022
@ArtiomTr
Copy link
Owner

ArtiomTr commented Jun 2, 2022

@all-contributors please add @maciejtutak for code

@allcontributors
Copy link
Contributor

@ArtiomTr

I've put up a pull request to add @maciejtutak! 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Generate truncated report if limit 65535 reached
2 participants