Skip to content
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.

Sign up for GitHub

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

Jump to bottom

ioJS: handle NoSuchFileException in isDirectory, isFile and isSymbolicLink #2727

Merged
merged 3 commits into from
Nov 30, 2021
Merged

ioJS: handle NoSuchFileException in isDirectory, isFile and isSymbolicLink #2727

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
109a71f
ioJS: handle NoSuchFileException in isDirectory, isFile and isSymboli…
Nov 28, 2021
712b266
true-case test for isSymbolicLink
Nov 30, 2021
28ab8ce
true-case test for isHidden, fix and make isHidden consistent
Nov 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 5 additions & 8 deletions io/js/src/main/scala/fs2/io/file/FilesPlatform.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,27 +217,24 @@ private[fs2] trait FilesCompanionPlatform {
getPosixFileAttributes(path, followLinks).map(_.permissions)

override def isDirectory(path: Path, followLinks: Boolean): F[Boolean] =
stat(path, followLinks).map(_.isDirectory())
stat(path, followLinks).map(_.isDirectory()).recover { case _: NoSuchFileException => false }

override def isExecutable(path: Path): F[Boolean] =
access(path, fsMod.constants.X_OK)

private val HiddenPattern = raw"/(^|\/)\.[^\/\.]/g".r
override def isHidden(path: Path): F[Boolean] = F.pure {
path.toString match {
case HiddenPattern() => true
case _ => false
}
val fileName = path.fileName.toString
fileName.length >= 2 && fileName(0) == '.' && fileName(1) != '.'
}

override def isReadable(path: Path): F[Boolean] =
access(path, fsMod.constants.R_OK)

override def isRegularFile(path: Path, followLinks: Boolean): F[Boolean] =
stat(path, followLinks).map(_.isFile())
stat(path, followLinks).map(_.isFile()).recover { case _: NoSuchFileException => false }

override def isSymbolicLink(path: Path): F[Boolean] =
stat(path).map(_.isSymbolicLink())
stat(path).map(_.isSymbolicLink()).recover { case _: NoSuchFileException => false }

override def isWritable(path: Path): F[Boolean] =
access(path, fsMod.constants.W_OK)
Expand Down
153 changes: 152 additions & 1 deletion io/shared/src/test/scala/fs2/io/file/FilesSuite.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ package fs2
package io
package file

import cats.effect.IO
import cats.effect.{IO, Resource}
import cats.kernel.Order
import cats.syntax.all._

Expand Down Expand Up @@ -541,6 +541,13 @@ class FilesSuite extends Fs2Suite with BaseFileSuite {
.use(Files[IO].isDirectory(_))
.assertEquals(true)
}

test("returns false if the path does not exist") {
tempDirectory
.map(_.resolve("non-existent-file"))
.use(Files[IO].isDirectory(_))
.assertEquals(false)
}
}

group("isFile") {
Expand All @@ -555,5 +562,149 @@ class FilesSuite extends Fs2Suite with BaseFileSuite {
.use(Files[IO].isRegularFile(_))
.assertEquals(false)
}

test("returns false if the path does not exist") {
tempDirectory
.map(_.resolve("non-existent-file"))
.use(Files[IO].isRegularFile(_))
.assertEquals(false)
}
}

group("isReadable") {
test("returns true if the path is for a readable file") {
tempFile
.use(Files[IO].isReadable(_))
.assertEquals(true)
}

test("returns true if the path is for a directory") {
tempDirectory
.use(Files[IO].isReadable(_))
.assertEquals(true)
}

test("returns false if the path does not exist") {
tempDirectory
.map(_.resolve("non-existent-file"))
.use(Files[IO].isReadable(_))
.assertEquals(false)
}
}

group("isWritable") {
test("returns true if the path is for a readable file") {
tempFile
.use(Files[IO].isWritable(_))
.assertEquals(true)
}

test("returns true if the path is for a directory") {
tempDirectory
.use(Files[IO].isWritable(_))
.assertEquals(true)
}

test("returns false if the path does not exist") {
tempDirectory
.map(_.resolve("non-existent-file"))
.use(Files[IO].isWritable(_))
.assertEquals(false)
}
}

group("isHidden") {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even though there's no windows CI, this will be annoying for anyone developing on Windows. Not sure how big a deal that is.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah 🤔
If someone's working on fs2 in Windows and runs the tests.. that will be annoying (I kind of blanked out such a possibility :/ )
I wonder if we can do some platform checks in the test code and, at least, always pass isHidden true-case if it's windows?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, it's merged. Let's see if anyone complains :P

The problem is we need cross-platform (JVM/JS) checks for checking the platform (Unix/Windows). Possible, but annoying!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eh, it's merged. Let's see if anyone complains :P

=)

The problem is we need cross-platform (JVM/JS) checks for checking the platform (Unix/Windows). Possible, but annoying!

Should be non-crazy enough to implement, right?

Is there a good place for it? (like Files[F] is for the file stuff)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, as a public API? Probably not in fs2. I was thinking as an internal/test utility.


test("returns false if the path is for a non-hidden file") {
tempFile
.use(Files[IO].isHidden(_))
.assertEquals(false)
}

test("returns false if the path is for a non-hidden directory") {
tempDirectory
.use(Files[IO].isHidden(_))
.assertEquals(false)
}

test("returns true if the path is for a hidden file") {
val hiddenFile = for {
dir <- tempDirectory
hiddenFilePath = dir.resolve(".should-be-hidden")
_ <- Resource.make(Files[IO].createFile(hiddenFilePath)) { _ =>
Files[IO].deleteIfExists(hiddenFilePath).void
}
} yield hiddenFilePath
hiddenFile
.use(Files[IO].isHidden(_))
.assertEquals(true)
}

test("returns false if the path is for a non-hidden file in a hidden directory") {
val fileInHiddenDir = for {
dir <- tempDirectory
hiddenDirPath = dir.resolve(".should-be-hidden")
_ <- Resource.make(Files[IO].createDirectory(hiddenDirPath)) { _ =>
Files[IO].deleteIfExists(hiddenDirPath).void
}
filePath = hiddenDirPath.resolve("not-hidden")
_ <- Resource.make(Files[IO].createFile(filePath)) { _ =>
Files[IO].deleteIfExists(filePath).void
}
} yield filePath
fileInHiddenDir
.use(Files[IO].isHidden(_))
.assertEquals(false)
}

test("returns true if the path is for a hidden directory") {
val hiddenDir = for {
dir <- tempDirectory
hiddenDirPath = dir.resolve(".should-be-hidden")
_ <- Resource.make(Files[IO].createDirectory(hiddenDirPath)) { _ =>
Files[IO].deleteIfExists(hiddenDirPath).void
}
} yield hiddenDirPath
hiddenDir
.use(Files[IO].isHidden(_))
.assertEquals(true)
}

test("returns false if the path does not exist") {
tempDirectory
.map(_.resolve("non-existent-file"))
.use(Files[IO].isWritable(_))
.assertEquals(false)
}
}

group("isSymbolicLink") {

test("returns true if the path is for a symbolic link") {
val symLink = for {
dir <- tempDirectory
file <- tempFile
symLinkPath = dir.resolve("temp-sym-link")
_ <- Resource.make(Files[IO].createSymbolicLink(symLinkPath, file)) { _ =>
Files[IO].deleteIfExists(symLinkPath).void
}
} yield symLinkPath
symLink
.use(Files[IO].isSymbolicLink(_))
.assertEquals(true)
}

test("returns false if the path is for a directory") {
tempDirectory
.use(Files[IO].isSymbolicLink(_))
.assertEquals(false)
}

test("returns false if the path does not exist") {
tempDirectory
.map(_.resolve("non-existent-file"))
.use(Files[IO].isSymbolicLink(_))
.assertEquals(false)
}
}
}