Updates to stressgraphics #20886
Open
Updates to stressgraphics #20886
+35
−431
Conversation
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
hageboeck
force-pushed
the
stressgraphics
branch
from
f3afa19 to
dad5cc9
Compare
- Use non-zero exit code when a test fails. - When a test fails, print the file name of the generated graphics. - Label all stressGraphics runs as longtest. - Print differences in SVGs similar to how a unified diff would be printed.
In the past, stressGraphics was retrieving a histogram from a file, modifying it and drawing it. In subsequent tests, the histogram is again read from the TFile, but this returns the in-memory object. This means that the previous tests influence the subsequent tests, e.g. whether or not statistics objects are displayed. In particular, subsequent tests will have the statistics box drawn twice, once from THistPainter::PaintStat(), and once from the fact that the box is added to the list of functions. This impacts output files such as PDF/PS/SVG. Here, the histogram is always cloned when it is retrieved from the file, so the tests are independent. The reference files for SVG are regenerated, because the statistics box is now only drawn once. Similarly, the output size of the .ps files has been corrected to reflect that the boxes are only drawn once.
hageboeck
force-pushed
the
stressgraphics
branch
from
dad5cc9 to
4276a2d
Compare
Test Results 23 files 23 suites 3d 19h 57m 13s ⏱️ For more details on these failures, see this check. Results for commit 4276a2d. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
1. Add a few small improvements for stressgraphics
printed.
2. Needed for #18083: Always use a new histogram for the tests
This makes the test future proof, working with or without implicit ownership of histograms.
Otherwise, due to TFile possibly returning a histogram that has been drawn in another test, tests can influence each other. This manifests in the stats box being drawn twice (once in the stats draw function of THistPainter, and a second time when the list of functions is drawn). This leads to a test failure in the svg tests, because here it's visible whether drawing happens once or twice, even if the output looks the same.
With this PR, the test will always use a fresh histogram, making the output independent of whether the same histogram is used in another test.
(I also tried a bit to remove the double drawing of the stats box, but this breaks other features like adding a custom stats box to the list of functions or enabling / disabling the stats box using the stats option. Always cloning the original histogram is the most robust option.)
Note:
My clang-format did not have any issues, so the failure looks to be due to version differences in clang-format