Test result duration is not reset by test restarts or guest reboots#4928
Open
happz wants to merge 3 commits into
Open
Test result duration is not reset by test restarts or guest reboots#4928happz wants to merge 3 commits into
happz wants to merge 3 commits into
Conversation
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors execution timing by replacing class-level stopwatch measurements with instance-level tracking. It introduces a stopwatch instance field on TestInvocation to replace individual start_time, end_time, and real_duration fields, and updates the Stopwatch class to support instance-based measurement with a new started property. I have no feedback to provide as there are no review comments.
Every test start got new `Stopwatch` instances, new measurement, and only the last one was stored in the corresponding result. The patch does the following: * `Stopwatch.measure()` is no longer a classmethod. To be restartable, and retain its original starting point, an instance is needed, and it is really, *really* hard to make a method both instance and classmethod. * `Stopwatch.measure()` does not reset its startign poitn when it's already set. This allows stopwatch to be restartable, continuing the measurement. * Instead of a fresh one `Stopwatch` every time a test is started, `Stopwatch` instance is attached to `TestInvocation` instance owning the test. `execute/tmt` then calls `this instance's `measure()` method every time the test is started - thanks to its starting point not beign reset every time, `TestInvocation.stopwatch` now records the duration across all reboots and restarts. * And instead of calling `format_duration()`, `Result` construction accessess properties of the aforementioned `Stopwatch` instance, getting the right strings every time.
32fb248 to
2ebac01
Compare
This file contains hidden or 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
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.
Every test start got new
Stopwatchinstances, new measurement, and only the last one was stored in the corresponding result. The patch does the following:Stopwatch.measure()is no longer a classmethod. To be restartable, and retain its original starting point, an instance is needed, and it is really, really hard to make a method both instance and classmethod.Stopwatch.measure()does not reset its startign poitn when it's already set. This allows stopwatch to be restartable, continuing the measurement.Stopwatchevery time a test is started,Stopwatchinstance is attached toTestInvocationinstance owning the test.execute/tmtthen callsthis instance'smeasure()method every time the test is started - thanks to its starting point not beign reset every time,TestInvocation.stopwatch` now records the duration across all reboots and restarts.format_duration(),Resultconstruction accessess properties of the aforementionedStopwatchinstance, getting the right strings every time.Pull Request Checklist