uber / mockolo

Efficient Mock Generator for Swift
Apache License 2.0
804 stars 85 forks source link

Support Actor Protocol #216

Open tsuzukihashi opened 1 year ago

tsuzukihashi commented 1 year ago

Could you support to actor protocol? I would like to automatically generate an "Actor" compliant protocol as follows.

/// @mockable
protocol Hoge: Actor {
  func get(hoge: String) async -> Result<String, Error>
  var count: Int { get }
}

↓ Generated

// NOTE: change actor
actor HogeMock: Hoge {
  init() {}
  init(count: Int = 0) {
    self.count = count
  }

  private(set) var getCallCount = 0
  var getArgValues = [String]()
  // NOTE: add @MainActor
  @MainActor
  var getHandler: ((String) async -> (Result<String, Error>))?
  func get(hoge: String) async -> Result<String, Error> {
    getCallCount += 1
    getArgValues.append(hoge)
    // NOTE: add await
    if let getHandler = await getHandler {
      return await getHandler(hoge)
    }
    fatalError("getHandler returns can't have a default value thus its handler must be set")
  }

  private(set) var countSetCallCount = 0
  var count: Int = 0 { didSet { countSetCallCount += 1 } }
  // NOTE: add set func
  func setCount(_ value: Int) {
    self.count = value
  }
}

Thank you.

uhooi commented 1 year ago

Thanks for the Issue! I'm waiting for the PR🥺

treastrain commented 1 year ago

I hope this feature exists in this OSS! I see three ways to support this:

  1. Add an annotation specifying the object nominal types of the mock
  2. Mocks for that protocol that conform to the Actor automatically become actor
  3. Mocks for that protocol that conform to the another protocol conforming Actor automatically become actor
// 1.
/// @mockable(object: actor)
protocol Hoge: Actor {
    var count: Int { get }
}

///
/// @Generated by Mockolo
///
actor HogeMock: Hoge {
    init() { }
    init(count: Int = 0) {
        self.count = count
    }

    private(set) var countSetCallCount = 0
    var count: Int = 0 { didSet { countSetCallCount += 1 } }
}
// 2.
/// @mockable
protocol Hoge: Actor {
    var count: Int { get }
}

///
/// @Generated by Mockolo
///
actor HogeMock: Hoge {
    init() { }
    init(count: Int = 0) {
        self.count = count
    }

    private(set) var countSetCallCount = 0
    var count: Int = 0 { didSet { countSetCallCount += 1 } }
}
// 3.
/// @mockable
protocol Hoge: Fuga {
    var count: Int { get }
}
protocol Fuga: Actor {
}

///
/// @Generated by Mockolo
///
actor HogeMock: Hoge {
    init() { }
    init(count: Int = 0) {
        self.count = count
    }

    private(set) var countSetCallCount = 0
    var count: Int = 0 { didSet { countSetCallCount += 1 } }
}

I implemented the changes that fulfill "1." and "2." above in my forked branch: https://github.com/uber/mockolo/compare/master...treastrain:c20f64b140c78b9f05202e661a07c43c15834358 (No test has been added to this yet now.)

However, I have yet to come up with a way to achieve "3." Any suggestions?

uhooi commented 1 year ago

@treastrain Thanks for the suggestion!

I agree with "2." since only actor can conform to the Actor protocol. Couldn't "3." be achieved using SwiftSyntax and the is operator? "1." may cause mock build errors when marking protocols that are not Actor. It seems like it would be better to support only "2." (and "3." if possible) first, what do you think?

Japanese 提案ありがとうございます! `Actor` プロトコルに準拠できるのは `actor` しかないので、"2." に賛成です。 "3." はSwiftSyntaxや `is` 演算子を使って実現できないでしょうか? "1." は `Actor` でないプロトコルにマークした場合にモックがビルドエラーになることがあります。 まずは "2." のみ(できたら "3." も)対応するのがよさそうですがどうでしょうか?
treastrain commented 1 year ago

@uhooi

Thanks for your response!

The diffs shown in https://github.com/uber/mockolo/compare/master...treastrain:c20f64b140c78b9f05202e661a07c43c15834358 are a mixture of "1." and "2." but the implementation effort is light < "1." < "2." < heavy.

I agree with "2." since only actor can conform to the Actor protocol.

I see. I will create a pull request for this part in my spare time.

"1." may cause mock build errors when marking protocols that are not Actor.

The same could be said here for the already existing annotations typealias, rx, combine, etc. It is the responsibility of the person making this statement, and we do not believe that we should be concerned about mock build errors when marking protocols that are not Actor.

Couldn't "3." be achieved using SwiftSyntax and the is operator?

My knowledge of SwiftSyntax is lacking, so a solution using it is not immediately obvious. But as for the is operator, I understand that when this library generates a mock, it does not build and import the source code for the mock generation target, so it can't handle that object directly, so I don't think I can solve it that way.

Japanese ありがとうございます! https://github.com/uber/mockolo/compare/master...treastrain:c20f64b140c78b9f05202e661a07c43c15834358 で示した差分は "1." と "2." が混ざったものになっていますが、実装工数は 軽 < "1." < "2." < 重 になっています。 > `Actor` プロトコルに準拠できるのは `actor` しかないので、"2." に賛成です。 わかりました。私の余暇の時間でここの部分の pull request を作成してみたいと思います。 > "1." は `Actor` でないプロトコルにマークした場合にモックがビルドエラーになることがあります。 こちらは、すでに存在するアノテーションである `typealias` や `rx`、`combine ` などでも同じことが言えるかと思います。この記述をする人が責任を持つべき事項であり、「`Actor` でないプロトコルにマークした場合にモックがビルドエラーになること」をこちら側が心配することではないと思っています。 > "3." はSwiftSyntaxや `is` 演算子を使って実現できないでしょうか? 私の SwiftSyntax に対する知識が足りないため、それを用いた解決方法はすぐには分かりません。 しかし、`is` 演算子の方については、このライブラリがモックを生成する際に、モック生成対象のソースコードをビルドして import しているわけではないと理解しており、そのオブジェクトを直接取り扱えるわけではないため、その方法ではそれを解決できないと考えます。
uhooi commented 1 year ago

@treastrain

I agree with "2." since only actor can conform to the Actor protocol.

I see. I will create a pull request for this part in my spare time.

Thanks, waiting for PR :pray:

"1." may cause mock build errors when marking protocols that are not Actor.

The same could be said here for the already existing annotations typealias, rx, combine, etc. It is the responsibility of the person making this statement, and we do not believe that we should be concerned about mock build errors when marking protocols that are not Actor.

Since you are right, it seems that if "3." cannot be realized by any means, "1." may be adopted. As for how to realize "3.", the is operator is just an example, and I also thought it could not be solved :sob:

Japanese > > `Actor` プロトコルに準拠できるのは `actor` しかないので、"2." に賛成です。 > > わかりました。私の余暇の時間でここの部分の pull request を作成してみたいと思います。 ありがとうございます。PR お待ちしております :pray: > > "1." は `Actor` でないプロトコルにマークした場合にモックがビルドエラーになることがあります。 > > こちらは、すでに存在するアノテーションである `typealias` や `rx`、`combine ` などでも同じことが言えるかと思います。> この記述をする人が責任を持つべき事項であり、「`Actor` でないプロトコルにマークした場合にモックがビルドエラーになること」をこちら側が心配することではないと思っています。 仰る通りなので、どうしても "3." が実現できない場合は "1." を採用してよさそうです。 "3." の実現方法について、 `is` 演算子は例として挙げただけであり、私も解決できないと思っていました :sob:
fummicc1 commented 1 year ago

Thank you! let me say one opinion from my side. :pray: I prefer option 1 ~because we can't take care of @globalActor if we adopt option 2. (if it is not correct please teach me.)~ Besides that, option 1 is more flexible and independent on certain swift future change.

oh I am sorry. I found globalActor protocol should be mocked with class.

Besides that, option 1 is more flexible and independent on certain swift future change.

one of example is typealias and AnyActor, DistributedActor.