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 < [email protected]> 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 [email protected]: > > 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 % or 95 % 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- & Post-Processors</h3> > > > > > -- Cordialement. Philippe Mouawad.
