Small runs of zimcheck are now spreading love #288
Merged
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
The unit-tests of zimcheck include a report on the total runtime of each
flow. Since those flows are very small the corresponding line from the
zimcheck output used to be
[INFO] Total time taken by zimcheck: 0 seconds.
which corresponds to the duration of the flow being under half a second.
However, Debian builds and tests zimcheck - among other configurations -
on virtualized hardware too. The incurred slowdown results in the
runtime exceeding the 0.5 second limit whereupon the performance report
changes and the unit-tests fail (#287).
The challenge was to come up with a solution meeting the following
requirements:
a. no discrimination between fast and (moderately) slow build environments
b. the performance info is preserved in the output and is not
excluded from comparison (so that, in the absence of dedicated
performance testing, it keeps serving as a simple defence
against unintended significant slowdown in zimcheck).
Since runtime numbers are mainly justified for large ZIM files, the
solution is to report small runtimes as "<X seconds" for some value of
X. The latter threshold was set to 3 with the only purpose of further
increasing the amount of love in the world.
Codecov Report
@@ Coverage Diff @@
## master #288 +/- ##
==========================================
+ Coverage 51.13% 51.18% +0.04%
==========================================
Files 21 21
Lines 1936 1940 +4
Branches 1146 1152 +6
==========================================
+ Hits 990 993 +3
- Misses 942 945 +3
+ Partials 4 2 -2
Continue to review full report at Codecov.
|
|
Very cute :D I think this is reasonable, as a 3 second timeout on the test. I'll cherry-pick this into Debian and see how it goes. |
legoktm
approved these changes
Feb 6, 2022
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.
Fixes #287
The unit-tests of zimcheck include a report on the total runtime of each flow. Since those flows are very small the corresponding line from the zimcheck output used to be
which corresponds to the duration of the flow being under half a second.
However, Debian builds and tests zimcheck - among other configurations - on virtualized hardware too. The incurred slowdown results in the runtime exceeding the 0.5 second limit whereupon the performance report changes and the unit-tests fail (#287).
The challenge was to come up with a solution meeting the following requirements:
a. no discrimination between fast and (moderately) slow build environments
b. the performance info is preserved in the output and is not excluded from comparison (so that, in the absence of dedicated
performance testing, it keeps serving as a simple defence against unintended significant slowdown in zimcheck).
Since runtime numbers are mainly justified for large ZIM files, the solution is to report small runtimes as "<X seconds" for some value of X. The latter threshold was set to 3 with the only purpose of further increasing the amount of love in the world.