Skip to content

Commit

Permalink
test: disable failing tests on Windows
Browse files Browse the repository at this point in the history
Disable a couple of tests that fail on Windows. The functionality is
tested sufficiently on other platforms, and getting the tests to work on
Windows has proven challenging.

These changes also converts the files containing those tests to JUnit 5.
  • Loading branch information
apotterri committed Oct 8, 2023
1 parent a388566 commit d7ce3ba
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 71 deletions.
1 change: 1 addition & 0 deletions agent/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ dependencies {
testImplementation 'org.junit.vintage:junit-vintage-engine'

testImplementation 'com.github.stefanbirkner:system-rules:1.19.0'
testImplementation 'com.github.stefanbirkner:system-lambda:1.2.1'
testImplementation "org.mockito:mockito-core:[3.4,4)"
testImplementation 'com.github.marschall:memoryfilesystem:2.6.1'
}
Expand Down
55 changes: 19 additions & 36 deletions agent/src/test/java/com/appland/appmap/config/AppMapConfigTest.java
Original file line number Diff line number Diff line change
@@ -1,54 +1,37 @@
package com.appland.appmap.config;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertTrue;
import static com.github.stefanbirkner.systemlambda.SystemLambda.tapSystemErr;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;

import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
import java.io.PrintStream;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;

import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.condition.OS;
import org.junit.rules.TemporaryFolder;
import org.junit.jupiter.api.io.TempDir;

public class AppMapConfigTest {

private final ByteArrayOutputStream errContent = new ByteArrayOutputStream();
private final PrintStream originalErr = System.err;

@Rule
public TemporaryFolder tmpdir = new TemporaryFolder();

@Before
public void setUpStreams() {
System.setErr(new PrintStream(errContent));
}

@After
public void restoreStreams() {
System.setErr(originalErr);
}
@TempDir
Path tmpdir;

@Test
@DisabledOnOs(OS.WINDOWS)
public void loadBadDirectory() {
public void loadBadDirectory() throws Exception {
// Trying to load appmap.yml in non-existent directory (that can't be created)
// shouldn't work.
Path badDir = Paths.get(File.listRoots()[0].toString(), "Program Files", "no-such-dir");
Path badDir = Paths.get("/no-such-dir");
assertFalse(Files.exists(badDir));
AppMapConfig.load(badDir.resolve("appmap.yml"), false);
assertNotNull(errContent.toString());
assertTrue(errContent.toString().contains("file not found"));
String actualErr = tapSystemErr(() -> AppMapConfig.load(badDir.resolve("appmap.yml"), false));
assertNotNull(actualErr.toString());
assertTrue(actualErr.contains("file not found"));
}

@Test
Expand All @@ -72,13 +55,13 @@ public void preservesExisting() throws IOException {
}

@Test
public void requiresExisting() throws IOException {
public void requiresExisting() throws Exception {
Path configFile = Paths.get(System.getProperty("java.io.tmpdir"), "appmap.yml");
Files.deleteIfExists(configFile);

AppMapConfig.load(configFile, true);
assertNotNull(errContent.toString());
assertTrue(errContent.toString().contains("file not found"));
String actualErr = tapSystemErr(() -> AppMapConfig.load(configFile, true));
assertNotNull(actualErr.toString());
assertTrue(actualErr.contains("file not found"));
}

// If a non-existent config file in a subdirectory is specified, the
Expand All @@ -96,7 +79,7 @@ public void loadParent() {

@Test
public void hasAppmapDir() throws Exception {
Path configFile = tmpdir.newFile("appmap.yml").toPath();
Path configFile = tmpdir.resolve("appmap.yml");
final String contents = "appmap_dir: /appmap\n";
Files.write(configFile, contents.getBytes());
AppMapConfig.load(configFile, true);
Expand Down
59 changes: 24 additions & 35 deletions agent/src/test/java/com/appland/appmap/record/RecorderTest.java
Original file line number Diff line number Diff line change
@@ -1,13 +1,11 @@
package com.appland.appmap.record;

import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import static org.junit.Assert.fail;
import static org.junit.Assume.assumeThat;
import static com.github.stefanbirkner.systemlambda.SystemLambda.tapSystemErr;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue;
import static org.junit.jupiter.api.Assertions.fail;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.spy;

Expand All @@ -33,12 +31,11 @@
import java.util.concurrent.Semaphore;

import org.apache.commons.lang3.StringUtils;
import org.junit.After;
import org.junit.Before;
import org.junit.Rule;
import org.junit.Test;
import org.junit.contrib.java.lang.system.SystemErrRule;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.BeforeEach;
import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.condition.DisabledOnOs;
import org.junit.jupiter.api.condition.EnabledOnOs;
import org.junit.jupiter.api.condition.OS;

import com.alibaba.fastjson.JSON;
Expand All @@ -47,10 +44,7 @@

public class RecorderTest {

@Rule
public final SystemErrRule systemErrRule = new SystemErrRule().enableLog();

@Before
@BeforeEach
public void before() throws Exception {
AppMapConfig.initialize(FileSystems.getDefault());

Expand All @@ -60,7 +54,7 @@ public void before() throws Exception {
Recorder.getInstance().start(metadata);
}

@After
@AfterEach
public void after() throws Exception {
if ( Recorder.getInstance().hasActiveSession()) {
Recorder.getInstance().stop();
Expand Down Expand Up @@ -112,7 +106,7 @@ public void testUnwriteableOutputFile() throws IOException {
final Recording recording = recorder.stop();
Exception exception = null;
try {
recording.moveTo(Paths.get(File.listRoots()[0].toString(), "Program Files", "no-such-directory", "."));
recording.moveTo("/no-such-directory/.");
} catch (RuntimeException e) {
exception = e;
}
Expand All @@ -121,44 +115,41 @@ public void testUnwriteableOutputFile() throws IOException {
}

@Test
@EnabledOnOs(OS.LINUX)
// for Files.move when: REPLACE_EXISTING, ATOMIC_MOVE
public void testWriteFileAcrossFilesystems() throws IOException {
// /dev/shm exists only on Linux
assumeThat(System.getProperty("os.name"), is("Linux"));
systemErrRule.clearLog();
public void testWriteFileAcrossFilesystems() throws Exception {
// /dev/shm exists only on Linux
String sourceFilename = "/tmp/recordertest_file";
String targetFilename = "/dev/shm/recordertest_file";
File sourceFile = new File(sourceFilename);
File targetFile = new File(targetFilename);
Exception exception = null;
// if the file exists createNewFile returns false
sourceFile.createNewFile();

String actualErr = tapSystemErr(() -> {
// Copying a file across filesystems should not throw an exception.
final Recording recording = new Recording("recorderName", sourceFile);
try {
recording.moveTo(targetFilename);
} catch (RuntimeException e) {
exception = e;
System.out.println(exception.getMessage());
} catch (RuntimeException e) {
fail("recording.moveTo failed, exception, writing across filesystems threw an exception", e);
} finally {
sourceFile.delete();
targetFile.delete();
}
assertNull("recording.moveTo failed, writing across filesystems threw an exception", exception);
assertFalse(systemErrRule.getLog().contains("Invalid cross-device link"));
});
assertFalse(actualErr.contains("Invalid cross-device link"));
}

@Test
@EnabledOnOs(OS.LINUX)
// for Files.move when: REPLACE_EXISTING
public void testCantOverwriteTargetFile() throws IOException {
// /dev/shm exists only on Linux
assumeThat(System.getProperty("os.name"), is("Linux"));
// /dev/shm exists only on Linux
String sourceFilename = "/tmp/recordertest_file";
String targetFilename = "/dev/shm/recordertest_file";
File sourceFile = new File(sourceFilename);
File targetFile = new File(targetFilename);
Exception exception = null;
// if the file exists createNewFile returns false
sourceFile.createNewFile();
targetFile.createNewFile();
Expand All @@ -176,13 +167,11 @@ public void testCantOverwriteTargetFile() throws IOException {
try {
recording.moveTo(targetFilename);
} catch (RuntimeException e) {
exception = e;
System.out.println(exception.getMessage());
fail("recording.moveTo failed, overwriting the destination threw an exception", e);
} finally {
sourceFile.delete();
targetFile.delete();
}
assertNull("recording.moveTo failed, overwriting the destination threw an exception", exception);
}

@Test
Expand Down

0 comments on commit d7ce3ba

Please sign in to comment.