[prev in list] [next in list] [prev in thread] [next in thread]
List: openjdk-serviceability-dev
Subject: Re: RFR: 8244133: Refactor nsk/jdi tests to reduce code duplication in settingBreakpoint communicati
From: "serguei.spitsyn () oracle ! com" <serguei ! spitsyn () oracle ! com>
Date: 2020-04-30 4:53:26
Message-ID: cd907f8b-5de1-630e-5883-8e2b2b1fc123 () oracle ! com
[Download RAW message or body]
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body>
<div class="moz-cite-prefix">Hi Leonid,<br>
<br>
The fix looks good.<br>
Thank you for taking care about it!<br>
I hope we can do the same for the rest of files that have similar
copy-paste fragments.<br>
Also, the JDIBase class needs refactoring and improvements.<br>
I know you are up to it though. :)<br>
<br>
Thanks,<br>
Serguei<br>
<br>
<br>
On 4/29/20 16:45, Leonid Mesnik wrote:<br>
</div>
<blockquote type="cite"
cite="mid:53C1DDD1-5FB7-47E8-9DE3-1DA0DE84579B@oracle.com">Hi
<div class=""><br class="">
</div>
<div class="">Could you please review following fix which remove
code duplication by moving methods BreakpointRequest
settingBreakpoint, getEventSet, breakpointForCommunication and
log1,2,3 in base class.</div>
<div class=""><br class="">
</div>
<div class="">The fix is huge but pretty straight forward, I tried
to keep changes as small as possible so most tests just changed
to extends JDIBase and corresponding methods and fields were
deleted. </div>
<div class=""><br class="">
</div>
<div class="">Some tests used slightly different
'breakpointForCommunication()' implementation so they override
it now. </div>
<div class=""><br class="">
</div>
<div class="">Some tests contain change like this:</div>
<div class="">
<pre class=""><span class="removed">- eventRequest1 = \
eventRManager.createBreakpointRequest(location);</span> <span class="new">+ \
eventRequest1 = eventRManager.createBreakpointRequest(breakpLocation);</span></pre> \
<div class="">it is because they had 'location' and not 'breakpLocation' which is \
set by settingBreakpoint(...). So just merged location and breakpLocation.</div>
</div>
<div class=""><br class="">
</div>
<div class="">All tests passed, so the main goal is to ensure that
changes don't introduce false positive results ignoring exit
codes and statuses.</div>
<div class=""><br class="">
</div>
<div class="">I quickly verified that updated files don't contain
declarations of variable like PASSED, testExitCode and other.</div>
<div class="">
<div class=""><br class="">
</div>
<div class="">The JDIBase and all tests could be improved, also
there are still a lot of tests which could use it as a base as
well as other base classes for idi debuggers. However I want
to keep this fix as simple as possible.</div>
<div class=""><br class="">
</div>
<div class="">werbev: <a
href="http://cr.openjdk.java.net/~lmesnik/8244133/webrev.00/"
class="" \
moz-do-not-send="true">http://cr.openjdk.java.net/~lmesnik/8244133/webrev.00/</a></div>
<div class="">bug: <a
href="https://bugs.openjdk.java.net/browse/JDK-8244133"
class="" \
moz-do-not-send="true">https://bugs.openjdk.java.net/browse/JDK-8244133</a></div> \
</div> </blockquote>
<br>
</body>
</html>
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic