Skip to content

Commit

Permalink
Replace custom stream comparison assertion with library call #903
Browse files Browse the repository at this point in the history 
The ResourceTestUtil provides a compareContents() method for comparing
the contents of two streams. This method is used in assertTrue()
statements to check for equality of contents of two stream. Modern
assertion frameworks already provide that functionality and, in
addition, also provide proper error messages when the comparison yields
differences.

This change removes the compareContents() method and replaces all calls
with either direct string comparison if the indirection over a stream
became unnecesary or using assertThat().hasContent() or
assertThat().hasSameContentAs().

Contributes to
#903
  • Loading branch information
@HeikoKlare
HeikoKlare committed Aug 18, 2024
1 parent 5f7b6e7 commit 45aa1a4
Show file tree
Hide file tree
Showing 15 changed files with 208 additions and 194 deletions.
51 changes: 38 additions & 13 deletions ...org.eclipse.core.tests.resources/src/org/eclipse/core/tests/filesystem/FileStoreTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@
*******************************************************************************/
package org.eclipse.core.tests.filesystem;

import static org.assertj.core.api.Assertions.assertThat;
import static org.eclipse.core.resources.ResourcesPlugin.getWorkspace;
import static org.eclipse.core.tests.harness.FileSystemHelper.getTempDir;
import static org.eclipse.core.tests.internal.localstore.LocalStoreTestUtil.createTree;
import static org.eclipse.core.tests.internal.localstore.LocalStoreTestUtil.getTree;
import static org.eclipse.core.tests.resources.ResourceTestUtil.compareContent;
import static org.eclipse.core.tests.resources.ResourceTestUtil.createInFileSystem;
import static org.eclipse.core.tests.resources.ResourceTestUtil.createInputStream;
import static org.eclipse.core.tests.resources.ResourceTestUtil.createRandomString;
Expand All @@ -35,6 +35,7 @@

import java.io.File;
import java.io.IOException;
import java.io.InputStream;
import java.io.OutputStream;
import java.net.URI;
import java.util.ArrayList;
Expand Down Expand Up @@ -241,15 +242,21 @@ public void testCaseInsensitive() throws Throwable {
createFile(fileWithSmallName, content);
System.out.println(fileWithSmallName.fetchInfo().getName());
assertTrue("1.3", fileWithSmallName.fetchInfo().exists());
assertTrue("1.4", compareContent(createInputStream(content), fileWithSmallName.openInputStream(EFS.NONE, null)));
try (InputStream stream = fileWithSmallName.openInputStream(EFS.NONE, null)) {
assertThat(stream).hasContent(content);
}

IFileStore fileWithOtherName = temp.getChild("FILENAME");
System.out.println(fileWithOtherName.fetchInfo().getName());
// file content is already the same for both Cases:
assertTrue("2.0", compareContent(createInputStream(content), fileWithOtherName.openInputStream(EFS.NONE, null)));
try (InputStream stream = fileWithOtherName.openInputStream(EFS.NONE, null)) {
assertThat(stream).hasContent(content);
}
fileWithSmallName.copy(fileWithOtherName, IResource.DEPTH_INFINITE, null); // a NOP Operation
// file content is still the same for both Cases:
assertTrue("2.1", compareContent(createInputStream(content), fileWithOtherName.openInputStream(EFS.NONE, null)));
try (InputStream stream = fileWithOtherName.openInputStream(EFS.NONE, null)) {
assertThat(stream).hasContent(content);
}
assertTrue("3.0", fileWithOtherName.fetchInfo().exists());
assertTrue("3.1", fileWithSmallName.fetchInfo().exists());
fileWithOtherName.delete(EFS.NONE, null);
Expand All @@ -271,12 +278,16 @@ public void testCopyFile() throws Throwable {
target.delete(EFS.NONE, null);
createFile(target, content);
assertTrue("1.3", target.fetchInfo().exists());
assertTrue("1.4", compareContent(createInputStream(content), target.openInputStream(EFS.NONE, null)));
try (InputStream stream = target.openInputStream(EFS.NONE, null)) {
assertThat(stream).hasContent(content);
}

/* temp\target -> temp\copy of target */
IFileStore copyOfTarget = temp.getChild("copy of target");
target.copy(copyOfTarget, IResource.DEPTH_INFINITE, null);
assertTrue("2.1", compareContent(createInputStream(content), copyOfTarget.openInputStream(EFS.NONE, null)));
try (InputStream stream = copyOfTarget.openInputStream(EFS.NONE, null)) {
assertThat(stream).hasContent(content);
}
copyOfTarget.delete(EFS.NONE, null);

// We need to know whether or not we can unset the read-only flag
Expand All @@ -287,7 +298,9 @@ public void testCopyFile() throws Throwable {
setReadOnly(target, true);

target.copy(copyOfTarget, IResource.DEPTH_INFINITE, null);
assertTrue("3.1", compareContent(createInputStream(content), copyOfTarget.openInputStream(EFS.NONE, null)));
try (InputStream stream = copyOfTarget.openInputStream(EFS.NONE, null)) {
assertThat(stream).hasContent(content);
}
// reset read only flag for cleanup
setReadOnly(copyOfTarget, false);
copyOfTarget.delete(EFS.NONE, null);
Expand All @@ -304,12 +317,16 @@ public void testCopyFile() throws Throwable {
IFileStore bigFile = temp.getChild("bigFile");
createFile(bigFile, sb.toString());
assertTrue("7.1", bigFile.fetchInfo().exists());
assertTrue("7.2", compareContent(createInputStream(sb.toString()), bigFile.openInputStream(EFS.NONE, null)));
try (InputStream stream = bigFile.openInputStream(EFS.NONE, null)) {
assertThat(stream).hasContent(sb.toString());
}
IFileStore destination = temp.getChild("copy of bigFile");
// IProgressMonitor monitor = new LoggingProgressMonitor(System.out);
IProgressMonitor monitor = createTestMonitor();
bigFile.copy(destination, EFS.NONE, monitor);
assertTrue("7.3", compareContent(createInputStream(sb.toString()), destination.openInputStream(EFS.NONE, null)));
try (InputStream stream = destination.openInputStream(EFS.NONE, null)) {
assertThat(stream).hasContent(sb.toString());
}
destination.delete(EFS.NONE, null);
}

Expand Down Expand Up @@ -338,21 +355,27 @@ public void testCopyFileAcrossVolumes() throws Throwable {
createFile(target, content);
workspaceRule.deleteOnTearDown(target);
assertTrue("1.3", target.fetchInfo().exists());
assertTrue("1.4", compareContent(createInputStream(content), target.openInputStream(EFS.NONE, null)));
try (InputStream stream = target.openInputStream(EFS.NONE, null)) {
assertThat(stream).hasContent(content);
}

/* c:\temp\target -> d:\temp\target */
IFileStore destination = tempDest.getChild(subfolderName);
workspaceRule.deleteOnTearDown(destination);
target.copy(destination, IResource.DEPTH_INFINITE, null);
assertTrue("3.1", compareContent(createInputStream(content), destination.openInputStream(EFS.NONE, null)));
try (InputStream stream = destination.openInputStream(EFS.NONE, null)) {
assertThat(stream).hasContent(content);
}
destination.delete(EFS.NONE, null);

/* c:\temp\target -> d:\temp\copy of target */
String copyOfSubfoldername = "copy of " + subfolderName;
destination = tempDest.getChild(copyOfSubfoldername);
workspaceRule.deleteOnTearDown(destination);
target.copy(destination, IResource.DEPTH_INFINITE, null);
assertTrue("4.1", compareContent(createInputStream(content), destination.openInputStream(EFS.NONE, null)));
try (InputStream stream = destination.openInputStream(EFS.NONE, null)) {
assertThat(stream).hasContent(content);
}
destination.delete(EFS.NONE, null);

/* c:\temp\target -> d:\temp\target (but the destination is already a file */
Expand All @@ -362,7 +385,9 @@ public void testCopyFileAcrossVolumes() throws Throwable {
createFile(destination, anotherContent);
assertTrue("5.1", !destination.fetchInfo().isDirectory());
target.copy(destination, IResource.DEPTH_INFINITE, null);
assertTrue("5.2", compareContent(createInputStream(content), destination.openInputStream(EFS.NONE, null)));
try (InputStream stream = destination.openInputStream(EFS.NONE, null)) {
assertThat(stream).hasContent(content);
}
destination.delete(EFS.NONE, null);

/* c:\temp\target -> d:\temp\target (but the destination is already a folder */
Expand Down
12 changes: 7 additions & 5 deletions ...se.core.tests.resources/src/org/eclipse/core/tests/internal/localstore/BlobStoreTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*******************************************************************************/
package org.eclipse.core.tests.internal.localstore;

import static org.eclipse.core.tests.resources.ResourceTestUtil.compareContent;
import static org.assertj.core.api.Assertions.assertThat;
import static org.eclipse.core.tests.resources.ResourceTestUtil.createInFileSystem;
import static org.eclipse.core.tests.resources.ResourceTestUtil.createInputStream;
import static org.junit.Assert.assertFalse;
Expand Down Expand Up @@ -109,8 +109,9 @@ public void testGetBlob() throws CoreException, IOException {
createInputStream(content).transferTo(output);
}
uuid = store.addBlob(target, true);
InputStream input = store.getBlob(uuid);
assertTrue(compareContent(createInputStream(content), input));
try (InputStream input = store.getBlob(uuid)) {
assertThat(input).hasContent(content);
}
}

@Test
Expand All @@ -127,8 +128,9 @@ public void testSetBlob() throws CoreException, IOException {
createInputStream(content).transferTo(output);
}
uuid = store.addBlob(target, true);
InputStream input = store.getBlob(uuid);
assertTrue(compareContent(createInputStream(content), input));
try (InputStream input = store.getBlob(uuid)) {
assertThat(input).hasContent(content);
}
}

}
48 changes: 25 additions & 23 deletions ...sources/src/org/eclipse/core/tests/internal/localstore/FileSystemResourceManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,9 @@
*******************************************************************************/
package org.eclipse.core.tests.internal.localstore;

import static org.assertj.core.api.Assertions.assertThat;
import static org.eclipse.core.resources.ResourcesPlugin.getWorkspace;
import static org.eclipse.core.tests.resources.ResourceTestUtil.buildResources;
import static org.eclipse.core.tests.resources.ResourceTestUtil.compareContent;
import static org.eclipse.core.tests.resources.ResourceTestUtil.createInFileSystem;
import static org.eclipse.core.tests.resources.ResourceTestUtil.createInWorkspace;
import static org.eclipse.core.tests.resources.ResourceTestUtil.createInputStream;
Expand Down Expand Up @@ -149,7 +149,9 @@ public void testCreateFile() throws Exception {
assertTrue(file.exists());
assertTrue(file.isLocal(IResource.DEPTH_ZERO));
assertEquals(file.getStore().fetchInfo().getLastModified(), file.getResourceInfo(false, false).getLocalSyncInfo());
assertTrue(compareContent(createInputStream(originalContent), getLocalManager().read(file, true, null)));
try (InputStream readFile = getLocalManager().read(file, true, null)) {
assertThat(readFile).hasContent(originalContent);
}
}

@Test
Expand Down Expand Up @@ -275,32 +277,30 @@ public void testWriteFile() throws Exception {
/* common contents */
String originalContent = "this string should not be equal the other";
String anotherContent = "and this string should not... well, you know...";
InputStream original;
InputStream another;

/* write file for the first time */
original = createInputStream(originalContent);
write(file, original, true, null);

original = createInputStream(originalContent);
assertTrue("Unexpected content in " + original,
compareContent(original, getLocalManager().read(file, true, null)));
try (InputStream original = createInputStream(originalContent)) {
write(file, original, true, null);
}
try (InputStream readFile = getLocalManager().read(file, true, null)) {
assertThat(readFile).hasContent(originalContent);
}

/* test the overwrite parameter (false) */
another = createInputStream(anotherContent);
write(file, another, false, null);

another = createInputStream(anotherContent);
assertTrue("Unexpected content in " + another,
compareContent(another, getLocalManager().read(file, true, null)));
try (InputStream another = createInputStream(anotherContent)) {
write(file, another, false, null);
}
try (InputStream readFile = getLocalManager().read(file, true, null)) {
assertThat(readFile).hasContent(anotherContent);
}

/* test the overwrite parameter (true) */
original = createInputStream(originalContent);
write(file, original, true, null);

original = createInputStream(originalContent);
assertTrue("Unexpected content in " + original,
compareContent(original, getLocalManager().read(file, true, null)));
try (InputStream original = createInputStream(originalContent)) {
write(file, original, true, null);
}
try (InputStream readFile = getLocalManager().read(file, true, null)) {
assertThat(readFile).hasContent(originalContent);
}

/* test the overwrite parameter (false) */
ensureOutOfSync(file);
Expand All @@ -310,7 +310,9 @@ public void testWriteFile() throws Exception {
ensureOutOfSync(file);
assertThrows("Should fail writing out of sync file #2", CoreException.class,
() -> file.setContents(another2, false, false, null));
file.setContents(another, true, false, null);
try (InputStream another = createInputStream(anotherContent)) {
file.setContents(another, true, false, null);
}

/* test the overwrite parameter (false) */
removeFromFileSystem(file); // FIXME Race Condition with asynchronous workplace refresh see Bug 571133
Expand Down
Loading

0 comments on commit 45aa1a4

Please sign in to comment.