Skip to content

Commit

Permalink
Fix NPE in TestRunSession#toString and use AtomicReference
Browse files Browse the repository at this point in the history
currently if one calls TestRunSession#toString (e.g. in a debugger) a
NPE can occur if the session is not yet started and the fStartTime is
null. Also it is possible that fStartTime is concurrently set.

This checks for this case adjust the message accordingly and also
replace the volatile field with an AtomicReference to ensure the start
time is only ever set by the first thread, alongside with that an
unnecessary synchronized is removed from the method
addTestSessionListener as ListenerList is already synchronized.
  • Loading branch information
laeubi committed May 11, 2024
1 parent a2ad0b3 commit b04304b
Showing 1 changed file with 13 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.concurrent.atomic.AtomicReference;

import org.eclipse.unittest.internal.UnitTestPlugin;
import org.eclipse.unittest.internal.launcher.TestViewSupportRegistry;
Expand Down Expand Up @@ -78,7 +79,7 @@ public class TestRunSession extends TestSuiteElement implements ITestRunSession,
*/
private HashMap<String, TestElement> fIdToTest;

volatile Instant fStartTime;
private final AtomicReference<Instant> fStartTime = new AtomicReference<>();
volatile Integer fPredefinedTestCount;

volatile boolean fIsAborted;
Expand All @@ -105,7 +106,7 @@ public TestRunSession(String testRunName, Instant startTime, ILaunchConfiguratio
fIdToTest = new HashMap<>();

fTestRunnerClient = null;
fStartTime = startTime;
fStartTime.set(startTime);

fSessionListeners = new ListenerList<>();
}
Expand Down Expand Up @@ -254,7 +255,7 @@ public int getCurrentIgnoredCount() {
* @return an {@link Instant} object indicating a test run session start time
*/
public Instant getStartTime() {
return fStartTime;
return fStartTime.get();
}

/**
Expand All @@ -273,7 +274,7 @@ public boolean isStopped() {
*
* @param listener an {@link ITestSessionListener} object
*/
public synchronized void addTestSessionListener(ITestSessionListener listener) {
public void addTestSessionListener(ITestSessionListener listener) {
fSessionListeners.add(listener);
}

Expand Down Expand Up @@ -381,7 +382,8 @@ private class TestSessionNotifier {
* @param testCount number of tests in this run
*/
public void testRunStarted(Integer testCount) {
fStartTime = Instant.now();
// only update if not already set!
fStartTime.compareAndSet(null, Instant.now());
fPredefinedTestCount = testCount;

for (ITestSessionListener listener : fSessionListeners) {
Expand Down Expand Up @@ -590,7 +592,12 @@ private void addFailures(Collection<TestElement> failures, TestElement testEleme

@Override
public String toString() {
return fTestRunName + " " + DateFormat.getDateTimeInstance().format(new Date(fStartTime.toEpochMilli())); //$NON-NLS-1$
Instant startTime = getStartTime();
if (startTime == null) {
return fTestRunName + " (not started)"; //$NON-NLS-1$
} else {
return fTestRunName + " " + DateFormat.getDateTimeInstance().format(new Date(startTime.toEpochMilli())); //$NON-NLS-1$
}
}

@Override
Expand Down

0 comments on commit b04304b

Please sign in to comment.