-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Revamp the end-of-test summary #4089
Open
joanlopez
wants to merge
47
commits into
master
Choose a base branch
from
new-end-of-test-summary-output
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
+3,315
−536
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Co-authored-by: oleiade <[email protected]>
Co-authored-by: oleiade <[email protected]>
Co-authored-by: oleiade <[email protected]>
Co-authored-by: oleiade <[email protected]>
Co-authored-by: oleiade <[email protected]>
Co-authored-by: oleiade <[email protected]>
Co-authored-by: oleiade <[email protected]>
Co-authored-by: oleiade <[email protected]>
Co-authored-by: oleiade <[email protected]>
Co-authored-by: oleiade <[email protected]>
joanlopez
requested review from
mstoykov and
olegbespalov
and removed request for
a team
December 4, 2024 17:58
oleiade
force-pushed
the
new-end-of-test-summary-output
branch
from
December 17, 2024 15:58
58138ac
to
126a188
Compare
…6 into new-end-of-test-summary-output
…6 into new-end-of-test-summary-output
joanlopez
force-pushed
the
new-end-of-test-summary-output
branch
from
February 7, 2025 21:11
1904999
to
4bb8f7a
Compare
joanlopez
force-pushed
the
new-end-of-test-summary-output
branch
from
February 7, 2025 21:14
4bb8f7a
to
9c1ed70
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
This pull request changes the current end-of-test summary by a new design, with two available formats:
aiming to bring clearer a more valuable results to users. Find a screenshot below.
User-facing details
compact
summary with--with-summary=compact
, or with no argument, as it is the default choice.full
summary with--with-summary=full
.legacy
summary with--with-summary=legacy
.handleSummary
function is now different. So, those users relying on it must migrate their implementation or use--with-summary=legacy
meanwhile.Technical details (for review purposes)
output.Output
namedsummary
.internal/cmd/testdata/summary/...
with different scenarios, groups, thresholds, custom metrics and what not... that can be used for both automated and manual testing. If you think anything is missing, just suggest it.lib.Summary
vslib.LegacySummary
), to keep support for the legacy summary for some time, in an easy way, so things aren't complexity mixed and the the clean up in the future is simpler, just by removing that type, all the references to it, and simplifying the few conditionals that behave depending on which summary type is provided.summary-legacy.js
, for simpler cleanup whenever we remove that support, which I guess might be for v2 (once we ship the formalized JSON output format within v1).Internal Checklist
Before review readiness
lib.Report
as the new API for customhandleSummary
(note this would be a breaking change), or if we want to ship this progressively.js/summary.go
according to that decision.js/summary.js
:summarizeMetrics
andsummarizeMetricsWithThresholds
, or just replace the first with the second one (as we may no longer need the first one if we remove the old summary code).output/summary
package:playground/full-summary
files, or define a proper location for them.--- Ideas left for a second iteration---
General
make lint
) and all checks pass.make tests
) and all tests pass.