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.

Sign up for GitHub

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

Jump to bottom

Fix HTML report merging for multiple suites #105

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix HTML report merging for multiple suites #105

wants to merge 1 commit into from

Conversation

reinholdfuereder
Copy link
Contributor

Fixes #69

So far only the suites of the first HTML report were in the final merged HTML report.
And when the first HTML report (luckily) already contained all suites, then all tests from the other HTML reports were always merged into the first suite (instead of the same non-first one).

@reinholdfuereder
Fix HTML report merging for multiple suites
9d7c9bf 
So far only the suites of the first HTML report were in the final merged HTML report.
And when the first HTML report (luckily) already contained all suites, then all tests from the other HTML reports were always merged into the first suite (instead of the same non-first one).
@reinholdfuereder reinholdfuereder mentioned this pull request Apr 21, 2022
Open
@ccsuperstar
Copy link
Contributor

ccsuperstar commented Apr 28, 2022 •
edited
Loading

Thanks a lot, I still have a problem, with duplicate suite
Capture d’écran 2022-04-28 à 11 34 57

And in a parallel run context, each run has an execution time
The final merge of the reports aggregates each of the reports
However, in my opinion, the execution time of the final merge should be the execution time of the longest run and not the sum of the execution times of each run

Would it be possible to do it in this PR, or am I opening a new issue?

@reinholdfuereder
Copy link
Contributor Author

Thanks a lot, I still have a problem, with duplicate suite ...

And in a parallel run context, each run has an execution time The final merge of the reports aggregates each of the reports However, in my opinion, the execution time of the final merge should be the execution time of the longest run and not the sum of the execution times of each run

Would it be possible to do it in this PR, or am I opening a new issue?

Hm, I (as a complete newbie to this project) would say please open a new issue for that, because:

  • this (summing up of execution times) seems to be implemented on purpose like that (even the method comments claim that) and is not just a bug,
  • and I am also not really sure what users would expect: I think in fact both execution times (sum and maximum) may make sense or may be of interest?

@vansari vansari self-requested a review July 10, 2022 19:38
@vansari vansari marked this pull request as draft July 10, 2022 19:41
@vansari
Copy link
Collaborator

vansari commented Jul 10, 2022

@reinholdfuereder Thank you for your contribution. Is it possible that you write a Unit Test which covers this new changes? Thanks a lot.

@vansari vansari marked this pull request as ready for review November 13, 2022 13:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vansari vansari Awaiting requested review from vansari

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Merged report with multiple suites
3 participants
@reinholdfuereder @ccsuperstar @vansari