jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Philippe Mouawad <philippe.moua...@gmail.com>
Subject Re: svn commit: r1859277 - in /jmeter/trunk: src/core/org/apache/jmeter/samplers/SampleResult.java test/src/org/apache/jmeter/samplers/TestSampleResult.java xdocs/changes.xml
Date Wed, 15 May 2019 11:35:19 GMT
Hi Felix,

Sorry for that, I have to refrain myself from proceeding this way.
I started refactoring, didn't commit , then fixed bug and finally got lazy
to revert and committed

Will try to do better next time.

PS : Did you have time reviewing https://github.com/apache/jmeter/pull/458
?
Thanks
Regards

On Wed, May 15, 2019 at 12:19 PM Felix Schumacher <
felix.schumacher@internetallee.de> wrote:

> Hi Philippe,
>
> I think it would have been nice to split these changes into two or more
> commits.
>
> One with the fix for not storing null pointers in the set together with
> the test case and another one for the refactoring.
>
> It would be easier -- at least for me -- to spot the real change in that
> way.
>
> All in all, nice to see you fixing that fast the bugs :)
>
>  Felix
>
> Am 15.05.19 um 10:31 schrieb pmouawad@apache.org:
> > Author: pmouawad
> > Date: Wed May 15 08:31:06 2019
> > New Revision: 1859277
> >
> > URL: http://svn.apache.org/viewvc?rev=1859277&view=rev
> > Log:
> > Bug 63433 - ListenerNotifier: Detected problem in Listener
> NullPointerException if filename is null
> > Bugzilla Id: 63433
> >
> > Modified:
> >     jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleResult.java
> >
>  jmeter/trunk/test/src/org/apache/jmeter/samplers/TestSampleResult.java
> >     jmeter/trunk/xdocs/changes.xml
> >
> > Modified:
> jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleResult.java
> > URL:
> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleResult.java?rev=1859277&r1=1859276&r2=1859277&view=diff
> >
> ==============================================================================
> > --- jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleResult.java
> (original)
> > +++ jmeter/trunk/src/core/org/apache/jmeter/samplers/SampleResult.java
> Wed May 15 08:31:06 2019
> > @@ -61,6 +61,7 @@ public class SampleResult implements Ser
> >
> >      private static final String OK_CODE =
> Integer.toString(HttpURLConnection.HTTP_OK);
> >      private static final String OK_MSG = "OK"; // $NON-NLS-1$
> > +    private static final String INVALID_CALL_SEQUENCE_MSG = "Invalid
> call sequence"; // $NON-NLS-1$
> >
> >
> >      // Bug 33196 - encoding ISO-8859-1 is only suitable for Western
> countries
> > @@ -137,15 +138,17 @@ public class SampleResult implements Ser
> >      private static final long NANOTHREAD_SLEEP =
> >              JMeterUtils.getPropDefault("sampleresult.nanoThreadSleep",
> 5000);  // $NON-NLS-1$
> >
> > +    private static final String NULL_FILENAME = "NULL";
> > +
> >      static {
> >          if (START_TIMESTAMP) {
> >              log.info("Note: Sample TimeStamps are START times");
> >          } else {
> >              log.info("Note: Sample TimeStamps are END times");
> >          }
> > -        log.info("sampleresult.default.encoding is set to " +
> DEFAULT_ENCODING);
> > -        log.info("sampleresult.useNanoTime="+USE_NANO_TIME);
> > -        log.info("sampleresult.nanoThreadSleep="+NANOTHREAD_SLEEP);
> > +        log.info("sampleresult.default.encoding is set to {}",
> DEFAULT_ENCODING);
> > +        log.info("sampleresult.useNanoTime={}", USE_NANO_TIME);
> > +        log.info("sampleresult.nanoThreadSleep={}", NANOTHREAD_SLEEP);
> >
> >          if (USE_NANO_TIME && NANOTHREAD_SLEEP > 0) {
> >              // Make sure we start with a reasonable value
> > @@ -455,7 +458,7 @@ public class SampleResult implements Ser
> >      public long currentTimeInMillis() {
> >          if (useNanoTime){
> >              if (nanoTimeOffset == Long.MIN_VALUE){
> > -                throw new RuntimeException("Invalid call;
> nanoTimeOffset has not been set");
> > +                throw new IllegalStateException("Invalid call;
> nanoTimeOffset has not been set");
> >              }
> >              return sampleNsClockInMs() + nanoTimeOffset;
> >          }
> > @@ -488,7 +491,7 @@ public class SampleResult implements Ser
> >       */
> >      public void setStampAndTime(long stamp, long elapsed) {
> >          if (startTime != 0 || endTime != 0){
> > -            throw new RuntimeException("Calling setStampAndTime() after
> start/end times have been set");
> > +            throw new IllegalStateException("Calling setStampAndTime()
> after start/end times have been set");
> >          }
> >          stampAndTime(stamp, elapsed);
> >      }
> > @@ -500,7 +503,7 @@ public class SampleResult implements Ser
> >       * @return <code>true</code> if the result was previously marked
> >       */
> >      public boolean markFile(String filename) {
> > -        return !files.add(filename);
> > +        return !files.add(filename != null ? filename : NULL_FILENAME);
> >      }
> >
> >      public String getResponseCode() {
> > @@ -642,10 +645,10 @@ public class SampleResult implements Ser
> >          }
> >          String tn = getThreadName();
> >          if (tn.length()==0) {
> > -            tn=Thread.currentThread().getName();//TODO do this more
> efficiently
> > +            tn=Thread.currentThread().getName();
> >              this.setThreadName(tn);
> >          }
> > -        subResult.setThreadName(tn); // TODO is this really necessary?
> > +        subResult.setThreadName(tn);
> >
> >          // Extend the time to the end of the added sample
> >          setEndTime(Math.max(getEndTime(), subResult.getEndTime() +
> nanoTimeOffset - subResult.nanoTimeOffset)); // Bug 51855
> > @@ -1100,8 +1103,7 @@ public class SampleResult implements Ser
> >              timeStamp = endTime;
> >          }
> >          if (startTime == 0) {
> > -            log.error("setEndTime must be called after setStartTime",
> new Throwable("Invalid call sequence"));
> > -            // TODO should this throw an error?
> > +            log.error("setEndTime must be called after setStartTime",
> new Throwable(INVALID_CALL_SEQUENCE_MSG));
> >          } else {
> >              elapsedTime = endTime - startTime - idleTime;
> >          }
> > @@ -1129,7 +1131,7 @@ public class SampleResult implements Ser
> >          if (startTime == 0) {
> >              setStartTime(currentTimeInMillis());
> >          } else {
> > -            log.error("sampleStart called twice", new
> Throwable("Invalid call sequence"));
> > +            log.error("sampleStart called twice", new
> Throwable(INVALID_CALL_SEQUENCE_MSG));
> >          }
> >      }
> >
> > @@ -1141,7 +1143,7 @@ public class SampleResult implements Ser
> >          if (endTime == 0) {
> >              setEndTime(currentTimeInMillis());
> >          } else {
> > -            log.error("sampleEnd called twice", new Throwable("Invalid
> call sequence"));
> > +            log.error("sampleEnd called twice", new
> Throwable(INVALID_CALL_SEQUENCE_MSG));
> >          }
> >      }
> >
> > @@ -1151,7 +1153,7 @@ public class SampleResult implements Ser
> >       */
> >      public void samplePause() {
> >          if (pauseTime != 0) {
> > -            log.error("samplePause called twice", new
> Throwable("Invalid call sequence"));
> > +            log.error("samplePause called twice", new
> Throwable(INVALID_CALL_SEQUENCE_MSG));
> >          }
> >          pauseTime = currentTimeInMillis();
> >      }
> > @@ -1162,7 +1164,7 @@ public class SampleResult implements Ser
> >       */
> >      public void sampleResume() {
> >          if (pauseTime == 0) {
> > -            log.error("sampleResume without samplePause", new
> Throwable("Invalid call sequence"));
> > +            log.error("sampleResume without samplePause", new
> Throwable(INVALID_CALL_SEQUENCE_MSG));
> >          }
> >          idleTime += currentTimeInMillis() - pauseTime;
> >          pauseTime = 0;
> >
> > Modified:
> jmeter/trunk/test/src/org/apache/jmeter/samplers/TestSampleResult.java
> > URL:
> http://svn.apache.org/viewvc/jmeter/trunk/test/src/org/apache/jmeter/samplers/TestSampleResult.java?rev=1859277&r1=1859276&r2=1859277&view=diff
> >
> ==============================================================================
> > ---
> jmeter/trunk/test/src/org/apache/jmeter/samplers/TestSampleResult.java
> (original)
> > +++
> jmeter/trunk/test/src/org/apache/jmeter/samplers/TestSampleResult.java Wed
> May 15 08:31:06 2019
> > @@ -371,5 +371,16 @@ public class TestSampleResult implements
> >                  plan.setFunctionalMode(prevValue);
> >              }
> >          }
> > +
> > +        @Test
> > +        public void testBug63433() {
> > +            SampleResult firstResult = new SampleResult();
> > +            assertFalse("Expected false on first call of markFile",
> firstResult.markFile("result.csv"));
> > +            assertTrue("Expected true on second call of markFile",
> firstResult.markFile("result.csv"));
> > +
> > +            SampleResult secondResult = new SampleResult();
> > +            assertFalse("Expected false on first call of markFile with
> null", secondResult.markFile(null));
> > +            assertTrue("Expected true on second call of markFile with
> null", secondResult.markFile(null));
> > +        }
> >  }
> >
> >
> > Modified: jmeter/trunk/xdocs/changes.xml
> > URL:
> http://svn.apache.org/viewvc/jmeter/trunk/xdocs/changes.xml?rev=1859277&r1=1859276&r2=1859277&view=diff
> >
> ==============================================================================
> > --- jmeter/trunk/xdocs/changes.xml [utf-8] (original)
> > +++ jmeter/trunk/xdocs/changes.xml [utf-8] Wed May 15 08:31:06 2019
> > @@ -148,6 +148,7 @@ to view the last major behaviors with th
> >      <li><bug>63319</bug><code>ArrayIndexOutOfBoundsException</code>
in
> Aggregate Graph when selecting 90&nbsp;% or 95&nbsp;% columns</li>
> >      <li><bug>63423</bug>Selection of table rows in Aggregate
Graph gets
> lost too often</li>
> >      <li><bug>63347</bug>View result tree: The search field is
so small
> that even a single character is not visible on Windows 7</li>
> > +    <li><bug>63433</bug>ListenerNotifier: Detected problem in
Listener
> NullPointerException if filename is null. Contributed by Ubik Load Pack
> (support at ubikloadpack.com)</li>
> >  </ul>
> >
> >  <h3>Timers, Assertions, Config, Pre- &amp; Post-Processors</h3>
> >
> >
>


-- 
Cordialement.
Philippe Mouawad.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message