On Wed, 27 Sep 2023 07:11:59 GMT, Jaikiran Pai <j...@openjdk.org> wrote:
> Can I please get a review of this change which addresses the issue noted in > https://bugs.openjdk.org/browse/JDK-8316946? > > Failure handler actions can specify a "successArtifacts" property value, > which are file name(s) for which hypherlinks will be created in the > "processes.html" file. This property value can contain replacable placeholder > tokens like "%p" (process id) "%iterCount" (iteration count, if the action's > command is repeated). > > The issue here was caused by a bug in the handling of the "successArtifacts" > value. If there were more than one process for which we were running the > action (of collecting thread dumps), then the original (raw) value of > "successArtifacts" property wasn't being held on to. As a result, for the > second process for which the hyperlink was being created, an incorrect link > would be generated. > > The commit here retains the raw value of "successArtifacts" property in the > `PatternAction` (just like we do for "args" property) so that it can be used > to repeatedly replace the placeholders, each time the action is run for a > different process. > > I've run this change against our internal CI to verify it doesn't cause any > regressions. tier1, tier2 and tier3 was run successfully. Additionally, I > have run a couple of jtreg test cases which intentionally timeout, so that I > can verify that the generated "processes.html" now contains the correct > hyperlinks to the thread dump files for each process. Based on the description this change looks good. Thanks. ------------- Marked as reviewed by dholmes (Reviewer). PR Review: https://git.openjdk.org/jdk/pull/15939#pullrequestreview-1647620646
