feat: exclude delay from test execution time in summary #3388
Conversation
Currently, the test summary's 'Total time taken' includes the user-provided delay (--delay flag), which makes the metric misleading as it includes application startup wait time. Changes: - Moved timing start point to after application delay - Updated label from 'Total time taken' to 'Total test execution time' - Updated per-testset summary labels for consistency This ensures the reported time accurately reflects actual test execution duration, excluding initialization delays. Closes keploy#3351 Signed-off-by: WillyEverGreen <saibalkawade10@gmail.com>
WillyEverGreen
requested review from
Sarthak160 and
gouravkrosx
as code owners
|
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
|
The CLA check failed. Please ensure you have:
After fixing these issues, comment 'recheck' to trigger the workflow again. |
1 similar comment
|
The CLA check failed. Please ensure you have:
After fixing these issues, comment 'recheck' to trigger the workflow again. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
|
I have read the CLA Document and I hereby sign the CLA. |
|
recheck |
pkg/service/replay/replay.go
Outdated
| } | ||
|
|
||
| // Start timing test execution after application delay | ||
| startTime := time.Now() |
| runTestSetCtx = context.WithValue(runTestSetCtx, models.ErrGroupKey, runTestSetErrGrp) | ||
| runTestSetCtx, runTestSetCtxCancel := context.WithCancel(runTestSetCtx) | ||
|
|
||
| startTime := time.Now() |
There was a problem hiding this comment.
why was this removed?
please revert this
|
the pipelines are failing. did you test this PR locally? |
|
Thanks for pointing this out — you're right. By moving startTime I ended up affecting the execution flow instead of just the reporting logic, which wasn’t the intention. That explains the pipeline failures as well. I’ll revert the timer change back to its original place and update the summary calculation so the reported execution time excludes the --delay duration without touching the core execution lifecycle. I’ll also run the tests locally before pushing the fix. |
Track user-provided delay time separately and subtract it from the total execution time when printing the test run summary. This ensures the reported time accurately reflects actual test execution duration. Closes keploy#3351 Signed-off-by: WillyEverGreen <saibalkawade10@gmail.com>
WillyEverGreen
force-pushed
the
fix/3351-exclude-delay-from-summary
branch
from
7ddf505 to
306d6d0
Compare
2 participants
Currently, the test summary’s reported execution time includes the user-provided application delay (
--delayflag), which makes the metric misleading as it also accounts for startup wait time rather than actual test execution.This PR corrects the timing calculation so that only the real test execution duration is reported.
Closes #3351
Describe the changes that are made
Total time takentoTotal test execution timeLinks & References
Closes: #3351
What type of PR is this? (check all applicable)
Added e2e test pipeline?
Added comments for hard-to-understand areas?
Added to documentation?
Are there any sample code or steps to test the changes?
Self Review done?
Any relevant screenshots, recordings or logs?