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.

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

Generalize findSwiftExecutable into findExecutable #436

Merged
merged 2 commits into from
May 17, 2024

Conversation

omochi
Copy link
Contributor

@omochi omochi commented May 17, 2024

課題

テストでは findSwiftExecutable という関数が便利に使われていますが、
swift コマンド以外のコマンドを探すことができません。
#434 のように他のコマンドも探せると便利です。

提案

findExecutable を実装します。

見つからなかった場合には以下のようにエラーメッセージを出します。

/Users/omochi/github/swiftwasm/carton/Tests/CartonCommandTests/CommandTestHelper.swift:38: error: -[CartonCommandTests.TestCommandTests testWithNoArguments] : failed: caught error: "Executable foobar was not found: status=1"

背景

#434 で curl を使おうとしたらどうやら見つからなかったのですが、
findExecutable の実装上エラーログが貧弱で対処しづらそうでした。

#434 の変更をなるべく小さくするため、
この改善だけ単品で切り出したパッチを作りました。

return try AbsolutePath(validating: path)
}

func findSwiftExecutable() throws -> AbsolutePath {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

String ではなく AbsolutePath を返すように変更しています。
その方がさまざまな CartonHelper の他のコードと一緒に使いやすいからです。

@kateinoigakukun
Copy link
Member

CI failing

<EXPR>:0: error: TestCommandTests.testSkipBuild : threw error "Output from /usr/bin/which is empty"

@omochi
Copy link
Contributor Author

omochi commented May 17, 2024

2重否定間違えるやつ やらかした

    guard path.isEmpty else {
       throw CommandTestError("Output from \(whichBin) is empty")
     }

@omochi
Copy link
Contributor Author

omochi commented May 17, 2024

@kateinoigakukun CIが治ったので改めて確認お願いします

@kateinoigakukun kateinoigakukun added the refactor No user-visible functionality change label May 17, 2024
@kateinoigakukun kateinoigakukun merged commit c7cad49 into swiftwasm:main May 17, 2024
3 of 4 checks passed
@kateinoigakukun kateinoigakukun changed the title テストで使われている findSwiftExecutable を一般化して findExecutable を実装する Generalize findSwiftExecutable into findExecutable May 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactor No user-visible functionality change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants