Skip to content

Commit

Permalink
Improve array length assertions in UA tests
Browse files Browse the repository at this point in the history 
In order to reduce/avoid usage of ambiguous `assertEquals` (hard to
distinguish expected/actual) and prepare for a migration to JUnit 5 with
swapped expected/actual parameters in the `assertEquals` signature, this
change replaces certain `assertEquals` calls with AssertJ assertions:
* Replace `assertEquals(MESSAGE, EXPECTED, ACTUAL.length)` with
`assertThat(ACTUAL).describedAs(MESSAGE).hasSize(EXPECTED))` and remove
obsolete message where possible
* Replace `assertEquals(EXPECTED, ACTUAL.length)` with
`assertThat(ACTUAL).hasSize(EXPECTED))`
* Replace `assertEquals(EXPECTED, ACTUAL.length)` with
`assertThat(ACTUAL.length, is(EXPECTED))` if `ACTUAL` is a primitive
type array
* Replace `hasSize(0)` with `isEmpty()` to improve failure messages
* Combine array size comparison with array contents comparison where
possible
  • Loading branch information
@HeikoKlare
HeikoKlare committed Jan 11, 2024
1 parent 6a574c3 commit 187c536
Show file tree
Hide file tree
Showing 47 changed files with 402 additions and 450 deletions.
1 change: 1 addition & 0 deletions ua/org.eclipse.ua.tests/META-INF/MANIFEST.MF
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Bundle-ActivationPolicy: lazy
Bundle-Vendor: Eclipse.org
Import-Package: javax.servlet;version="3.1.0",
javax.servlet.http;version="3.1.0",
org.assertj.core.api,
org.junit.jupiter.api,
org.junit.platform.suite.api
Bundle-RequiredExecutionEnvironment: JavaSE-17
Expand Down
21 changes: 9 additions & 12 deletions ...se.ua.tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/composite/TestCompositeParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package org.eclipse.ua.tests.cheatsheet.composite;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
Expand Down Expand Up @@ -131,12 +132,10 @@ public void testDependency() {
assertTrue(parser.getStatus().isOK());
AbstractTask task1 = model.getDependencies().getTask("task1");
AbstractTask task2 = model.getDependencies().getTask("task2");
assertTrue(task1.getRequiredTasks().length == 0);
assertTrue(task1.getSuccessorTasks().length == 1);
assertEquals(task2, task1.getSuccessorTasks()[0]);
assertTrue(task2.getSuccessorTasks().length == 0);
assertTrue(task2.getRequiredTasks().length == 1);
assertEquals(task1, task2.getRequiredTasks()[0]);
assertThat(task1.getRequiredTasks()).isEmpty();
assertThat(task1.getSuccessorTasks()).containsExactly(task2);
assertThat(task2.getSuccessorTasks()).isEmpty();
assertThat(task2.getRequiredTasks()).containsExactly(task1);
assertTrue(task1.isSkippable());
assertFalse(task2.isSkippable());
}
Expand All @@ -148,12 +147,10 @@ public void testBackwardDependency() {
assertTrue(parser.getStatus().isOK());
AbstractTask task1 = model.getDependencies().getTask("task1");
AbstractTask task2 = model.getDependencies().getTask("task2");
assertTrue(task1.getRequiredTasks().length == 0);
assertTrue(task1.getSuccessorTasks().length == 1);
assertEquals(task2, task1.getSuccessorTasks()[0]);
assertTrue(task2.getSuccessorTasks().length == 0);
assertTrue(task2.getRequiredTasks().length == 1);
assertEquals(task1, task2.getRequiredTasks()[0]);
assertThat(task1.getRequiredTasks()).isEmpty();
assertThat(task1.getSuccessorTasks()).containsExactly(task2);
assertThat(task2.getSuccessorTasks()).isEmpty();
assertThat(task2.getRequiredTasks()).containsExactly(task1);
}

@Test
Expand Down
31 changes: 15 additions & 16 deletions ua/org.eclipse.ua.tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/composite/TestState.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package org.eclipse.ua.tests.cheatsheet.composite;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
Expand Down Expand Up @@ -152,14 +153,12 @@ public void testRestartStates() {
task1.setState(ICompositeCheatSheetTask.IN_PROGRESS);
task2.setState(ICompositeCheatSheetTask.COMPLETED);
task4.setState(ICompositeCheatSheetTask.SKIPPED);
assertEquals(1, TaskStateUtilities.getRestartTasks(task1).length);
assertEquals(task1, TaskStateUtilities.getRestartTasks(task1)[0]);
assertEquals(1, TaskStateUtilities.getRestartTasks(task2).length);
assertEquals(0, TaskStateUtilities.getRestartTasks(task3).length);
assertEquals(1, TaskStateUtilities.getRestartTasks(task4).length);
assertEquals(1, TaskStateUtilities.getRestartTasks(group).length);
assertEquals(task4, TaskStateUtilities.getRestartTasks(group)[0]);
assertEquals(3, TaskStateUtilities.getRestartTasks(root).length);
assertThat(TaskStateUtilities.getRestartTasks(task1)).containsExactly(task1);
assertThat(TaskStateUtilities.getRestartTasks(task2)).hasSize(1);
assertThat(TaskStateUtilities.getRestartTasks(task3)).isEmpty();
assertThat(TaskStateUtilities.getRestartTasks(task4)).hasSize(1);
assertThat(TaskStateUtilities.getRestartTasks(group)).containsExactly(task4);
assertThat(TaskStateUtilities.getRestartTasks(root)).hasSize(3);
}

/**
Expand Down Expand Up @@ -190,21 +189,21 @@ public void testResetDependents() {
task2.complete();
task3.complete();
task5.setStarted();
assertEquals(4, TaskStateUtilities.getRestartTasks(root).length);
assertEquals(4, TaskStateUtilities.getRestartTasks(task1).length);
assertEquals(3, TaskStateUtilities.getRestartTasks(task2).length);
assertEquals(2, TaskStateUtilities.getRestartTasks(task3).length);
assertEquals(1, TaskStateUtilities.getRestartTasks(subGroup).length);
assertThat(TaskStateUtilities.getRestartTasks(root)).hasSize(4);
assertThat(TaskStateUtilities.getRestartTasks(task1)).hasSize(4);
assertThat(TaskStateUtilities.getRestartTasks(task2)).hasSize(3);
assertThat(TaskStateUtilities.getRestartTasks(task3)).hasSize(2);
assertThat(TaskStateUtilities.getRestartTasks(subGroup)).hasSize(1);
// Reset task5 and start task 1 and 3 and skip task 2.
// Resetting task1 will not require task2 or task3 to be reset
task5.setState(ICompositeCheatSheetTask.NOT_STARTED);
task3.setState(ICompositeCheatSheetTask.NOT_STARTED);
task2.setState(ICompositeCheatSheetTask.NOT_STARTED);
task2.setState(ICompositeCheatSheetTask.SKIPPED);
task3.setStarted();
assertEquals(3, TaskStateUtilities.getRestartTasks(root).length);
assertEquals(1, TaskStateUtilities.getRestartTasks(task1).length);
assertEquals(2, TaskStateUtilities.getRestartTasks(task2).length);
assertThat(TaskStateUtilities.getRestartTasks(root)).hasSize(3);
assertThat(TaskStateUtilities.getRestartTasks(task1)).hasSize(1);
assertThat(TaskStateUtilities.getRestartTasks(task2)).hasSize(2);
}

}
11 changes: 4 additions & 7 deletions ...eclipse.ua.tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/composite/TestSuccessors.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package org.eclipse.ua.tests.cheatsheet.composite;

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

import org.eclipse.ui.internal.cheatsheets.composite.model.CompositeCheatSheetModel;
Expand Down Expand Up @@ -70,14 +71,13 @@ private void setupModel(boolean subGroupIsChoice) {
private void assertSingleSuccessor(ICompositeCheatSheetTask task, ICompositeCheatSheetTask expectedSuccessor) {
SuccesorTaskFinder finder = new SuccesorTaskFinder(task);
ICompositeCheatSheetTask[] successors = finder.getRecommendedSuccessors();
assertEquals(1, successors.length);
assertEquals(expectedSuccessor, successors[0]);
assertThat(successors).containsExactly(expectedSuccessor);
}

private void assertNoSuccessors(ICompositeCheatSheetTask task) {
SuccesorTaskFinder finder = new SuccesorTaskFinder(task);
ICompositeCheatSheetTask[] successors = finder.getRecommendedSuccessors();
assertEquals(0, successors.length);
assertThat(successors).isEmpty();
}

@Test
Expand Down Expand Up @@ -155,11 +155,8 @@ public void testUnstartedChoice() {
setupModel(true);
SuccesorTaskFinder finder = new SuccesorTaskFinder(subGroup);
ICompositeCheatSheetTask[] successors = finder.getRecommendedSuccessors();
assertEquals(3, successors.length);
// The successors should be in task order
assertEquals(task1, successors[0]);
assertEquals(task2, successors[1]);
assertEquals(task3, successors[2]);
assertThat(successors).containsExactly(task1, task2, task3);
}

@Test
Expand Down
5 changes: 2 additions & 3 deletions ...se.ua.tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/execution/TestActionExecution.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package org.eclipse.ua.tests.cheatsheet.execution;

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

Expand Down Expand Up @@ -103,9 +104,7 @@ public void testActionWithParameters() {
assertTrue(status.isOK());
assertEquals(1, ActionEnvironment.getTimesCompleted());
String[] actuals = ActionEnvironment.getParams();
assertEquals(2, actuals.length);
assertEquals(value0, actuals[0]);
assertEquals(value1, actuals[1]);
assertThat(actuals).containsExactly(value0, value1);
}

private String getPluginId() {
Expand Down
21 changes: 11 additions & 10 deletions ...e.ua.tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/other/TestCheatSheetCollection.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package org.eclipse.ua.tests.cheatsheet.other;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNull;
Expand Down Expand Up @@ -63,8 +64,8 @@ public void setUp() throws Exception {

@Test
public void testRoot() {
assertEquals(2, root.getChildren().length);
assertEquals(2, root.getCheatSheets().length);
assertThat(root.getChildren()).hasSize(2);
assertThat(root.getCheatSheets()).hasSize(2);
assertFalse(root.isEmpty());
assertEquals("rootName", root.getLabel(null));
assertEquals("rootId", root.getId());
Expand All @@ -76,11 +77,11 @@ public void testTopLevelChildCategories() {
Object[] children = root.getChildren();
assertEquals(c1, children[0]);
assertEquals(c2, children[1]);
assertEquals(2, c1.getChildren().length);
assertEquals(0, c1.getCheatSheets().length);
assertThat(c1.getChildren()).hasSize(2);
assertThat(c1.getCheatSheets()).isEmpty();
assertFalse(c1.isEmpty());
assertEquals(0, c2.getChildren().length);
assertEquals(1, c2.getCheatSheets().length);
assertThat(c2.getChildren()).isEmpty();
assertThat(c2.getCheatSheets()).hasSize(1);
assertFalse(c2.isEmpty());
}

Expand All @@ -96,11 +97,11 @@ public void testSecondLevelChildCategories() {
Object[] children = c1.getChildren();
assertEquals(c11, children[0]);
assertEquals(c12, children[1]);
assertEquals(0, c11.getChildren().length);
assertEquals(0, c11.getCheatSheets().length);
assertThat(c11.getChildren()).isEmpty();
assertThat(c11.getCheatSheets()).isEmpty();
assertTrue(c11.isEmpty());
assertEquals(0, c12.getChildren().length);
assertEquals(1, c12.getCheatSheets().length);
assertThat(c12.getChildren()).isEmpty();
assertThat(c12.getCheatSheets()).hasSize(1);
assertFalse(c12.isEmpty());
}

Expand Down
4 changes: 3 additions & 1 deletion ....tests/cheatsheet/org/eclipse/ua/tests/cheatsheet/util/CheatSheetModelSerializerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*******************************************************************************/
package org.eclipse.ua.tests.cheatsheet.util;

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

import java.io.FileOutputStream;
import java.io.IOException;
import java.io.PrintWriter;
Expand Down Expand Up @@ -48,7 +50,7 @@ public class CheatSheetModelSerializerTest {
public void testRunSerializer() throws IOException {
URL[] urls = ResourceFinder.findFiles(FrameworkUtil.getBundle(getClass()), "data/cheatsheet/valid", ".xml",
true);
Assert.assertTrue("Unable to find sample cheat sheets to test parser", urls.length > 0);
assertThat(urls).as("check sample cheat sheets to test parser").hasSizeGreaterThan(0);
for (URL url : urls) {
CheatSheetParser parser = new CheatSheetParser();
CheatSheet sheet = (CheatSheet) parser.parse(url, FrameworkUtil.getBundle(getClass()).getSymbolicName(),
Expand Down
15 changes: 8 additions & 7 deletions ua/org.eclipse.ua.tests/help/org/eclipse/ua/tests/help/criteria/CriteriaDefinitionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@

package org.eclipse.ua.tests.help.criteria;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
Expand Down Expand Up @@ -43,7 +44,7 @@ public void testCriterionValueDefinition() {
UserCriterionValueDefinition valueDefinition = new UserCriterionValueDefinition(VALUEID1, VALUENAME1);
assertEquals(VALUEID1, valueDefinition.getId());
assertEquals(VALUENAME1, valueDefinition.getName());
assertEquals(0, valueDefinition.getChildren().length);
assertThat(valueDefinition.getChildren()).isEmpty();
}

@Test
Expand All @@ -52,7 +53,7 @@ public void testCopyCriterionValueDefinition() {
ICriterionValueDefinition copy = new CriterionValueDefinition(valueDefinition);
assertEquals(VALUEID1, copy.getId());
assertEquals(VALUENAME1, copy.getName());
assertEquals(0, copy.getChildren().length);
assertThat(copy.getChildren()).isEmpty();
}

@Test
Expand All @@ -64,7 +65,7 @@ public void testFactoryCreateValueDefinition() {
ICriterionValueDefinition copy = (ICriterionValueDefinition) element;
assertEquals(VALUEID1, copy.getId());
assertEquals(VALUENAME1, copy.getName());
assertEquals(0, copy.getChildren().length);
assertThat(copy.getChildren()).isEmpty();
}

// Criterion - no children
Expand All @@ -73,7 +74,7 @@ public void testCriterionDefinition() {
UserCriterionDefinition definition = new UserCriterionDefinition(CRITERIONID1, CRITERIONNAME1);
assertEquals(CRITERIONID1, definition.getId());
assertEquals(CRITERIONNAME1, definition.getName());
assertEquals(0, definition.getChildren().length);
assertThat(definition.getChildren()).isEmpty();
}

@Test
Expand All @@ -82,7 +83,7 @@ public void testCopyCriterionDefinition() {
ICriterionDefinition copy = new CriterionDefinition(definition);
assertEquals(CRITERIONID1, copy.getId());
assertEquals(CRITERIONNAME1, copy.getName());
assertEquals(0, copy.getChildren().length);
assertThat(copy.getChildren()).isEmpty();
}

@Test
Expand All @@ -94,7 +95,7 @@ public void testFactoryCreateDefinition() {
ICriterionDefinition copy = (ICriterionDefinition) element;
assertEquals(CRITERIONID1, copy.getId());
assertEquals(CRITERIONNAME1, copy.getName());
assertEquals(0, copy.getChildren().length);
assertThat(copy.getChildren()).isEmpty();
}

@Test
Expand Down Expand Up @@ -139,7 +140,7 @@ private void checkDefinitionWithValues(ICriterionDefinition copy) {
assertEquals(CRITERIONID1, copy.getId());
assertEquals(CRITERIONNAME1, copy.getName());
ICriterionValueDefinition[] values = copy.getCriterionValueDefinitions();
assertEquals(2, values.length);
assertThat(values).hasSize(2);
assertEquals(VALUEID1, values[0].getId());
assertEquals(VALUENAME1, values[0].getName());
assertEquals(VALUEID2, values[1].getId());
Expand Down
19 changes: 10 additions & 9 deletions ua/org.eclipse.ua.tests/help/org/eclipse/ua/tests/help/criteria/ParseTocWithCriteria.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*******************************************************************************/
package org.eclipse.ua.tests.help.criteria;

import static org.assertj.core.api.Assertions.assertThat;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
Expand Down Expand Up @@ -101,7 +102,7 @@ public void testCopyTocWithCriteria() throws Exception {
public void testTopicWithCriteria() throws Exception {
IToc toc = parseToc("data/help/criteria/c1.xml");
ITopic[] topics = toc.getTopics();
assertEquals(topics.length, 2);
assertThat(topics).hasSize(2);
// First topic
Map<String, Set<String>> criteria = new HashMap<>();
assertTrue(topics[0] instanceof ITopic2);
Expand Down Expand Up @@ -129,7 +130,7 @@ public void testTopicWithCriteria() throws Exception {
public void testCriteriaScoping1() throws Exception {
IToc toc = parseToc("data/help/criteria/c1.xml");
ITopic[] topics = toc.getTopics();
assertEquals(topics.length, 2);
assertThat(topics).hasSize(2);
CriterionResource[] resource = new CriterionResource[1];
resource[0] = new CriterionResource("version");
resource[0].addCriterionValue("1.0");
Expand All @@ -143,7 +144,7 @@ public void testCriteriaScoping1() throws Exception {
public void testCriteriaScoping2() throws Exception {
IToc toc = parseToc("data/help/criteria/c1.xml");
ITopic[] topics = toc.getTopics();
assertEquals(topics.length, 2);
assertThat(topics).hasSize(2);
CriterionResource[] resource = new CriterionResource[1];
resource[0] = new CriterionResource("platform");
resource[0].addCriterionValue("linux");
Expand All @@ -157,7 +158,7 @@ public void testCriteriaScoping2() throws Exception {
public void testMultipleCriteriaScoping() throws Exception {
IToc toc = parseToc("data/help/criteria/c1.xml");
ITopic[] topics = toc.getTopics();
assertEquals(topics.length, 2);
assertThat(topics).hasSize(2);
CriterionResource[] resource = new CriterionResource[2];
resource[0] = new CriterionResource("platform");
resource[0].addCriterionValue("linux");
Expand All @@ -178,7 +179,7 @@ public void testMultipleCriteriaOnlyOneSatisfied() throws Exception {
resource[0].addCriterionValue("linux");
resource[1] = new CriterionResource("version");
resource[1].addCriterionValue("2.0");
assertEquals(topics.length, 2);
assertThat(topics).hasSize(2);
CriteriaHelpScope scope = new CriteriaHelpScope(resource);
assertTrue(scope.inScope(toc));
assertFalse(scope.inScope(topics[0]));
Expand All @@ -194,7 +195,7 @@ public void testUserTocWithCriteria() throws Exception {
toc.addCriterion(criterion2);

ICriteria[] criteria = toc.getCriteria();
assertEquals(2, criteria.length);
assertThat(criteria).hasSize(2);
assertEquals("version", criteria[0].getName());
assertEquals("1.0", criteria[0].getValue());
assertEquals("version", criteria[1].getName());
Expand All @@ -212,7 +213,7 @@ public void testCopyUserTocWithCriteria() throws Exception {
Toc copy = new Toc(toc);

ICriteria[] criteria = copy.getCriteria();
assertEquals(2, criteria.length);
assertThat(criteria).hasSize(2);
assertEquals("version", criteria[0].getName());
assertEquals("1.0", criteria[0].getValue());
assertEquals("version", criteria[1].getName());
Expand All @@ -230,7 +231,7 @@ public void testUserTopicWithCriteria() throws Exception {
Topic copy = new Topic(topic);

ICriteria[] criteria = copy.getCriteria();
assertEquals(2, criteria.length);
assertThat(criteria).hasSize(2);
assertEquals("version", criteria[0].getName());
assertEquals("1.0", criteria[0].getValue());
assertEquals("version", criteria[1].getName());
Expand All @@ -245,7 +246,7 @@ public void testCopyUserTopicWithCriteria() throws Exception {
topic.addCriterion(criterion1);
topic.addCriterion(criterion2);
ICriteria[] criteria = topic.getCriteria();
assertEquals(2, criteria.length);
assertThat(criteria).hasSize(2);
assertEquals("version", criteria[0].getName());
assertEquals("1.0", criteria[0].getValue());
assertEquals("version", criteria[1].getName());
Expand Down
Loading

0 comments on commit 187c536

Please sign in to comment.