lucassabreu / comment-coverage-clover

Github Action that automatically adds a comment with a summary of coverage reports (clover)
https://github.com/marketplace/actions/coverage-report-as-comment-clover
15 stars 4 forks source link

feature request: combine multiple clover reports into a single segmented pr comment #40

Closed klippx closed 1 year ago

klippx commented 1 year ago

Screenshot 2023-06-05 at 14 46 56

I work in a javascript monorepo with lots of workspaces. Each workspaces has their own test suite and their own coverage (distributed ownership).

Now, if a PR touches multiple workspaces, comment-coverage-clover will create a PR comment for each of the workspace touched. See screenshot. It then works correctly and keeps updating individual comments with corresponding changes with the associated workspace.

The HTML of the comments are in (react) high level pseudo code:

files.forEach((filename) => 
  <Comment key={filename}>    // One comment for each workspace
    <CommitSummary /> // Same commit sha for all comments!
    <FileSummary filename={filename} />
    <AsciiGraph />
    <Foldable>
      <Summary />
    </Foldable>
  </Comment>
);

I want to raise the discussion asking if it would it be nicer to have the option of getting one single comment? Pseudo code:

<Comment>  // NEW: One single comment for all workspaces
  <CommitSummary /> // NEW: One single commit summary since it is always "latest"
  files.forEach((filename) => 
    <div id={filename}>
      <FileSummary filename={filename} />
      <Foldable> // NEW: Fold each workspace summary as to not have a huge graph expanded for each of them
        <AsciiGraph />
        <Foldable>
          <Summary />
        </Foldable>
      </Foldable>
    </div>
  );
</Comment>

To summarize:

I guess the challenge is when posting many comments to keep updating the same PR comment (concurrently?) and only updating the relevant section. Perhaps not even possible? I mean, sure, it is possible but might require a server or comment locking due to concurrency or something else that makes it totally unfeasible.

WDYT?

lucassabreu commented 1 year ago

hi @klippx , just to be sure i understood it...

you have many clover reports; they are created from the same commit; but from different inner projects.

for each one of those reports one comment is created; but you would like that all of them were "merged" into one comment.

but, the merged comment would have the summary combined for all of them, and have a chart and table for each individual report file?

if i understood it correctly there is a problem that the comment has a limit size; and creating this many tables & charts will consume the limit very quickly, so i don't think we could do that.

if you "just" want one comment for all of the clover reports combined; it could be done by providing a merged file of the individual ones.

PHP has tools for that; so i believe Node must have too.

the merging of files should be solved by another action before calling this one; so i do not think it should be done here.

lucassabreu commented 1 year ago

if you meant something else; please elaborate

klippx commented 1 year ago

Ah, I see, good answer, makes sense! I will close the request, as I dont think it seems like a feasible approach considering the limitations.

I also think it makes sense that the user combines the report, if needed.