Skip to content

Commit

Permalink
Replace Hamcrest assertions with AssertJ assertions
Browse files Browse the repository at this point in the history 
Comprehensive assertions can be defined with different libraries, of
which Hamcrest and AssertJ are two of the most popular ones. While
Platform tests currently only use Hamcrest at some places, AssertJ has
several advantages including:
* Easier to apply and learn due to fluent API rather than matcher
providing comprehensive code completion support
* Higher expressiveness by easily chaining multiple assertions on the
same object
* Better comprehensibility by chaining instead of nesting calls and
starting `assertThat` always with the subject under test and not a
custom message in case such a message is desired

This change migrates the existing usages of Hamcrest to AssertJ (except
for one custom Matcher implementation). Doing that it also makes use of
String formatting rather than String concatenation with a potential
slight performance improvement.
  • Loading branch information
@HeikoKlare
HeikoKlare committed Jan 7, 2024
1 parent ef98ac4 commit 58b19ac
Show file tree
Hide file tree
Showing 51 changed files with 892 additions and 1,022 deletions.
50 changes: 24 additions & 26 deletions .../org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/InputStreamMonitorTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*******************************************************************************/
package org.eclipse.debug.tests.console;

import static org.junit.Assert.assertEquals;
import static org.assertj.core.api.Assertions.assertThat;

import java.io.IOException;
import java.io.OutputStream;
Expand All @@ -29,9 +29,6 @@
import org.eclipse.debug.tests.TestsPlugin;
import org.junit.Test;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;

/**
* Tests the {@link InputStreamMonitor}.
*/
Expand All @@ -49,21 +46,21 @@ public void testInputStreamMonitor() throws Exception {
OutputStream outputFromSysin = new PipedOutputStream(sysin);
) {
monitor = new InputStreamMonitor(outputFromSysin);
byte[] content = new byte[100];
for (int i = 0; i < content.length; i++) {
content[i] = (byte) (i % 255);
byte[] originalContent = new byte[100];
for (int i = 0; i < originalContent.length; i++) {
originalContent[i] = (byte) (i % 255);
}
int half = content.length / 2;
monitor.write(content, 0, half);
int half = originalContent.length / 2;
monitor.write(originalContent, 0, half);
monitor.startMonitoring();
monitor.write(content, half, content.length - half);
waitForElementsInStream(sysin, content.length);

byte[] readBack = new byte[content.length];
int read = sysin.read(readBack);
assertEquals("Monitor wrote to few bytes.", read, content.length);
assertThat("Monitor wrote to much bytes.", sysin.available(), is(0));
assertThat("Monitor wrote wrong content.", readBack, is(content));
monitor.write(originalContent, half, originalContent.length - half);
waitForElementsInStream(sysin, originalContent.length);

byte[] contentWrittenByMonitor = new byte[originalContent.length];
sysin.read(contentWrittenByMonitor);
int additionalBytesWritten = sysin.available();
assertThat(additionalBytesWritten).isZero();
assertThat(contentWrittenByMonitor).isEqualTo(originalContent);
} finally {
if (monitor != null) {
monitor.close();
Expand All @@ -80,7 +77,7 @@ private void waitForElementsInStream(PipedInputStream sysin, int numberOfElement
}
return true;
}, CONDITION_TIMEOUT_IN_MILLIS);
assertThat(sysin.available(), is(numberOfElements));
assertThat(sysin.available()).isEqualTo(numberOfElements);
}

/**
Expand All @@ -98,7 +95,8 @@ public void testNullCharset() throws Exception {

byte[] readBack = new byte[1000];
int len = sysin.read(readBack);
assertThat("Monitor wrote wrong content.", text, is(new String(readBack, 0, len)));
String readContent = new String(readBack, 0, len);
assertThat(readContent).isEqualTo(text);
} finally {
if (monitor != null) {
monitor.close();
Expand Down Expand Up @@ -126,34 +124,34 @@ public void testClose() throws Exception {
{
ClosableTestOutputStream testStream = new ClosableTestOutputStream();
InputStreamMonitor monitor = new InputStreamMonitor(testStream);
assertThat("Stream closed to early.", testStream.numClosed, is(0));
assertThat(testStream.numClosed).withFailMessage("stream closed too early").isZero();
monitor.closeInputStream();
TestUtil.waitWhile(() -> testStream.numClosed == 0, CONDITION_TIMEOUT_IN_MILLIS);
assertThat("Stream not closed.", testStream.numClosed, is(1));
assertThat(testStream.numClosed).withFailMessage("stream not closed").isNotZero();
}
{
ClosableTestOutputStream testStream = new ClosableTestOutputStream();
InputStreamMonitor monitor = new InputStreamMonitor(testStream);
monitor.startMonitoring(threadName);
assertThat("Stream closed to early.", testStream.numClosed, is(0));
assertThat(testStream.numClosed).withFailMessage("stream closed too early").isZero();
monitor.close();
TestUtil.waitWhile(() -> testStream.numClosed == 0, CONDITION_TIMEOUT_IN_MILLIS);
assertThat("Stream not closed.", testStream.numClosed, is(1));
assertThat(testStream.numClosed).withFailMessage("stream not closed").isNotZero();
}
{
ClosableTestOutputStream testStream = new ClosableTestOutputStream();
InputStreamMonitor monitor = new InputStreamMonitor(testStream);
monitor.startMonitoring(threadName);
assertThat("Stream closed to early.", testStream.numClosed, is(0));
assertThat(testStream.numClosed).withFailMessage("stream closed too early").isZero();
monitor.closeInputStream();
monitor.close();
monitor.close();
TestUtil.waitWhile(() -> testStream.numClosed == 0, CONDITION_TIMEOUT_IN_MILLIS);
assertThat("Stream not closed or to often.", testStream.numClosed, is(1));
assertThat(testStream.numClosed).as("stream should be closed once").isEqualTo(1);
}

TestUtil.waitWhile(() -> getInputStreamMonitorThreads.get() > 0, CONDITION_TIMEOUT_IN_MILLIS);
assertThat("Leaked monitor threads.", getInputStreamMonitorThreads.get(), is(0L));
assertThat(getInputStreamMonitorThreads.get()).as("leaked monitor threads").isZero();
}

/**
Expand Down
61 changes: 32 additions & 29 deletions ...org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/OutputStreamMonitorTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*******************************************************************************/
package org.eclipse.debug.tests.console;

import static org.assertj.core.api.Assertions.assertThat;

import java.io.ByteArrayOutputStream;
import java.io.IOException;
import java.io.InputStream;
Expand All @@ -36,9 +38,6 @@
import org.junit.After;
import org.junit.Before;
import org.junit.Test;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.is;
import static org.hamcrest.Matchers.empty;

/**
* Tests the {@link OutputStreamMonitor}.
Expand Down Expand Up @@ -78,7 +77,7 @@ public byte[] getRecordedBytes() {
}

public void assertNoExceptions() {
assertThat(exceptions, is(empty()));
assertThat(exceptions).isEmpty();
}
}

Expand Down Expand Up @@ -134,32 +133,36 @@ public void testBufferedOutputStreamMonitor() throws Exception {
monitor.startMonitoring();
binaryListener.waitForBytes(2);
streamListener.waitForBytes(1);
String contents = monitor.getContents();
assertThat("Monitor read wrong content.", contents, is(input.substring(0, 1)));
assertThat("Notified and buffered content differ.", streamListener.getRecordedChars(), is(contents));
assertThat("Failed to access buffered content twice.", monitor.getContents(), is(contents));
byte[] data = monitor.getData();
byte[] expected = new byte[2];
System.arraycopy(byteInput, 0, expected, 0, 2);
assertThat("Monitor read wrong binary content.", data, is(expected));
assertThat("Notified and buffered binary content differ.", binaryListener.getRecordedBytes(), is(data));
assertThat("Failed to access buffered binary content twice.", monitor.getData(), is(data));
String monitorContents = monitor.getContents();
assertThat(monitorContents).isEqualTo(input.substring(0, 1));
assertThat(streamListener.getRecordedChars()).isEqualTo(monitorContents);
String monitorContentsOnSecondAccess = monitor.getContents();
assertThat(monitorContentsOnSecondAccess).isEqualTo(monitorContents);
byte[] binaryMonitorData = monitor.getData();
byte[] expectedBinaryData = new byte[2];
System.arraycopy(byteInput, 0, expectedBinaryData, 0, 2);
assertThat(binaryMonitorData).isEqualTo(expectedBinaryData);
assertThat(binaryListener.getRecordedBytes()).isEqualTo(binaryMonitorData);
byte[] binaryMonitorDataOnSecondAccess = monitor.getData();
assertThat(binaryMonitorDataOnSecondAccess).isEqualTo(binaryMonitorData);

monitor.flushContents();
sysout.write(byteInput, 2, byteInput.length - 2);
sysout.flush();
binaryListener.waitForBytes(byteInput.length);
streamListener.waitForBytes(new String(byteInput).length());
contents = monitor.getContents();
assertThat("Monitor buffered wrong content.", contents, is(input.substring(1)));
assertThat("Failed to access buffered content twice.", monitor.getContents(), is(contents));
assertThat("Wrong content through listener.", streamListener.getRecordedChars(), is(input));
data = monitor.getData();
expected = new byte[byteInput.length - 2];
System.arraycopy(byteInput, 2, expected, 0, expected.length);
assertThat("Monitor read wrong binary content.", data, is(expected));
assertThat("Failed to access buffered binary content twice.", monitor.getData(), is(data));
assertThat("Wrong binary content through listener.", binaryListener.getRecordedBytes(), is(byteInput));
monitorContents = monitor.getContents();
assertThat(monitorContents).isEqualTo(input.substring(1));
monitorContentsOnSecondAccess = monitor.getContents();
assertThat(monitorContentsOnSecondAccess).isEqualTo(monitorContents);
assertThat(streamListener.getRecordedChars()).isEqualTo(input);
binaryMonitorData = monitor.getData();
expectedBinaryData = new byte[byteInput.length - 2];
System.arraycopy(byteInput, 2, expectedBinaryData, 0, expectedBinaryData.length);
assertThat(binaryMonitorData).isEqualTo(expectedBinaryData);
binaryMonitorDataOnSecondAccess = monitor.getData();
assertThat(binaryMonitorDataOnSecondAccess).isEqualTo(binaryMonitorData);
assertThat(binaryListener.getRecordedBytes()).isEqualTo(byteInput);

binaryListener.assertNoExceptions();
}
Expand All @@ -183,20 +186,20 @@ public void testUnbufferedOutputStreamMonitor() throws Exception {
monitor.startMonitoring();
binaryListener.waitForBytes(2);
streamListener.waitForBytes(1);
assertThat("Monitor read wrong content.", streamListener.getRecordedChars(), is(input.substring(0, 1)));
assertThat(streamListener.getRecordedChars()).isEqualTo(input.substring(0, 1));
byte[] expected = new byte[2];
System.arraycopy(byteInput, 0, expected, 0, 2);
assertThat("Monitor read wrong binary content.", binaryListener.getRecordedBytes(), is(expected));
assertThat(binaryListener.getRecordedBytes()).isEqualTo(expected);

monitor.flushContents();
sysout.write(byteInput, 2, byteInput.length - 2);
sysout.flush();
binaryListener.waitForBytes(byteInput.length);
streamListener.waitForBytes(new String(byteInput).length());
assertThat("Wrong content through listener.", streamListener.getRecordedChars(), is(input));
assertThat(streamListener.getRecordedChars()).isEqualTo(input);
expected = new byte[byteInput.length - 2];
System.arraycopy(byteInput, 2, expected, 0, expected.length);
assertThat("Wrong binary content through listener.", binaryListener.getRecordedBytes(), is(byteInput));
assertThat(binaryListener.getRecordedBytes()).isEqualTo(byteInput);

binaryListener.assertNoExceptions();
}
Expand All @@ -223,7 +226,7 @@ public void testNullCharset() throws Exception {
sysout.flush();

streamListener.waitForBytes(input.length());
assertThat("Monitor read wrong content.", streamListener.getRecordedChars(), is(input));
assertThat(streamListener.getRecordedChars()).isEqualTo(input);
}

/**
Expand Down
5 changes: 2 additions & 3 deletions ...g.eclipse.debug.tests/src/org/eclipse/debug/tests/console/ProcessConsoleManagerTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,7 @@
package org.eclipse.debug.tests.console;

import static java.util.stream.Collectors.joining;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.hasSize;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
Expand Down Expand Up @@ -130,7 +129,7 @@ public void testBug546710_ConsoleCreationRaceCondition() throws Exception {
String launchesString = Stream.of(launches).map(launch -> Stream.of(launch.getProcesses()).map(IProcess::getLabel).collect(joining(",", "[", "]"))).collect(joining());
String consolesString = openConsoles.stream().map(IConsole::getName).collect(joining());
String failureMessage = String.format("ProcessConsoleManager and LaunchManager got out of sync.\nLaunches: %s\nConsoles: %s", launchesString, consolesString);
assertThat(failureMessage, openConsoles, hasSize(launches.length));
assertThat(openConsoles).as(failureMessage).hasSameSizeAs(launches);

final ConsoleRemoveAllTerminatedAction removeAction = new ConsoleRemoveAllTerminatedAction();
assertTrue("Remove terminated action should be enabled.", removeAction.isEnabled() || launchManager.getLaunches().length == 0);
Expand Down
8 changes: 3 additions & 5 deletions debug/org.eclipse.debug.tests/src/org/eclipse/debug/tests/console/ProcessConsoleTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@
*******************************************************************************/
package org.eclipse.debug.tests.console;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.is;
import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertTrue;
Expand Down Expand Up @@ -110,7 +108,7 @@ public void tearDown() throws Exception {

super.tearDown();

assertThat("Test triggered errors.", loggedErrors, empty());
assertThat(loggedErrors).as("logged errors").isEmpty();
}

/**
Expand Down Expand Up @@ -527,7 +525,7 @@ public void testBinaryOutputToFile() throws Exception {
}

byte[] receivedOutput = Files.readAllBytes(outFile.toPath());
assertThat("unexpected output", receivedOutput, is(output));
assertThat(receivedOutput).as("received output").isEqualTo(output);
}

/**
Expand Down
25 changes: 13 additions & 12 deletions ...pse.debug.tests/src/org/eclipse/debug/tests/ui/LaunchConfigurationTabGroupViewerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,8 @@

package org.eclipse.debug.tests.ui;

import static java.util.function.Predicate.not;
import static org.assertj.core.api.Assertions.assertThat;
import static org.hamcrest.CoreMatchers.not;
import static org.hamcrest.MatcherAssert.assertThat;

import java.util.Arrays;
import java.util.concurrent.atomic.AtomicReference;
Expand All @@ -32,6 +31,7 @@
import org.eclipse.debug.ui.ILaunchConfigurationTabGroup;
import org.eclipse.swt.widgets.Display;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;

public class LaunchConfigurationTabGroupViewerTest {
Expand All @@ -56,9 +56,9 @@ public void createDialog() throws CoreException {
ILaunchConfigurationTab[] tabs = tabGroup.getTabs();

assertThat(tabs).hasSizeGreaterThanOrEqualTo(2);
assertThat(tabs).allMatch(SpyTab.class::isInstance, "Use only SpyTabs in the group");
long typesOfTabs = Arrays.stream(tabs).map(Object::getClass).distinct().count();
assertThat("There are tabs of the exact same type in the group", tabs.length == typesOfTabs);
assertThat(tabs).allSatisfy(tab -> assertThat(tab).isInstanceOf(SpyTab.class));
int typesOfTabs = Math.toIntExact(Arrays.stream(tabs).map(Object::getClass).distinct().count());
assertThat(tabs).withFailMessage("there are tabs of the exact same type in the group").hasSize(typesOfTabs);
}

@Test
Expand All @@ -71,7 +71,7 @@ public void testAllTabsAreInitializedByDefault() {
final ILaunchConfigurationTab[] tabs = runOnDialog(createAndSelect1LaunchConfig);

for (int i = 0; i < tabs.length; i++) {
assertThat("Tab " + i + " was not initialized", ((SpyTab) tabs[i]).isInitialized());
assertThat(((SpyTab) tabs[i])).withFailMessage("tab %s was not initialized", i).matches(SpyTab::isInitialized);
}
}

Expand All @@ -83,7 +83,7 @@ public void testFirstTabIsActivatedByDefault() {
};

final ILaunchConfigurationTab[] tabs = runOnDialog(createAndSelect1LaunchConfig);
assertThat("The 1st tab was not activated", ((SpyTab) tabs[0]).isActivated());
assertThat(((SpyTab) tabs[0])).matches(SpyTab::isActivated, "is activated");
}

@Test
Expand All @@ -104,11 +104,12 @@ public void testOtherTabInOtherConfigIsActivated() {

final ILaunchConfigurationTab[] tabs = runOnDialog(setActiveTab);

assertThat("The 1st tab of the other launch configuration shouldn't have been activated", not(((SpyTab) tabs[0]).isActivated()));
assertThat("The tab was not activated", ((SpyTab) tabs[secondTabIndex]).isActivated());
assertThat((SpyTab) tabs[0]).withFailMessage("the 1st tab of the other launch configuration shouldn't have been activated").matches(not(SpyTab::isActivated));
assertThat((SpyTab) tabs[secondTabIndex]).matches(SpyTab::isActivated, "is activated");
}

@Test
@Ignore("https://github.com/eclipse-platform/eclipse.platform/issues/1075")
public void testOnlyDefaultTabInOtherConfigIsActivated() {
int overflowTabIndex = Integer.MAX_VALUE;

Expand All @@ -126,11 +127,11 @@ public void testOnlyDefaultTabInOtherConfigIsActivated() {

final ILaunchConfigurationTab[] tabs = runOnDialog(setActiveTab);

assertThat("The 1st tab of the other launch configuration should have been activated", ((SpyTab) tabs[0]).isActivated());
assertThat(((SpyTab) tabs[0])).withFailMessage("the 1st tab of the other launch configuration should have been activated").matches(SpyTab::isActivated);

// All other tabs should not have been initialized
for (int i = 1; i < tabs.length; i++) {
assertThat("Tab " + i + " should not have been initialized", not(((SpyTab) tabs[i]).isInitialized()));
assertThat((SpyTab) tabs[i]).withFailMessage("tab %s should not have been initialized", i).matches(not(SpyTab::isInitialized));
}
}

Expand All @@ -148,7 +149,7 @@ public void testOtherTabIsActivated() {

final ILaunchConfigurationTab[] tabs = runOnDialog(setActiveTab);

assertThat("The tab was not activated", ((SpyTab) tabs[secondTabIndex]).isActivated());
assertThat((SpyTab) tabs[secondTabIndex]).matches(SpyTab::isActivated, "is activated");
}

private ILaunchConfigurationWorkingCopy createLaunchConfigurationInstance() throws CoreException {
Expand Down
3 changes: 2 additions & 1 deletion resources/tests/org.eclipse.core.tests.resources/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ Require-Bundle: org.eclipse.core.resources,
org.eclipse.core.filesystem,
org.eclipse.core.runtime,
org.eclipse.pde.junit.runtime;bundle-version="3.5.0"
Import-Package: org.mockito
Import-Package: org.mockito,
org.assertj.core.api
Bundle-ActivationPolicy: lazy
Bundle-RequiredExecutionEnvironment: JavaSE-17
Eclipse-BundleShape: dir
Expand Down
Loading

0 comments on commit 58b19ac

Please sign in to comment.