diff --git a/.github/workflows/main-ci.yml b/.github/workflows/main-ci.yml index acb8e375b..64ffecbca 100644 --- a/.github/workflows/main-ci.yml +++ b/.github/workflows/main-ci.yml @@ -88,5 +88,5 @@ jobs: ./gradlew --configure-on-demand :core:test - name: Parse test reports - run: python ./tools/parse-reports.py + run: python3 ./tools/parse-reports.py if: ${{ failure() }} diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 1e47b94c2..736c217cb 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -106,6 +106,7 @@ jqwik-engine = { module = "net.jqwik:jqwik-engine", version.ref = "jqwik" } junit-jupiter-api = { module = "org.junit.jupiter:junit-jupiter-api", version.ref = "junit" } junit-jupiter-engine = { module = "org.junit.jupiter:junit-jupiter-engine", version.ref = "junit" } junit-jupiter-params = { module = "org.junit.jupiter:junit-jupiter-params", version.ref = "junit" } +logback = { module = "ch.qos.logback:logback-classic", version.ref = "logback" } # Build tools cctJavadoc = { module = "cc.tweaked:cct-javadoc", version.ref = "cctJavadoc" } diff --git a/illuaminate.sexp b/illuaminate.sexp index c1e984e81..70737cae3 100644 --- a/illuaminate.sexp +++ b/illuaminate.sexp @@ -112,6 +112,6 @@ (lint (globals :max sleep write - cct_test describe expect howlci fail it pending stub))) + cct_test describe expect howlci fail it pending stub before_each))) (at /projects/web/src/mount/expr_template.lua (lint (globals :max __expr__))) diff --git a/projects/core-api/src/main/java/dan200/computercraft/api/filesystem/WritableMount.java b/projects/core-api/src/main/java/dan200/computercraft/api/filesystem/WritableMount.java index c30a6845c..e2660b60c 100644 --- a/projects/core-api/src/main/java/dan200/computercraft/api/filesystem/WritableMount.java +++ b/projects/core-api/src/main/java/dan200/computercraft/api/filesystem/WritableMount.java @@ -10,7 +10,6 @@ import java.io.IOException; import java.io.OutputStream; import java.nio.channels.WritableByteChannel; -import java.util.OptionalLong; /** * Represents a part of a virtual filesystem that can be mounted onto a computer using {@link IComputerAccess#mount(String, Mount)} @@ -40,6 +39,17 @@ public interface WritableMount extends Mount { */ void delete(String path) throws IOException; + /** + * Rename a file or directory, moving it from one path to another. + *
+ * The destination path should not exist. The parent of the destination should exist and be a directory. If source
+ * and destination are the same, this method should do nothing.
+ *
+ * @param source The source file or directory to move.
+ * @param dest The destination path.
+ */
+ void rename(String source, String dest) throws IOException;
+
/**
* Opens a file with a given path, and returns an {@link OutputStream} for writing to it.
*
@@ -75,9 +85,7 @@ public interface WritableMount extends Mount {
*
* @return The capacity of this mount, in bytes.
*/
- default OptionalLong getCapacity() {
- return OptionalLong.empty();
- }
+ long getCapacity();
/**
* Returns whether a file with a given path is read-only or not.
diff --git a/projects/core/build.gradle.kts b/projects/core/build.gradle.kts
index 370af85ea..2799f00e0 100644
--- a/projects/core/build.gradle.kts
+++ b/projects/core/build.gradle.kts
@@ -25,6 +25,7 @@ dependencies {
testImplementation(libs.bundles.test)
testRuntimeOnly(libs.bundles.testRuntime)
+ testRuntimeOnly(libs.logback)
}
tasks.processResources {
diff --git a/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileMount.java b/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileMount.java
index 6e2cfc370..f72fd7526 100644
--- a/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileMount.java
+++ b/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileMount.java
@@ -14,12 +14,14 @@
import java.io.File;
import java.io.IOException;
import java.nio.ByteBuffer;
-import java.nio.channels.*;
+import java.nio.channels.ClosedChannelException;
+import java.nio.channels.NonReadableChannelException;
+import java.nio.channels.SeekableByteChannel;
+import java.nio.channels.WritableByteChannel;
import java.nio.file.*;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.Collections;
import java.util.List;
-import java.util.OptionalLong;
import java.util.Set;
public class FileMount implements WritableMount {
@@ -185,7 +187,7 @@ public long getSize(String path) throws IOException {
public SeekableByteChannel openForRead(String path) throws IOException {
if (created()) {
var file = getRealPath(path);
- if (file.exists() && !file.isDirectory()) return FileChannel.open(file.toPath(), READ_OPTIONS);
+ if (file.exists() && !file.isDirectory()) return Files.newByteChannel(file.toPath(), READ_OPTIONS);
}
throw new FileOperationException(path, "No such file");
@@ -259,6 +261,22 @@ private void deleteRecursively(File file) throws IOException {
}
}
+ @Override
+ public void rename(String source, String dest) throws IOException {
+ var sourceFile = getRealPath(source);
+ var destFile = getRealPath(dest);
+ if (!sourceFile.exists()) throw new FileOperationException(source, "No such file");
+ if (destFile.exists()) throw new FileOperationException(dest, "File exists");
+
+ var sourcePath = sourceFile.toPath();
+ var destPath = destFile.toPath();
+ if (destPath.startsWith(sourcePath)) {
+ throw new FileOperationException(source, "Cannot move a directory inside itself");
+ }
+
+ Files.move(sourcePath, destPath);
+ }
+
@Override
public WritableByteChannel openForWrite(String path) throws IOException {
create();
@@ -298,8 +316,8 @@ public long getRemainingSpace() {
}
@Override
- public OptionalLong getCapacity() {
- return OptionalLong.of(capacity - MINIMUM_FILE_SIZE);
+ public long getCapacity() {
+ return capacity - MINIMUM_FILE_SIZE;
}
private File getRealPath(String path) {
diff --git a/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileSystem.java b/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileSystem.java
index f19606f17..c2c6ceb6a 100644
--- a/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileSystem.java
+++ b/projects/core/src/main/java/dan200/computercraft/core/filesystem/FileSystem.java
@@ -248,20 +248,19 @@ public synchronized void delete(String path) throws FileSystemException {
public synchronized void move(String sourcePath, String destPath) throws FileSystemException {
sourcePath = sanitizePath(sourcePath);
destPath = sanitizePath(destPath);
- if (isReadOnly(sourcePath) || isReadOnly(destPath)) {
- throw new FileSystemException("Access denied");
+
+ if (isReadOnly(sourcePath) || isReadOnly(destPath)) throw new FileSystemException("Access denied");
+ if (!exists(sourcePath)) throw new FileSystemException("No such file");
+ if (exists(destPath)) throw new FileSystemException("File exists");
+ if (contains(sourcePath, destPath)) throw new FileSystemException("Can't move a directory inside itself");
+
+ var mount = getMount(sourcePath);
+ if (mount == getMount(destPath)) {
+ mount.rename(sourcePath, destPath);
+ } else {
+ copy(sourcePath, destPath);
+ delete(sourcePath);
}
- if (!exists(sourcePath)) {
- throw new FileSystemException("No such file");
- }
- if (exists(destPath)) {
- throw new FileSystemException("File exists");
- }
- if (contains(sourcePath, destPath)) {
- throw new FileSystemException("Can't move a directory inside itself");
- }
- copy(sourcePath, destPath);
- delete(sourcePath);
}
public synchronized void copy(String sourcePath, String destPath) throws FileSystemException {
diff --git a/projects/core/src/main/java/dan200/computercraft/core/filesystem/MountWrapper.java b/projects/core/src/main/java/dan200/computercraft/core/filesystem/MountWrapper.java
index 018c90763..d37c39de3 100644
--- a/projects/core/src/main/java/dan200/computercraft/core/filesystem/MountWrapper.java
+++ b/projects/core/src/main/java/dan200/computercraft/core/filesystem/MountWrapper.java
@@ -14,6 +14,8 @@
import java.nio.channels.SeekableByteChannel;
import java.nio.channels.WritableByteChannel;
import java.nio.file.AccessDeniedException;
+import java.nio.file.FileAlreadyExistsException;
+import java.nio.file.NoSuchFileException;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.List;
import java.util.OptionalLong;
@@ -58,7 +60,7 @@ public long getFreeSpace() {
}
public OptionalLong getCapacity() {
- return writableMount == null ? OptionalLong.empty() : writableMount.getCapacity();
+ return writableMount == null ? OptionalLong.empty() : OptionalLong.of(writableMount.getCapacity());
}
public boolean isReadOnly(String path) throws FileSystemException {
@@ -163,6 +165,25 @@ public void delete(String path) throws FileSystemException {
}
}
+ public void rename(String source, String dest) throws FileSystemException {
+ if (writableMount == null) throw exceptionOf(source, "Access denied");
+
+ source = toLocal(source);
+ dest = toLocal(dest);
+ try {
+ if (!dest.isEmpty()) {
+ var destParent = FileSystem.getDirectory(dest);
+ if (!destParent.isEmpty() && !mount.exists(destParent)) writableMount.makeDirectory(destParent);
+ }
+
+ writableMount.rename(source, dest);
+ } catch (AccessDeniedException e) {
+ throw new FileSystemException("Access denied");
+ } catch (IOException e) {
+ throw localExceptionOf(source, e);
+ }
+ }
+
public WritableByteChannel openForWrite(String path) throws FileSystemException {
if (writableMount == null) throw exceptionOf(path, "Access denied");
@@ -223,7 +244,7 @@ private FileSystemException localExceptionOf(@Nullable String localPath, IOExcep
if (e instanceof java.nio.file.FileSystemException ex) {
// This error will contain the absolute path, leaking information about where MC is installed. We drop that,
// just taking the reason. We assume that the error refers to the input path.
- var message = ex.getReason().trim();
+ var message = getReason(ex);
return localPath == null ? new FileSystemException(message) : localExceptionOf(localPath, message);
}
@@ -238,4 +259,15 @@ private FileSystemException localExceptionOf(String path, String message) {
private static FileSystemException exceptionOf(String path, String message) {
return new FileSystemException("/" + path + ": " + message);
}
+
+ private static String getReason(java.nio.file.FileSystemException e) {
+ var reason = e.getReason();
+ if (reason != null) return reason.trim();
+
+ if (e instanceof FileAlreadyExistsException) return "File exists";
+ if (e instanceof NoSuchFileException) return "No such file";
+ if (e instanceof AccessDeniedException) return "Access denied";
+
+ return "Operation failed";
+ }
}
diff --git a/projects/core/src/test/java/dan200/computercraft/core/computer/ComputerThreadTest.java b/projects/core/src/test/java/dan200/computercraft/core/computer/ComputerThreadTest.java
index f510030cf..80f4bda61 100644
--- a/projects/core/src/test/java/dan200/computercraft/core/computer/ComputerThreadTest.java
+++ b/projects/core/src/test/java/dan200/computercraft/core/computer/ComputerThreadTest.java
@@ -17,10 +17,12 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
+import java.util.Locale;
import java.util.concurrent.TimeUnit;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.closeTo;
+import static org.hamcrest.Matchers.lessThanOrEqualTo;
import static org.junit.jupiter.api.Assertions.*;
@Timeout(value = 15)
@@ -94,7 +96,9 @@ public void testPauseIfSomeOtherMachine() throws Exception {
assertEquals(budget, TimeUnit.MILLISECONDS.toNanos(25), "Budget should be 25ms");
var delay = ConcurrentHelpers.waitUntil(timeout::isPaused);
- assertThat("Paused within 25ms", delay * 1e-9, closeTo(0.025, 0.025));
+ // Linux appears to have much more accurate timing than Windows/OSX. Or at least on CI!
+ var time = System.getProperty("os.name", "").toLowerCase(Locale.ROOT).contains("linux") ? 0.05 : 0.3;
+ assertThat("Paused within a short time", delay * 1e-9, lessThanOrEqualTo(time));
computer.shutdown();
return MachineResult.OK;
diff --git a/projects/core/src/test/java/dan200/computercraft/core/filesystem/FileMountTest.java b/projects/core/src/test/java/dan200/computercraft/core/filesystem/FileMountTest.java
index 6d9e7cba8..9b3a1581f 100644
--- a/projects/core/src/test/java/dan200/computercraft/core/filesystem/FileMountTest.java
+++ b/projects/core/src/test/java/dan200/computercraft/core/filesystem/FileMountTest.java
@@ -10,22 +10,14 @@
import dan200.computercraft.api.filesystem.WritableMount;
import org.junit.jupiter.api.AfterEach;
import org.junit.jupiter.api.Assumptions;
-import org.junit.jupiter.api.Test;
import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
-import java.nio.file.attribute.PosixFileAttributeView;
-import java.nio.file.attribute.PosixFilePermission;
import java.util.ArrayList;
import java.util.List;
-import java.util.Set;
-import static org.junit.jupiter.api.Assertions.assertFalse;
-import static org.junit.jupiter.api.Assertions.assertTrue;
-
-public class FileMountTest {
- private static final long CAPACITY = 1_000_000;
+public class FileMountTest implements WritableMountContract {
private final List
+ * This should recompute the value where possible, rather than using {@link WritableMount#getRemainingSpace()}:
+ * its purpose is to ensure the value is accurate!
+ *
+ * @return The new
+ * @throws IOException If the remaining space could not be computed.
+ */
+ long computeRemainingSpace() throws IOException;
+ }
+}
diff --git a/projects/core/src/test/resources/test-rom/mcfly.lua b/projects/core/src/test/resources/test-rom/mcfly.lua
index 258a51236..b362f6bb0 100644
--- a/projects/core/src/test/resources/test-rom/mcfly.lua
+++ b/projects/core/src/test/resources/test-rom/mcfly.lua
@@ -417,6 +417,9 @@ end
--- The stack of "describe"s.
local test_stack = { n = 0 }
+--- The stack of setup functions.
+local before_each_fns = { n = 0 }
+
--- Whether we're now running tests, and so cannot run any more.
local tests_locked = false
@@ -455,8 +458,14 @@ local function describe(name, body)
local n = test_stack.n + 1
test_stack[n], test_stack.n = name, n
+ local old_before, new_before = before_each_fns, { n = before_each_fns.n }
+ for i = 1, old_before.n do new_before[i] = old_before[i] end
+ before_each_fns = new_before
+
local ok, err = try(body)
+ before_each_fns = old_before
+
-- We count errors as a (failing) test.
if not ok then do_test { error = err, definition = format_loc(debug.getinfo(2, "Sl")) } end
@@ -477,7 +486,11 @@ local function it(name, body)
local n = test_stack.n + 1
test_stack[n], test_stack.n, tests_locked = name, n, true
- do_test { action = body, definition = format_loc(debug.getinfo(2, "Sl")) }
+ do_test {
+ action = body,
+ before = before_each_fns,
+ definition = format_loc(debug.getinfo(2, "Sl")),
+ }
-- Pop the test from the stack
test_stack.n, tests_locked = n - 1, false
@@ -498,6 +511,13 @@ local function pending(name)
test_stack.n = n - 1
end
+local function before_each(body)
+ check('it', 1, 'function', body)
+
+ local n = before_each_fns.n + 1
+ before_each_fns[n], before_each_fns.n = body, n
+end
+
local native_co_create, native_loadfile = coroutine.create, loadfile
local line_counts = {}
if cct_test then
@@ -568,7 +588,7 @@ do
end
-- When declaring tests, you shouldn't be able to use test methods
- set_env { describe = describe, it = it, pending = pending }
+ set_env { describe = describe, it = it, pending = pending, before_each = before_each }
local suffix = "_spec.lua"
local function run_in(sub_dir)
@@ -630,8 +650,13 @@ local function do_run(test)
-- Flush the event queue and ensure we're running with 0 timeout.
os.queueEvent("start_test") os.pullEvent("start_test")
- local ok
- ok, err = try(test.action)
+ local ok = true
+ for i = 1, test.before.n do
+ if not ok then break end
+ ok, err = try(test.before[i])
+ end
+ if ok then ok, err = try(test.action) end
+
status = ok and "pass" or (err.fail and "fail" or "error")
pop_state(state)
diff --git a/projects/core/src/test/resources/test-rom/spec/apis/fs_spec.lua b/projects/core/src/test/resources/test-rom/spec/apis/fs_spec.lua
index 7bcab9e86..539d30dc2 100644
--- a/projects/core/src/test/resources/test-rom/spec/apis/fs_spec.lua
+++ b/projects/core/src/test/resources/test-rom/spec/apis/fs_spec.lua
@@ -1,4 +1,8 @@
describe("The fs library", function()
+ local test_root = "/test-files/fs"
+ local function test_file(path) return fs.combine(test_root, path) end
+ before_each(function() fs.delete(test_root) end)
+
describe("fs.complete", function()
it("validates arguments", function()
fs.complete("", "")
@@ -139,7 +143,7 @@ describe("The fs library", function()
end)
it("errors when closing twice", function()
- local handle = fs.open("test-files/out.txt", "w")
+ local handle = fs.open(test_file "out.txt", "w")
handle.close()
expect.error(handle.close):eq("attempt to use a closed file")
end)
@@ -216,6 +220,48 @@ describe("The fs library", function()
expect.error(fs.move, "test-files", "rom/move"):eq("Access denied")
expect.error(fs.move, "rom", "test-files"):eq("Access denied")
end)
+
+ it("fails if source does not exist", function()
+ expect.error(fs.move, test_file "src", test_file "dest"):eq("No such file")
+ end)
+
+ it("fails if destination exists", function()
+ fs.open(test_file "src", "w").close()
+ fs.open(test_file "dest", "w").close()
+
+ expect.error(fs.move, test_file "src", test_file "dest"):eq("File exists")
+ end)
+
+ it("fails to move a directory inside itself", function()
+ fs.open(test_file "file", "w").close()
+ expect.error(fs.move, test_root, test_file "child"):eq("Can't move a directory inside itself")
+ expect.error(fs.move, "", "child"):eq("Can't move a directory inside itself")
+ end)
+
+ it("files can be renamed", function()
+ fs.open(test_file "src", "w").close()
+ fs.move(test_file "src", test_file" dest")
+
+ expect(fs.exists(test_file "src")):eq(false)
+ expect(fs.exists(test_file "dest")):eq(true)
+ end)
+
+ it("directories can be renamed", function()
+ fs.open(test_file "src/some/file", "w").close()
+ fs.move(test_file "src", test_file" dest")
+
+ expect(fs.exists(test_file "src")):eq(false)
+ expect(fs.exists(test_file "dest")):eq(true)
+ expect(fs.exists(test_file "dest/some/file")):eq(true)
+ end)
+
+ it("creates directories before renaming", function()
+ fs.open(test_file "src", "w").close()
+ fs.move(test_file "src", test_file "dest/file")
+
+ expect(fs.exists(test_file "src")):eq(false)
+ expect(fs.exists(test_file "dest/file")):eq(true)
+ end)
end)
describe("fs.getCapacity", function()
@@ -240,12 +286,11 @@ describe("The fs library", function()
it("returns information about files", function()
local now = os.epoch("utc")
- fs.delete("/tmp/basic-file")
- local h = fs.open("/tmp/basic-file", "w")
+ local h = fs.open(test_file "basic-file", "w")
h.write("A reasonably sized string")
h.close()
- local attributes = fs.attributes("tmp/basic-file")
+ local attributes = fs.attributes(test_file "basic-file")
expect(attributes):matches { isDir = false, size = 25, isReadOnly = false }
if attributes.created - now >= 1000 then
diff --git a/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/MemoryMount.java b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/MemoryMount.java
index 6b58f1648..3484adbdc 100644
--- a/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/MemoryMount.java
+++ b/projects/core/src/testFixtures/java/dan200/computercraft/test/core/filesystem/MemoryMount.java
@@ -29,7 +29,6 @@ public MemoryMount() {
directories.add("");
}
-
@Override
public void makeDirectory(String path) {
var file = new File(path);
@@ -56,6 +55,11 @@ public void delete(String path) {
}
}
+ @Override
+ public void rename(String source, String dest) throws IOException {
+ throw new IOException("Not supported");
+ }
+
@Override
public WritableByteChannel openForWrite(final String path) {
return Channels.newChannel(new ByteArrayOutputStream() {
@@ -110,6 +114,11 @@ public long getSize(String path) {
throw new RuntimeException("Not implemented");
}
+ @Override
+ public long getCapacity() {
+ return Long.MAX_VALUE;
+ }
+
@Override
public SeekableByteChannel openForRead(String path) throws FileOperationException {
var file = files.get(path);