-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Mingw64 support #809
Mingw64 support #809
Changes from 26 commits
74be8e3
630fda8
bd7227f
87e0a65
132b824
d8e321b
fcf5033
da5fad2
dee5386
f54b952
232d378
0bdd1c2
530baa7
09c8471
d581c7a
f309738
a30a95a
8b77c51
26596b5
c4a9d6b
18dbd7a
9e371ae
07d2a54
9176a5e
fb4fd92
758712d
67b2d15
b202bd3
860fa22
56f61d5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,3 +16,4 @@ | |
package okio | ||
|
||
internal expect val PLATFORM_FILESYSTEM: Filesystem | ||
internal expect val PLATFORM_SEPARATOR: String |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ import okio.Path | |
import okio.Path.Companion.toPath | ||
import okio.buffer | ||
import kotlin.random.Random | ||
import kotlin.test.Ignore | ||
import kotlin.test.Test | ||
import kotlin.test.assertEquals | ||
import kotlin.test.assertFailsWith | ||
|
@@ -31,10 +32,12 @@ import kotlin.test.assertTrue | |
|
||
/** This test assumes that okio-files/ is the current working directory when executed. */ | ||
class FileSystemTest { | ||
private val tmpDirectory = Filesystem.SYSTEM.temporaryDirectory().toString() | ||
|
||
@Test | ||
fun baseDirectory() { | ||
val cwd = Filesystem.SYSTEM.baseDirectory() | ||
assertTrue(cwd.toString()) { cwd.toString().endsWith("okio/okio-files") } | ||
assertTrue(cwd.toString()) { cwd.toString().endsWith("okio${Path.directorySepartor}okio-files") } | ||
} | ||
|
||
@Test | ||
|
@@ -46,45 +49,45 @@ class FileSystemTest { | |
@Test | ||
fun `list no such directory`() { | ||
assertFailsWith<IOException> { | ||
Filesystem.SYSTEM.list("/tmp/unlikely-directory/ce70dc67c24823e695e616145ce38403".toPath()) | ||
Filesystem.SYSTEM.list("$tmpDirectory/unlikely-directory/ce70dc67c24823e695e616145ce38403".toPath()) | ||
} | ||
} | ||
|
||
@Test | ||
fun `file source no such directory`() { | ||
assertFailsWith<IOException> { | ||
Filesystem.SYSTEM.source("/tmp/unlikely-directory/ce70dc67c24823e695e616145ce38403".toPath()) | ||
Filesystem.SYSTEM.source("$tmpDirectory/unlikely-directory/ce70dc67c24823e695e616145ce38403".toPath()) | ||
} | ||
} | ||
|
||
@Test | ||
fun `file source`() { | ||
val source = Filesystem.SYSTEM.source("gradle.properties".toPath()) | ||
val buffer = Buffer() | ||
assertEquals(47L, source.read(buffer, 100L)) | ||
assertTrue(source.read(buffer, 100L) <= 49L) // either 47 on posix or 49 with \r\n line feeds on windows | ||
martinbonnin marked this conversation as resolved.
Show resolved
Hide resolved
|
||
assertEquals(-1L, source.read(buffer, 100L)) | ||
assertEquals(""" | ||
|POM_ARTIFACT_ID=okio-files | ||
|POM_NAME=Okio Files | ||
|""".trimMargin(), buffer.readUtf8()) | ||
|""".trimMargin(), buffer.readUtf8().replace("\r\n", "\n")) | ||
source.close() | ||
} | ||
|
||
@Test | ||
fun `file sink`() { | ||
val path = "/tmp/FileSystemTest-file_sink.txt".toPath() | ||
val path = "$tmpDirectory/FileSystemTest-file_sink.txt".toPath() | ||
val sink = Filesystem.SYSTEM.sink(path) | ||
val buffer = Buffer().writeUtf8("hello, world!") | ||
sink.write(buffer, buffer.size) | ||
sink.close() | ||
assertTrue(path in Filesystem.SYSTEM.list("/tmp".toPath())) | ||
assertTrue(path in Filesystem.SYSTEM.list(tmpDirectory.toPath())) | ||
assertEquals(0, buffer.size) | ||
assertEquals("hello, world!", path.readUtf8()) | ||
} | ||
|
||
@Test | ||
fun `file sink flush`() { | ||
val path = "/tmp/FileSystemTest-file_sink.txt".toPath() | ||
val path = "$tmpDirectory/FileSystemTest-file_sink.txt".toPath() | ||
val sink = Filesystem.SYSTEM.sink(path) | ||
|
||
val buffer = Buffer().writeUtf8("hello,") | ||
|
@@ -101,92 +104,93 @@ class FileSystemTest { | |
@Test | ||
fun `file sink no such directory`() { | ||
assertFailsWith<IOException> { | ||
Filesystem.SYSTEM.sink("/tmp/ce70dc67c24823e695e616145ce38403/unlikely-file".toPath()) | ||
Filesystem.SYSTEM.sink("$tmpDirectory/ce70dc67c24823e695e616145ce38403/unlikely-file".toPath()) | ||
} | ||
} | ||
|
||
@Test | ||
fun createDirectory() { | ||
val path = "/tmp/FileSystemTest-${randomToken()}".toPath() | ||
val path = "$tmpDirectory/FileSystemTest-${randomToken()}".toPath() | ||
Filesystem.SYSTEM.createDirectory(path) | ||
assertTrue(path in Filesystem.SYSTEM.list("/tmp".toPath())) | ||
assertTrue(path in Filesystem.SYSTEM.list(tmpDirectory.toPath())) | ||
} | ||
|
||
@Test | ||
fun `createDirectory parent directory does not exist`() { | ||
val path = "/tmp/ce70dc67c24823e695e616145ce38403-unlikely-file/created".toPath() | ||
val path = "$tmpDirectory/ce70dc67c24823e695e616145ce38403-unlikely-file/created".toPath() | ||
assertFailsWith<IOException> { | ||
Filesystem.SYSTEM.createDirectory(path) | ||
} | ||
} | ||
|
||
@Test | ||
fun `atomicMove file`() { | ||
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
source.writeUtf8("hello, world!") | ||
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
Filesystem.SYSTEM.atomicMove(source, target) | ||
assertEquals("hello, world!", target.readUtf8()) | ||
assertTrue(source !in Filesystem.SYSTEM.list("/tmp".toPath())) | ||
assertTrue(target in Filesystem.SYSTEM.list("/tmp".toPath())) | ||
assertTrue(source !in Filesystem.SYSTEM.list(tmpDirectory.toPath())) | ||
assertTrue(target in Filesystem.SYSTEM.list(tmpDirectory.toPath())) | ||
} | ||
|
||
@Test | ||
fun `atomicMove directory`() { | ||
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
Filesystem.SYSTEM.createDirectory(source) | ||
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
Filesystem.SYSTEM.atomicMove(source, target) | ||
assertTrue(source !in Filesystem.SYSTEM.list("/tmp".toPath())) | ||
assertTrue(target in Filesystem.SYSTEM.list("/tmp".toPath())) | ||
assertTrue(source !in Filesystem.SYSTEM.list(tmpDirectory.toPath())) | ||
assertTrue(target in Filesystem.SYSTEM.list(tmpDirectory.toPath())) | ||
} | ||
|
||
@Test | ||
fun `atomicMove source is target`() { | ||
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
source.writeUtf8("hello, world!") | ||
Filesystem.SYSTEM.atomicMove(source, source) | ||
assertEquals("hello, world!", source.readUtf8()) | ||
assertTrue(source in Filesystem.SYSTEM.list("/tmp".toPath())) | ||
assertTrue(source in Filesystem.SYSTEM.list(tmpDirectory.toPath())) | ||
} | ||
|
||
@Test | ||
fun `atomicMove clobber existing file`() { | ||
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
source.writeUtf8("hello, world!") | ||
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
target.writeUtf8("this file will be clobbered!") | ||
Filesystem.SYSTEM.atomicMove(source, target) | ||
assertEquals("hello, world!", target.readUtf8()) | ||
assertTrue(source !in Filesystem.SYSTEM.list("/tmp".toPath())) | ||
assertTrue(target in Filesystem.SYSTEM.list("/tmp".toPath())) | ||
assertTrue(source !in Filesystem.SYSTEM.list(tmpDirectory.toPath())) | ||
assertTrue(target in Filesystem.SYSTEM.list(tmpDirectory.toPath())) | ||
} | ||
|
||
@Test | ||
fun `atomicMove source does not exist`() { | ||
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
assertFailsWith<IOException> { | ||
Filesystem.SYSTEM.atomicMove(source, target) | ||
} | ||
} | ||
|
||
@Test | ||
fun `atomicMove source is file and target is directory`() { | ||
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
source.writeUtf8("hello, world!") | ||
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
Filesystem.SYSTEM.createDirectory(target) | ||
assertFailsWith<IOException> { | ||
Filesystem.SYSTEM.atomicMove(source, target) | ||
} | ||
} | ||
|
||
@Test | ||
@Ignore // somehow the behaviour is different on windows | ||
fun `atomicMove source is directory and target is file`() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There's something with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yep. Windows cannot do this atomically, so callers need to delete the target first. We need to fix the test to handle either behavior. |
||
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
Filesystem.SYSTEM.createDirectory(source) | ||
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
target.writeUtf8("hello, world!") | ||
assertFailsWith<IOException> { | ||
Filesystem.SYSTEM.atomicMove(source, target) | ||
|
@@ -195,62 +199,62 @@ class FileSystemTest { | |
|
||
@Test | ||
fun `copy file`() { | ||
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
source.writeUtf8("hello, world!") | ||
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
Filesystem.SYSTEM.copy(source, target) | ||
assertTrue(target in Filesystem.SYSTEM.list("/tmp".toPath())) | ||
assertTrue(target in Filesystem.SYSTEM.list(tmpDirectory.toPath())) | ||
assertEquals("hello, world!", target.readUtf8()) | ||
} | ||
|
||
@Test | ||
fun `copy source does not exist`() { | ||
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
assertFailsWith<IOException> { | ||
Filesystem.SYSTEM.copy(source, target) | ||
} | ||
assertFalse(target in Filesystem.SYSTEM.list("/tmp".toPath())) | ||
assertFalse(target in Filesystem.SYSTEM.list(tmpDirectory.toPath())) | ||
} | ||
|
||
@Test | ||
fun `copy target is clobbered`() { | ||
val source = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val source = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
source.writeUtf8("hello, world!") | ||
val target = "/tmp/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
val target = "$tmpDirectory/FileSystemTest-atomicMove-${randomToken()}".toPath() | ||
target.writeUtf8("this file will be clobbered!") | ||
Filesystem.SYSTEM.copy(source, target) | ||
assertTrue(target in Filesystem.SYSTEM.list("/tmp".toPath())) | ||
assertTrue(target in Filesystem.SYSTEM.list(tmpDirectory.toPath())) | ||
assertEquals("hello, world!", target.readUtf8()) | ||
} | ||
|
||
@Test | ||
fun `delete file`() { | ||
val path = "/tmp/FileSystemTest-delete-${randomToken()}".toPath() | ||
val path = "$tmpDirectory/FileSystemTest-delete-${randomToken()}".toPath() | ||
path.writeUtf8("delete me") | ||
Filesystem.SYSTEM.delete(path) | ||
assertTrue(path !in Filesystem.SYSTEM.list("/tmp".toPath())) | ||
assertTrue(path !in Filesystem.SYSTEM.list(tmpDirectory.toPath())) | ||
} | ||
|
||
@Test | ||
fun `delete empty directory`() { | ||
val path = "/tmp/FileSystemTest-delete-${randomToken()}".toPath() | ||
val path = "$tmpDirectory/FileSystemTest-delete-${randomToken()}".toPath() | ||
Filesystem.SYSTEM.createDirectory(path) | ||
Filesystem.SYSTEM.delete(path) | ||
assertTrue(path !in Filesystem.SYSTEM.list("/tmp".toPath())) | ||
assertTrue(path !in Filesystem.SYSTEM.list(tmpDirectory.toPath())) | ||
} | ||
|
||
@Test | ||
fun `delete fails on no such file`() { | ||
val path = "/tmp/FileSystemTest-delete-${randomToken()}".toPath() | ||
val path = "$tmpDirectory/FileSystemTest-delete-${randomToken()}".toPath() | ||
assertFailsWith<IOException> { | ||
Filesystem.SYSTEM.delete(path) | ||
} | ||
} | ||
|
||
@Test | ||
fun `delete fails on nonempty directory`() { | ||
val path = "/tmp/FileSystemTest-delete-${randomToken()}".toPath() | ||
val path = "$tmpDirectory/FileSystemTest-delete-${randomToken()}".toPath() | ||
Filesystem.SYSTEM.createDirectory(path) | ||
(path / "file.txt").writeUtf8("inside directory") | ||
assertFailsWith<IOException> { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't feel like it belongs here since it's not a property intrinsic to a file system. What's the temporary directory for a read-only zip file mounted as a filesystem? A temp dir is a property intrinsic to an operating system or platform.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like it's only used by tests? Maybe just an actual/expect in there to start with?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wanna use this to write multiplatform tests that use files. For example, tests of DiskLruCache.
A zip file is not an exemplar file system. For example, it doesn't support random access writing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(and if you are doing a Zip filesystem, you can just return Filesystem.SYSTEM.temporaryDirectory().)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That argument doesn't sway me. I think you've only radicalized me further. Temporary directories aren't an intrinsic property of a file system, they're a property of an operating system. At best it's a path resolvable by
FileSystem.SYSTEM
but it is not intrinsic to all file systems. It belongs elsewhere.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am still opposed. What do you do when it returns null?
What about a Samba or NFS filesystem mounted over the network? Or exposing S3 as a filesystem either through HTTP calls or the AWS S3 SDK? A zip file just an easy example to litmus test the API against that should quickly allow us to identify what is something primitive to a filesystem and what is not.
In your DiskLruCache example the test should have no behavioral difference whether given the system FS or an in-memory FS. The test doesn't get to ask for a temporary directory, it's given an FS and a base Path. If the test runner (or parameterized factory or whatever) wants to use the system FS and the OS temporary directory it can, but for an in-memory FS the temporary path can be the root dir.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@martinbonnin wanna pull this back from the public API? @JakeWharton and I will figure out the API in a follow-up.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure, let me try
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just made
temporaryDirectory()
internal. I think that's enough to remove it from the public API. Let me know if you want me to bury it a bit deeperThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍