From 020c5cd2d373bef9c45813a8ca0fbe32ffd4ca5f Mon Sep 17 00:00:00 2001 From: Jonathan Coates Date: Sun, 4 Dec 2022 21:59:30 +0000 Subject: [PATCH] Support renaming files directly without copying/deleting In classic squid tradition: 20% code, and 80% test logic. Closes #962. Alas, whoever reported this has deleted their account, so they can't even be happy about it :(. --- .github/workflows/main-ci.yml | 2 +- gradle/libs.versions.toml | 1 + illuaminate.sexp | 2 +- .../api/filesystem/WritableMount.java | 16 ++- projects/core/build.gradle.kts | 1 + .../core/filesystem/FileMount.java | 28 ++++- .../core/filesystem/FileSystem.java | 25 ++-- .../core/filesystem/MountWrapper.java | 36 +++++- .../core/computer/ComputerThreadTest.java | 6 +- .../core/filesystem/FileMountTest.java | 67 +++++----- .../core/filesystem/FileSystemTest.java | 18 +-- .../filesystem/WritableMountContract.java | 114 ++++++++++++++++++ .../src/test/resources/test-rom/mcfly.lua | 33 ++++- .../resources/test-rom/spec/apis/fs_spec.lua | 53 +++++++- .../test/core/filesystem/MemoryMount.java | 11 +- 15 files changed, 330 insertions(+), 83 deletions(-) create mode 100644 projects/core/src/test/java/dan200/computercraft/core/filesystem/WritableMountContract.java 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 cleanup = new ArrayList<>(); @AfterEach @@ -33,43 +25,42 @@ public void cleanup() throws IOException { for (var mount : cleanup) MoreFiles.deleteRecursively(mount, RecursiveDeleteOption.ALLOW_INSECURE); } - private Path createRoot() throws IOException { + @Override + public MountAccess createMount(long capacity) throws IOException { var path = Files.createTempDirectory("cctweaked-test"); cleanup.add(path); - return path; + return new MountAccessImpl(path.resolve("mount"), capacity); } - private WritableMount getExisting(long capacity) throws IOException { - return new FileMount(createRoot().toFile(), capacity); - } + private static final class MountAccessImpl implements MountAccess { + private final Path root; + private final long capacity; + private final WritableMount mount; - private WritableMount getNotExisting(long capacity) throws IOException { - return new FileMount(createRoot().resolve("mount").toFile(), capacity); - } + private MountAccessImpl(Path root, long capacity) { + this.root = root; + this.capacity = capacity; + mount = new FileMount(root.toFile(), capacity); + } - @Test - public void testRootWritable() throws IOException { - assertFalse(getExisting(CAPACITY).isReadOnly("/")); - assertFalse(getNotExisting(CAPACITY).isReadOnly("/")); - } + @Override + public WritableMount mount() { + return mount; + } - @Test - public void testMissingDirWritable() throws IOException { - assertFalse(getExisting(CAPACITY).isReadOnly("/foo/bar/baz/qux")); - } + @Override + public void makeReadOnly(String path) { + Assumptions.assumeTrue(root.resolve(path).toFile().setReadOnly(), "Change file to read-only"); + } - @Test - public void testDirReadOnly() throws IOException { - var root = createRoot(); - var mount = new FileMount(root.toFile(), CAPACITY); - mount.makeDirectory("read-only"); + @Override + public void ensuresExist() throws IOException { + Files.createDirectories(root); + } - var attributes = Files.getFileAttributeView(root.resolve("read-only"), PosixFileAttributeView.class); - Assumptions.assumeTrue(attributes != null, "POSIX attributes are not available."); - - assertFalse(mount.isReadOnly("read-only"), "Directory should not be read-only yet"); - attributes.setPermissions(Set.of(PosixFilePermission.OWNER_READ, PosixFilePermission.OWNER_EXECUTE)); - assertTrue(mount.isReadOnly("read-only"), "Directory should not be read-only yet"); - assertTrue(mount.isReadOnly("read-only/child"), "Child should be read-only"); + @Override + public long computeRemainingSpace() { + return new FileMount(root.toFile(), capacity).getRemainingSpace(); + } } } diff --git a/projects/core/src/test/java/dan200/computercraft/core/filesystem/FileSystemTest.java b/projects/core/src/test/java/dan200/computercraft/core/filesystem/FileSystemTest.java index 16b292dec..ae96436e3 100644 --- a/projects/core/src/test/java/dan200/computercraft/core/filesystem/FileSystemTest.java +++ b/projects/core/src/test/java/dan200/computercraft/core/filesystem/FileSystemTest.java @@ -8,8 +8,8 @@ import com.google.common.io.Files; import dan200.computercraft.api.filesystem.WritableMount; import dan200.computercraft.api.lua.LuaException; +import dan200.computercraft.api.lua.ObjectArguments; import dan200.computercraft.core.TestFiles; -import dan200.computercraft.core.apis.ObjectWrapper; import dan200.computercraft.core.apis.handles.EncodedWritableHandle; import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; @@ -45,18 +45,18 @@ public void testWriteTruncates() throws FileSystemException, LuaException, IOExc { var writer = fs.openForWrite("out.txt", false, EncodedWritableHandle::openUtf8); - var wrapper = new ObjectWrapper(new EncodedWritableHandle(writer.get(), writer)); - wrapper.call("write", "This is a long line"); - wrapper.call("close"); + var handle = new EncodedWritableHandle(writer.get(), writer); + handle.write(new ObjectArguments("This is a long line")); + handle.doClose(); } assertEquals("This is a long line", Files.asCharSource(new File(ROOT, "out.txt"), StandardCharsets.UTF_8).read()); { var writer = fs.openForWrite("out.txt", false, EncodedWritableHandle::openUtf8); - var wrapper = new ObjectWrapper(new EncodedWritableHandle(writer.get(), writer)); - wrapper.call("write", "Tiny line"); - wrapper.call("close"); + var handle = new EncodedWritableHandle(writer.get(), writer); + handle.write(new ObjectArguments("Tiny line")); + handle.doClose(); } assertEquals("Tiny line", Files.asCharSource(new File(ROOT, "out.txt"), StandardCharsets.UTF_8).read()); @@ -69,11 +69,11 @@ public void testUnmountCloses() throws FileSystemException { fs.mountWritable("disk", "disk", mount); var writer = fs.openForWrite("disk/out.txt", false, EncodedWritableHandle::openUtf8); - var wrapper = new ObjectWrapper(new EncodedWritableHandle(writer.get(), writer)); + var handle = new EncodedWritableHandle(writer.get(), writer); fs.unmount("disk"); - var err = assertThrows(LuaException.class, () -> wrapper.call("write", "Tiny line")); + var err = assertThrows(LuaException.class, () -> handle.write(new ObjectArguments("Tiny line"))); assertEquals("attempt to use a closed file", err.getMessage()); } diff --git a/projects/core/src/test/java/dan200/computercraft/core/filesystem/WritableMountContract.java b/projects/core/src/test/java/dan200/computercraft/core/filesystem/WritableMountContract.java new file mode 100644 index 000000000..fcdcb506d --- /dev/null +++ b/projects/core/src/test/java/dan200/computercraft/core/filesystem/WritableMountContract.java @@ -0,0 +1,114 @@ +/* + * This file is part of ComputerCraft - http://www.computercraft.info + * Copyright Daniel Ratcliffe, 2011-2022. Do not distribute without permission. + * Send enquiries to dratcliffe@gmail.com + */ +package dan200.computercraft.core.filesystem; + +import dan200.computercraft.api.filesystem.WritableMount; +import org.junit.jupiter.api.Test; +import org.opentest4j.TestAbortedException; + +import java.io.IOException; + +import static org.junit.jupiter.api.Assertions.*; + +/** + * The contract that all {@link WritableMount}s must fulfill. + */ +public interface WritableMountContract { + long CAPACITY = 1_000_000; + + /** + * Create a new empty mount. + * + * @param capacity The capacity of this mount + * @return The newly created {@link WritableMount}. + * @throws IOException If the mount could not be created. + */ + MountAccess createMount(long capacity) throws IOException; + + /** + * Create a new empty mount, ensuring it exists on disk. + * + * @param capacity The capacity of this mount + * @return The newly created {@link WritableMount}. + * @throws IOException If the mount could not be created. + */ + default MountAccess createExisting(long capacity) throws IOException { + var mount = createMount(capacity); + mount.ensuresExist(); + return mount; + } + + @Test + default void testRootWritable() throws IOException { + assertFalse(createExisting(CAPACITY).mount().isReadOnly("/")); + assertFalse(createMount(CAPACITY).mount().isReadOnly("/")); + } + + @Test + default void testMissingDirWritable() throws IOException { + assertFalse(createExisting(CAPACITY).mount().isReadOnly("/foo/bar/baz/qux")); + } + + @Test + default void testDirReadOnly() throws IOException { + var root = createMount(CAPACITY); + var mount = root.mount(); + mount.makeDirectory("read-only"); + + assertFalse(mount.isReadOnly("read-only"), "Directory should not be read-only yet"); + root.makeReadOnly("read-only"); + assertTrue(mount.isReadOnly("read-only"), "Directory should not be read-only yet"); + assertTrue(mount.isReadOnly("read-only/child"), "Child should be read-only"); + } + + @Test + default void testMovePreservesSpace() throws IOException { + var access = createExisting(CAPACITY); + var mount = access.mount(); + mount.openForWrite("foo").close(); + + var remainingSpace = mount.getRemainingSpace(); + mount.rename("foo", "bar"); + + assertEquals(remainingSpace, mount.getRemainingSpace(), "Free space has changed after moving"); + assertEquals(access.computeRemainingSpace(), access.mount().getRemainingSpace(), "Free space is inconsistent"); + } + + /** + * Wraps a {@link WritableMount} with additional operations. + */ + interface MountAccess { + /** + * Get the underlying mount. + * + * @return The actual mount. + */ + WritableMount mount(); + + /** + * Make a path read-only. This may throw a {@link TestAbortedException} if + * + * @param path The mount-relative path. + */ + void makeReadOnly(String path) throws IOException; + + /** + * Ensures this mount exists. + */ + void ensuresExist() throws IOException; + + /** + * Get the remaining space for this mount. + *

+ * 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);