Skip to content

Commit

Permalink
Improve assertions in resource regression tests #903
Browse files Browse the repository at this point in the history 
This change removes non-useful messages from assertions in
org.eclipse.core.tests.resources.regression. In cases where multiple
assertions without proper messages are placed in subsequent lines of
code, assertions are improved to provide better messages and help
identifying the failing line of code. This particularly affects
assertTrue/assertFalse statements that are replaced with
assertThat().matches().

This also prepares for a migration of assertions to JUnit 5, which would
otherwise require swapping of parameters, as the message parameter has
been moved to the end of the parameter list.

Contributes to
#903
  • Loading branch information
@HeikoKlare
HeikoKlare committed Aug 18, 2024
1 parent 45aa1a4 commit a022584
Show file tree
Hide file tree
Showing 30 changed files with 395 additions and 356 deletions.
8 changes: 5 additions & 3 deletions ...ipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/Bug_006708.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,13 @@
*******************************************************************************/
package org.eclipse.core.tests.resources.regression;

import static org.junit.Assert.assertFalse;
import static java.util.function.Predicate.not;
import static org.assertj.core.api.Assertions.assertThat;

import java.io.ByteArrayInputStream;
import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IProject;
import org.eclipse.core.resources.IResource;
import org.eclipse.core.resources.IWorkspaceRoot;
import org.eclipse.core.resources.ResourcesPlugin;
import org.eclipse.core.runtime.CoreException;
Expand All @@ -34,14 +36,14 @@ public class Bug_006708 {
public void testBug() throws CoreException {
IWorkspaceRoot root = ResourcesPlugin.getWorkspace().getRoot();
IProject sourceProj = root.getProject("bug_6708");
assertFalse("Project bug_6708 exists already", sourceProj.exists());
assertThat(sourceProj).matches(not(IResource::exists), "does not exists already");
sourceProj.create(null);
sourceProj.open(null);
IFile source = sourceProj.getFile("Source.txt");
source.create(new ByteArrayInputStream("abcdef".getBytes()), false, null);

IProject destProj = root.getProject("bug_6708_2");
assertFalse("Project bug_6708_2 exists already", destProj.exists());
assertThat(destProj).matches(not(IResource::exists), "does not exists already");
destProj.create(null);
destProj.open(null);
IFile dest = destProj.getFile("Dest.txt");
Expand Down
27 changes: 14 additions & 13 deletions ...ipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/Bug_025457.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@
*******************************************************************************/
package org.eclipse.core.tests.resources.regression;

import static java.util.function.Predicate.not;
import static org.assertj.core.api.Assertions.assertThat;
import static org.eclipse.core.resources.ResourcesPlugin.getWorkspace;
import static org.eclipse.core.tests.resources.ResourceTestUtil.createInWorkspace;
import static org.eclipse.core.tests.resources.ResourceTestUtil.createRandomString;
import static org.eclipse.core.tests.resources.ResourceTestUtil.createTestMonitor;
import static org.eclipse.core.tests.resources.ResourceTestUtil.isReadOnlySupported;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeTrue;
import static org.junit.jupiter.api.Assertions.assertEquals;

Expand Down Expand Up @@ -66,11 +67,11 @@ public void testFile() throws Exception {
() -> sourceFile.move(destFile.getFullPath(), IResource.NONE, createTestMonitor()));
}
//ensure source still exists and has same content
assertTrue("2.0", source.exists());
assertTrue("2.1", sourceFile.exists());
assertThat(source).matches(IResource::exists, "exists");
assertThat(sourceFile).matches(IResource::exists, "exists");
assertEquals(content, sourceFile.readString());
//ensure destination file does not exist
assertTrue("2.3", !destFile.exists());
assertThat(destFile).matches(not(IResource::exists), "not exists");
}

@Test
Expand All @@ -93,13 +94,13 @@ public void testFolder() throws IOException, CoreException {
assertThrows(CoreException.class,
() -> sourceFolder.move(destFolder.getFullPath(), IResource.NONE, createTestMonitor()));
//ensure source still exists
assertTrue("2.0", source.exists());
assertTrue("2.1", sourceFolder.exists());
assertTrue("2.2", sourceFile.exists());
assertThat(source).matches(IResource::exists, "exists");
assertThat(sourceFolder).matches(IResource::exists, "exists");
assertThat(sourceFile).matches(IResource::exists, "exists");

//ensure destination does not exist
assertTrue("2.3", !destFolder.exists());
assertTrue("2.4", !destFile.exists());
assertThat(destFolder).matches(not(IResource::exists), "not exists");
assertThat(destFile).matches(not(IResource::exists), "not exists");
}
}

Expand All @@ -121,12 +122,12 @@ public void testProject() throws IOException, CoreException {
() -> source.move(destination.getFullPath(), IResource.NONE, createTestMonitor()));

//ensure source does not exist
assertTrue("2.0", !source.exists());
assertTrue("2.1", !sourceFile.exists());
assertThat(source).matches(not(IResource::exists), "not exists");
assertThat(sourceFile).matches(not(IResource::exists), "not exists");

//ensure destination does not exist
assertTrue("2.2", destination.exists());
assertTrue("2.3", destFile.exists());
assertThat(destination).matches(IResource::exists, "exists");
assertThat(destFile).matches(IResource::exists, "exists");
}
}
}
130 changes: 69 additions & 61 deletions ...ipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/Bug_026294.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*******************************************************************************/
package org.eclipse.core.tests.resources.regression;

import static java.util.function.Predicate.not;
import static org.assertj.core.api.Assertions.assertThat;
import static org.eclipse.core.resources.ResourcesPlugin.getWorkspace;
import static org.eclipse.core.tests.resources.ResourceTestUtil.assertDoesNotExistInFileSystem;
import static org.eclipse.core.tests.resources.ResourceTestUtil.assertDoesNotExistInWorkspace;
Expand All @@ -24,10 +26,10 @@
import static org.eclipse.core.tests.resources.ResourceTestUtil.isReadOnlySupported;
import static org.eclipse.core.tests.resources.ResourceTestUtil.setReadOnly;
import static org.junit.Assert.assertThrows;
import static org.junit.Assert.assertTrue;
import static org.junit.Assume.assumeTrue;

import java.io.InputStream;
import java.util.function.Predicate;
import org.eclipse.core.resources.IFile;
import org.eclipse.core.resources.IFolder;
import org.eclipse.core.resources.IProject;
Expand All @@ -47,6 +49,12 @@
*/
public class Bug_026294 {

private static final Predicate<IResource> isSynchronizedDepthInfinite = resource -> resource
.isSynchronized(IResource.DEPTH_INFINITE);

private static final Predicate<IResource> isSynchronizedDepthZero = resource -> resource
.isSynchronized(IResource.DEPTH_ZERO);

@Rule
public WorkspaceTestRule workspaceRule = new WorkspaceTestRule();

Expand Down Expand Up @@ -78,8 +86,8 @@ public void testDeleteOpenProjectWindows() throws Exception {

// opens a file so it cannot be removed on Windows
try (InputStream input = file1.getContents()) {
assertTrue("1.2", projectFile.exists());
assertTrue("1.3", projectFile.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(projectFile).matches(IResource::exists, "exists");
assertThat(projectFile).matches(isSynchronizedDepthInfinite, "is synchronized");

assertThrows(CoreException.class, () -> project.delete(IResource.FORCE, createTestMonitor()));

Expand All @@ -90,35 +98,35 @@ public void testDeleteOpenProjectWindows() throws Exception {

assertExistsInWorkspace(file1);
assertExistsInFileSystem(file1);
assertTrue("2.2.3", file1.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(file1).matches(isSynchronizedDepthInfinite, "is synchronized");

assertDoesNotExistInWorkspace(file2);
assertDoesNotExistInFileSystem(file2);
assertTrue("2.3.3", file2.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(file2).matches(isSynchronizedDepthInfinite, "is synchronized");

assertDoesNotExistInWorkspace(file3);
assertDoesNotExistInFileSystem(file3);
assertTrue("2.4.3", file3.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(file3).matches(isSynchronizedDepthInfinite, "is synchronized");

assertExistsInWorkspace(folder);
assertExistsInFileSystem(folder);
assertTrue("2.5.3", folder.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(folder).matches(isSynchronizedDepthInfinite, "is synchronized");

assertExistsInWorkspace(projectFile);
assertExistsInFileSystem(projectFile);
assertTrue("2.6.3", projectFile.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(projectFile).matches(isSynchronizedDepthInfinite, "is synchronized");

assertTrue("2.7.0", project.isSynchronized(IResource.DEPTH_ZERO));
assertTrue("2.7.1", project.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(project).matches(isSynchronizedDepthZero, "is synchronized");
assertThat(project).matches(isSynchronizedDepthInfinite, "is synchronized");
}

assertTrue("3.5", project.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(project).matches(isSynchronizedDepthInfinite, "is synchronized");
project.delete(IResource.FORCE, createTestMonitor());
assertTrue("5.1", !project.exists());
assertTrue("5.2", !file1.exists());
assertTrue("5.3", file1.isSynchronized(IResource.DEPTH_INFINITE));
assertTrue("5.4", project.isSynchronized(IResource.DEPTH_INFINITE));
assertTrue("6.0", !projectRoot.toFile().exists());
assertThat(project).matches(not(IResource::exists), "not exists");
assertThat(file1).matches(not(IResource::exists), "not exists");
assertThat(file1).matches(isSynchronizedDepthInfinite, "is synchronized");
assertThat(project).matches(isSynchronizedDepthInfinite, "is synchronized");
assertThat(projectRoot).matches(it -> !it.toFile().exists(), "not exists");
}

/**
Expand All @@ -144,29 +152,29 @@ public void testDeleteOpenProjectLinux() throws CoreException {
setReadOnly(folder, true);

IFile projectFile = project.getFile(".project");
assertTrue("1.2", projectFile.exists());
assertTrue("1.3", projectFile.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(projectFile).matches(IResource::exists, "exists");
assertThat(projectFile).matches(isSynchronizedDepthInfinite, "is synchronized");

assertThrows(CoreException.class, () -> project.delete(IResource.FORCE, createTestMonitor()));
assertTrue("2.1", project.exists());
assertTrue("2.2", file1.exists());
assertTrue("2.3", !file2.exists());
assertTrue("2.5", folder.exists());
assertTrue("2.6", projectFile.exists());
assertTrue("2.7", project.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(project).matches(IResource::exists, "exists");
assertThat(file1).matches(IResource::exists, "exists");
assertThat(file2).matches(not(IResource::exists), "not exists");
assertThat(folder).matches(IResource::exists, "exists");
assertThat(projectFile).matches(IResource::exists, "exists");
assertThat(project).matches(isSynchronizedDepthInfinite, "is synchronized");
} finally {
if (folder.exists()) {
setReadOnly(folder, false);
}
}

assertTrue("3.5", project.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(project).matches(isSynchronizedDepthInfinite, "is synchronized");
project.delete(IResource.FORCE, createTestMonitor());
assertTrue("5.1", !project.exists());
assertTrue("5.2", !file1.exists());
assertTrue("5.3", file1.isSynchronized(IResource.DEPTH_INFINITE));
assertTrue("5.4", project.isSynchronized(IResource.DEPTH_INFINITE));
assertTrue("6.0", !projectRoot.toFile().exists());
assertThat(project).matches(not(IResource::exists), "not exists");
assertThat(file1).matches(not(IResource::exists), "not exists");
assertThat(file1).matches(isSynchronizedDepthInfinite, "is synchronized");
assertThat(project).matches(isSynchronizedDepthInfinite, "is synchronized");
assertThat(projectRoot).matches(it -> !it.toFile().exists(), "not exists");
}

/**
Expand Down Expand Up @@ -194,16 +202,16 @@ public void testDeleteClosedProjectWindows() throws Exception {
project.close(createTestMonitor());
assertThrows(CoreException.class,
() -> project.delete(IResource.FORCE | IResource.ALWAYS_DELETE_PROJECT_CONTENT, createTestMonitor()));
assertTrue("2.1", project.exists());
assertTrue("2.7", project.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(project).matches(IResource::exists, "exists");
assertThat(project).matches(isSynchronizedDepthInfinite, "is synchronized");
assertExistsInFileSystem(projectFile);

}
assertTrue("3.5", project.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(project).matches(isSynchronizedDepthInfinite, "is synchronized");
project.delete(IResource.FORCE | IResource.ALWAYS_DELETE_PROJECT_CONTENT, createTestMonitor());
assertTrue("5.1", !project.exists());
assertTrue("5.3", project.isSynchronized(IResource.DEPTH_INFINITE));
assertTrue("6.0", !projectRoot.toFile().exists());
assertThat(project).matches(not(IResource::exists), "not exists");
assertThat(project).matches(isSynchronizedDepthInfinite, "is synchronized");
assertThat(projectRoot).matches(it -> !it.toFile().exists(), "not exists");
assertDoesNotExistInFileSystem(projectFile);
}

Expand Down Expand Up @@ -234,8 +242,8 @@ public void testDeleteClosedProjectLinux() throws CoreException {
assertThrows(CoreException.class,
() -> project.delete(IResource.FORCE | IResource.ALWAYS_DELETE_PROJECT_CONTENT, createTestMonitor()));

assertTrue("3.0", project.exists());
assertTrue("3.1", project.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(project).matches(IResource::exists, "exists");
assertThat(project).matches(isSynchronizedDepthInfinite, "is synchronized");
assertExistsInFileSystem(projectFile);

project.open(createTestMonitor());
Expand All @@ -246,9 +254,9 @@ public void testDeleteClosedProjectLinux() throws CoreException {
}

project.delete(IResource.FORCE | IResource.ALWAYS_DELETE_PROJECT_CONTENT, createTestMonitor());
assertTrue("6.0", !project.exists());
assertTrue("6.1", project.isSynchronized(IResource.DEPTH_INFINITE));
assertTrue("6.2", !projectRoot.toFile().exists());
assertThat(project).matches(not(IResource::exists), "not exists");
assertThat(project).matches(isSynchronizedDepthInfinite, "is synchronized");
assertThat(projectRoot).matches(it -> !it.toFile().exists(), "not exists");
assertDoesNotExistInFileSystem(projectFile);
}

Expand All @@ -273,18 +281,18 @@ public void testDeleteFolderWindows() throws Exception {
// opens a file so it cannot be removed on Windows
try (InputStream input = file1.getContents()) {
assertThrows(CoreException.class, () -> folder.delete(IResource.FORCE, createTestMonitor()));
assertTrue("2.2", file1.exists());
assertTrue("2.4", !file3.exists());
assertTrue("2.5", folder.exists());
assertTrue("2.7", folder.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(file1).matches(IResource::exists, "exists");
assertThat(file3).matches(not(IResource::exists), "not exists");
assertThat(folder).matches(IResource::exists, "exists");
assertThat(folder).matches(isSynchronizedDepthInfinite, "is synchronized");
}

assertTrue("3.5", project.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(project).matches(isSynchronizedDepthInfinite, "is synchronized");
folder.delete(IResource.FORCE, createTestMonitor());
assertTrue("5.1", !file1.exists());
assertTrue("5.2", !folder.exists());
assertTrue("5.3", file1.isSynchronized(IResource.DEPTH_INFINITE));
assertTrue("5.4", folder.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(file1).matches(not(IResource::exists), "not exists");
assertThat(folder).matches(not(IResource::exists), "not exists");
assertThat(file1).matches(isSynchronizedDepthInfinite, "is synchronized");
assertThat(folder).matches(isSynchronizedDepthInfinite, "is synchronized");
}

/**
Expand All @@ -311,24 +319,24 @@ public void testDeleteFolderLinux() throws CoreException {
setReadOnly(subFolder, true);

assertThrows(CoreException.class, () -> folder.delete(IResource.FORCE, createTestMonitor()));
assertTrue("2.2", file1.exists());
assertTrue("2.3", subFolder.exists());
assertTrue("2.4", !file3.exists());
assertTrue("2.5", folder.exists());
assertTrue("2.7", folder.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(file1).matches(IResource::exists, "exists");
assertThat(subFolder).matches(IResource::exists, "exists");
assertThat(file3).matches(not(IResource::exists), "not exists");
assertThat(folder).matches(IResource::exists, "exists");
assertThat(folder).matches(isSynchronizedDepthInfinite, "is synchronized");
} finally {
if (subFolder.exists()) {
setReadOnly(subFolder, false);
}
}

assertTrue("3.5", project.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(project).matches(isSynchronizedDepthInfinite, "is synchronized");
folder.delete(IResource.FORCE, createTestMonitor());
assertTrue("5.1", !file1.exists());
assertTrue("5.2", !subFolder.exists());
assertTrue("5.3", !folder.exists());
assertTrue("5.4", file1.isSynchronized(IResource.DEPTH_INFINITE));
assertTrue("5.5", folder.isSynchronized(IResource.DEPTH_INFINITE));
assertThat(file1).matches(not(IResource::exists), "not exists");
assertThat(subFolder).matches(not(IResource::exists), "not exists");
assertThat(folder).matches(not(IResource::exists), "not exists");
assertThat(file1).matches(isSynchronizedDepthInfinite, "is synchronized");
assertThat(folder).matches(isSynchronizedDepthInfinite, "is synchronized");
}

}
4 changes: 2 additions & 2 deletions ...ipse.core.tests.resources/src/org/eclipse/core/tests/resources/regression/Bug_027271.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@

import static org.assertj.core.api.Assertions.assertThat;
import static org.eclipse.core.resources.ResourcesPlugin.getWorkspace;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.assertFalse;
import static org.junit.Assume.assumeTrue;

import org.eclipse.core.resources.IPathVariableManager;
Expand Down Expand Up @@ -81,7 +81,7 @@ public void testBug() {
//sets invalid value (invalid path)
IPath invalidPath = IPath.fromOSString("c:\\a\\:\\b");
prefs.setValue(VARIABLE_PREFIX + "ANOTHER_INVALID_VAR", invalidPath.toPortableString());
assertTrue("3.0", !IPath.EMPTY.isValidPath(invalidPath.toPortableString()));
assertFalse(IPath.EMPTY.isValidPath(invalidPath.toPortableString()));
assertThat(pvm.getPathVariableNames()).containsExactly("VALID_VAR");
}

Expand Down
Loading

0 comments on commit a022584

Please sign in to comment.