jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Felix Schumacher <felix.schumac...@internetallee.de>
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 10:18:57 GMT
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>
>
>

Mime
View raw message