From 0bd2a957bb758569cca6311cb1a1fe1fb2ae8683 Mon Sep 17 00:00:00 2001 From: Koichi Yokota Date: Thu, 24 Feb 2022 21:49:47 +0900 Subject: [PATCH 1/2] Add error descriptions to `FileError` --- CHANGELOG.md | 1 + Sources/Puppy/FileError.swift | 25 ++++-- Sources/Puppy/FileLogger.swift | 10 +-- Tests/PuppyTests/FileLoggerTests.swift | 117 ++++++++++++++----------- 4 files changed, 90 insertions(+), 63 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0814eac..f3fff54 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,7 @@ - Change the type of intPermission to `UInt16`. [#41](https://github.com/sushichop/Puppy/pull/41) - Output a message prompting to override. [#42](https://github.com/sushichop/Puppy/pull/42) - Use String type for filePermission. [#43](https://github.com/sushichop/Puppy/pull/43) +- Add error descriptions to `FileError`. [#44](https://github.com/sushichop/Puppy/pull/44) ## [0.4.0](https://github.com/sushichop/Puppy/releases/tag/0.4.0) (2022-01-31) diff --git a/Sources/Puppy/FileError.swift b/Sources/Puppy/FileError.swift index 39105d8..6b59f18 100644 --- a/Sources/Puppy/FileError.swift +++ b/Sources/Puppy/FileError.swift @@ -1,20 +1,27 @@ import Foundation -public enum FileError: Error, Equatable { +public enum FileError: Error, Equatable, LocalizedError { case isNotFile(url: URL) + case invalidPermission(at: URL, filePermission: String) case creatingDirectoryFailed(at: URL) case creatingFileFailed(at: URL) - case writingFailed(at: URL) - case invalidPermission(at: URL, filePermission: String) -} - -public enum FileDeletingError: Error, Equatable, LocalizedError { - case failed(at: URL) + case openingForWritingFailed(at: URL) + case deletingFailed(at: URL) public var errorDescription: String? { switch self { - case .failed(at: let url): - return "failed to delete the file: \(url)" + case .isNotFile(url: let url): + return "\(url) is not a file" + case .invalidPermission(at: let url, filePermission: let filePermission): + return "invalid file permission. file: \(url), permission: \(filePermission)" + case .creatingDirectoryFailed(at: let url): + return "failed to create a directory: \(url)" + case .creatingFileFailed(at: let url): + return "failed to create a file: \(url)" + case .openingForWritingFailed(at: let url): + return "failed to open a file for writing: \(url)" + case .deletingFailed(at: let url): + return "failed to delete a file: \(url)" } } } diff --git a/Sources/Puppy/FileLogger.swift b/Sources/Puppy/FileLogger.swift index 5ff5fd8..df48e7d 100644 --- a/Sources/Puppy/FileLogger.swift +++ b/Sources/Puppy/FileLogger.swift @@ -47,22 +47,22 @@ public class FileLogger: BaseLogger { } } - public func delete(_ url: URL) -> Result { + public func delete(_ url: URL) -> Result { queue!.sync { Result { try FileManager.default.removeItem(at: url) } .map { url } .mapError { _ in - FileDeletingError.failed(at: url) + FileError.deletingFailed(at: url) } } } - public func delete(_ url: URL, completion: @escaping (Result) -> Void) { + public func delete(_ url: URL, completion: @escaping (Result) -> Void) { queue!.async { let result = Result { try FileManager.default.removeItem(at: url) } .map { url } .mapError { _ in - FileDeletingError.failed(at: url) + FileError.deletingFailed(at: url) } completion(result) } @@ -105,7 +105,7 @@ public class FileLogger: BaseLogger { do { fileHandle = try FileHandle(forWritingTo: fileURL) } catch { - throw FileError.writingFailed(at: fileURL) + throw FileError.openingForWritingFailed(at: fileURL) } } } diff --git a/Tests/PuppyTests/FileLoggerTests.swift b/Tests/PuppyTests/FileLoggerTests.swift index e3dc160..3fb27be 100644 --- a/Tests/PuppyTests/FileLoggerTests.swift +++ b/Tests/PuppyTests/FileLoggerTests.swift @@ -49,11 +49,67 @@ final class FileLoggerTests: XCTestCase { let emptyFileURL = URL(fileURLWithPath: "").absoluteURL // file:///private/tmp/ XCTAssertThrowsError(try FileLogger("com.example.yourapp.filelogger.notfile0", fileURL: emptyFileURL)) { error in XCTAssertEqual(error as? FileError, .isNotFile(url: emptyFileURL)) + XCTAssertEqual(error.localizedDescription, "\(emptyFileURL) is not a file") } let directoryURL = URL(fileURLWithPath: "./").absoluteURL // file:///private/tmp/ XCTAssertThrowsError(try FileLogger("com.example.yourapp.filelogger.notfile1", fileURL: directoryURL)) { error in XCTAssertEqual(error as? FileError, .isNotFile(url: directoryURL)) + XCTAssertEqual(error.localizedDescription, "\(directoryURL) is not a file") + } + } + + func testFilePermission() throws { + let fileURL = URL(fileURLWithPath: "./permission600.log").absoluteURL + let fileLogger = try FileLogger("com.example.yourapp.filelogger.permission600", fileURL: fileURL, filePermission: "600") + let log = Puppy() + log.add(fileLogger) + log.trace("permission, TRACE message using FileLogger") + log.verbose("permission, VERBOSE message using FileLogger") + fileLogger.flush() + + let attribute = try FileManager.default.attributesOfItem(atPath: fileURL.path) + // swiftlint:disable force_cast + let permission = attribute[FileAttributeKey.posixPermissions] as! UInt16 + // swiftlint:enable force_cast + + #if os(Windows) + // NOTE: If the file is writable, its permission is always "700" on Windows. + // Reference: https://github.com/apple/swift-corelibs-foundation/blob/main/Sources/Foundation/FileManager%2BWin32.swift + let expectedPermission = UInt16("700", radix: 8)! + #else + let expectedPermission = UInt16("600", radix: 8)! + #endif // os(Windows) + XCTAssertEqual(permission, expectedPermission) + + _ = fileLogger.delete(fileURL) + log.remove(fileLogger) + } + + func testFilePermissionError() throws { + let permission800FileURL = URL(fileURLWithPath: "./permission800.log").absoluteURL + let filePermission800 = "800" + XCTAssertThrowsError(try FileLogger("com.example.yourapp.filelogger.permission800", fileURL: permission800FileURL, filePermission: filePermission800)) { error in + XCTAssertEqual(error as? FileError, .invalidPermission(at: permission800FileURL, filePermission: filePermission800)) + XCTAssertEqual(error.localizedDescription, "invalid file permission. file: \(permission800FileURL), permission: \(filePermission800)") + } + + let permissionABCFileURL = URL(fileURLWithPath: "./permissionABC.log").absoluteURL + let filePermissionABC = "ABC" + XCTAssertThrowsError(try FileLogger("com.example.yourapp.filelogger.permissionABC", fileURL: permissionABCFileURL, filePermission: filePermissionABC)) { error in + XCTAssertEqual(error as? FileError, .invalidPermission(at: permissionABCFileURL, filePermission: filePermissionABC)) + XCTAssertEqual(error.localizedDescription, "invalid file permission. file: \(permissionABCFileURL), permission: \(filePermissionABC)") + } + } + + func testWritingError() throws { + let fileURL = URL(fileURLWithPath: "./readonly.log").absoluteURL + XCTAssertThrowsError(try FileLogger("com.example.yourapp.filelogger.readonly", fileURL: fileURL, filePermission: "400")) { error in + XCTAssertEqual(error as? FileError, .openingForWritingFailed(at: fileURL)) + XCTAssertEqual(error.localizedDescription, "failed to open a file for writing: \(fileURL)") + // swiftlint:disable force_try + try! FileManager.default.removeItem(at: fileURL) + // swiftlint:enable force_try } } @@ -64,12 +120,14 @@ final class FileLoggerTests: XCTestCase { XCTAssertThrowsError(try FileLogger("com.example.yourapp.filelogger.notcreatedirectory", fileURL: fileURLNotAbleToCreateDirectory)) { error in XCTAssertEqual(error as? FileError, .creatingDirectoryFailed(at: directoryURLNotAbleToCreateDirectory)) + XCTAssertEqual(error.localizedDescription, "failed to create a directory: \(directoryURLNotAbleToCreateDirectory)") } let fileURLNotAbleToCreateFile = URL(fileURLWithPath: "/foo.log").absoluteURL // file:///foo.log XCTAssertThrowsError(try FileLogger("com.example.yourapp.filelogger.notcreatefile", fileURL: fileURLNotAbleToCreateFile)) { error in XCTAssertEqual(error as? FileError, .creatingFileFailed(at: fileURLNotAbleToCreateFile)) + XCTAssertEqual(error.localizedDescription, "failed to create a file: \(fileURLNotAbleToCreateFile)") } } #endif // canImport(Darwin) @@ -84,7 +142,7 @@ final class FileLoggerTests: XCTestCase { case .success(let url): XCTAssertEqual(existentFileURL, url) case .failure: - XCTFail("shuould not be failed, but was failed") + XCTFail("should not be failed, but was failed") } let resultFailure = fileLogger.delete(noExistentFileURL) @@ -92,8 +150,8 @@ final class FileLoggerTests: XCTestCase { case .success: XCTFail("should not be successful, but was successful") case .failure(let error): - XCTAssertEqual(error as FileDeletingError, .failed(at: noExistentFileURL)) - XCTAssertEqual(error.localizedDescription, "failed to delete the file: \(noExistentFileURL)") + XCTAssertEqual(error as FileError, .deletingFailed(at: noExistentFileURL)) + XCTAssertEqual(error.localizedDescription, "failed to delete a file: \(noExistentFileURL)") } } @@ -109,7 +167,7 @@ final class FileLoggerTests: XCTestCase { XCTAssertEqual(existentFileURL, url) expSuccess.fulfill() case .failure: - XCTFail("shuould not be failed, but was failed") + XCTFail("should not be failed, but was failed") } } @@ -119,8 +177,8 @@ final class FileLoggerTests: XCTestCase { case .success: XCTFail("should not be successful, but was successful") case .failure(let error): - XCTAssertEqual(error as FileDeletingError, .failed(at: noExistentFileURL)) - XCTAssertEqual(error.localizedDescription, "failed to delete the file: \(noExistentFileURL)") + XCTAssertEqual(error as FileError, .deletingFailed(at: noExistentFileURL)) + XCTAssertEqual(error.localizedDescription, "failed to delete a file: \(noExistentFileURL)") expFailure.fulfill() } } @@ -144,8 +202,8 @@ final class FileLoggerTests: XCTestCase { _ = try fileLogger.delete(noExistentFileURL).get() XCTFail("error should be thrown while awaiting, but it was not thrown") } catch { - XCTAssertEqual(error as? FileDeletingError, .failed(at: noExistentFileURL)) - XCTAssertEqual(error.localizedDescription, "failed to delete the file: \(noExistentFileURL)") + XCTAssertEqual(error as? FileError, .deletingFailed(at: noExistentFileURL)) + XCTAssertEqual(error.localizedDescription, "failed to delete a file: \(noExistentFileURL)") } } @@ -171,8 +229,8 @@ final class FileLoggerTests: XCTestCase { _ = try result.get() XCTFail("error should be thrown while awaiting, but it was not thrown") } catch { - XCTAssertEqual(error as? FileDeletingError, .failed(at: noExistentFileURL)) - XCTAssertEqual(error.localizedDescription, "failed to delete the file: \(noExistentFileURL)") + XCTAssertEqual(error as? FileError, .deletingFailed(at: noExistentFileURL)) + XCTAssertEqual(error.localizedDescription, "failed to delete a file: \(noExistentFileURL)") expFailure.fulfill() } } @@ -213,43 +271,4 @@ final class FileLoggerTests: XCTestCase { wait(for: [exp], timeout: 5.0) } - - func testFilePermission() throws { - let fileURL = URL(fileURLWithPath: "./permission600.log").absoluteURL - let fileLogger = try FileLogger("com.example.yourapp.filelogger.permission600", fileURL: fileURL, filePermission: "600") - let log = Puppy() - log.add(fileLogger) - log.trace("permission, TRACE message using FileLogger") - log.verbose("permission, VERBOSE message using FileLogger") - fileLogger.flush() - - let attribute = try FileManager.default.attributesOfItem(atPath: fileURL.path) - // swiftlint:disable force_cast - let permission = attribute[FileAttributeKey.posixPermissions] as! UInt16 - // swiftlint:enable force_cast - - #if os(Windows) - // NOTE: If the file is writable, its permission is always "700" on Windows. - // Reference: https://github.com/apple/swift-corelibs-foundation/blob/main/Sources/Foundation/FileManager%2BWin32.swift - let expectedPermission = UInt16("700", radix: 8)! - #else - let expectedPermission = UInt16("600", radix: 8)! - #endif // os(Windows) - XCTAssertEqual(permission, expectedPermission) - - _ = fileLogger.delete(fileURL) - log.remove(fileLogger) - } - - func testFilePermissionError() throws { - let permission800FileURL = URL(fileURLWithPath: "./permission800.log").absoluteURL - XCTAssertThrowsError(try FileLogger("com.example.yourapp.filelogger.permission800", fileURL: permission800FileURL, filePermission: "800")) { error in - XCTAssertEqual(error as? FileError, .invalidPermission(at: permission800FileURL, filePermission: "800")) - } - - let permissionABCFileURL = URL(fileURLWithPath: "./permissionABC.log").absoluteURL - XCTAssertThrowsError(try FileLogger("com.example.yourapp.filelogger.permissionABC", fileURL: permissionABCFileURL, filePermission: "ABC")) { error in - XCTAssertEqual(error as? FileError, .invalidPermission(at: permissionABCFileURL, filePermission: "ABC")) - } - } } From 0463cb8718a41ca9c3ddee12624c4f08d8c4e2c5 Mon Sep 17 00:00:00 2001 From: Koichi Yokota Date: Thu, 24 Feb 2022 23:33:05 +0900 Subject: [PATCH 2/2] Tests only for Darwin --- Tests/PuppyTests/FileLoggerTests.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Tests/PuppyTests/FileLoggerTests.swift b/Tests/PuppyTests/FileLoggerTests.swift index 3fb27be..615b76f 100644 --- a/Tests/PuppyTests/FileLoggerTests.swift +++ b/Tests/PuppyTests/FileLoggerTests.swift @@ -102,6 +102,7 @@ final class FileLoggerTests: XCTestCase { } } + #if canImport(Darwin) func testWritingError() throws { let fileURL = URL(fileURLWithPath: "./readonly.log").absoluteURL XCTAssertThrowsError(try FileLogger("com.example.yourapp.filelogger.readonly", fileURL: fileURL, filePermission: "400")) { error in @@ -112,6 +113,7 @@ final class FileLoggerTests: XCTestCase { // swiftlint:enable force_try } } + #endif // canImport(Darwin) #if canImport(Darwin) func testCreatingError() throws {