-
-
Notifications
You must be signed in to change notification settings - Fork 45
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
Remove test utility ls
#469
Conversation
XCTAssertTrue(fs.isDirectory(bundleDirectory), "The Bundle directory should exist") | ||
XCTAssertTrue(bundleDirectory.ls().contains("index.html"), "Bundle does not have index.html") | ||
XCTAssertFalse( | ||
(bundleDirectory.ls().filter { $0.contains("wasm") }).isEmpty, |
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.
元コードですが、
どちらも「含む」ことをテストしているのに
片方は contains + assert true、
片方は filter + is empty + assert false
と、一貫性がありませんでした。
まとめて書き直しました。
let files = try fs.traverseRecursively(bundleDirectory) | ||
|
||
XCTAssertTrue( | ||
files.contains { $0.basename == "index.html" }, |
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.
こちらの API を使った方が、要素が AbsolutePath
で取れるので、
そのさきのフィルタ処理を適切に書けます。
XCTFail("No wasm binary found") | ||
return 0 | ||
} | ||
|
||
return try localFileSystem.getFileInfo(bundleDirectory.appending(component: wasmFile)).size | ||
return try localFileSystem.getFileInfo(wasmFile).size |
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.
すでに AbsolutePath
なのでパスの組み立て処理が不要です。
起動まちが3秒じゃ足りないか? |
それの修正はまた別でやろう |
AbsolutePath.ls
をしなくても、FileSystem.getDirectoryContents
やFileSystem.traverseRecursively
があります。テスト用のやり方を用意してもコード短縮の恩恵は少ないです。
また、
ls
という名前なのに再帰的に中身を取り出す挙動は予測しづらいです。必要ない道具は消したほうが、コードベースを把握しやすくなります。
また、
ls
はString
を返しますが、ここではファイルパスであることが確定しているので、
AbsolutePath
を返すAPIの方が、その先のデータ操作で様々なメソッドが使えて便利です。
使わなくても十分に整ったテストが書けるので消します。
#468 でもこの件に少し触れています。