google/truth

Document how to write a Truth extension under Kotlin

cpovirk opened this issue · 8 comments

cpovirk commented

It's pretty simple (and pretty similar to Java), but it's always nice for users to have an example.

[edit: I now have a draft example below.]

We could also consider suggesting that every Subject.Factory come with an extension-method "overload" of StandardSubjectBuilder.that (#660).

[edit: See also #660 (comment), which suggests that we may be able to make extensions easier to write under Kotlin than under Java.]

cpovirk commented

Depending on how we decide #572, we might also recommend an extension-method overload on the type under test.

[edit: To be explicit: This would probably be in addition to the old-style entry point, since the old-style entry point is probably the best way to serve Java users of the Kotlin Subject.]

However, as I noted in that thread, we'll want to think about how sad it would be if some custom Subject classes had these various extension methods and others did not. (That's probably tolerable, especially if there's a clear line (build-in Subject vs. custom Subject?) but we should think about it.)

(Even some pure-Kotlin extensions are likely to omit anything that we recommend: We've seen lots of custom Subject implementations that don't expose a Subject.Factory, for example (though it's possible that things have improved since we revamped our docs).)

cpovirk commented

Actually, I'm just now seeing that our internal Kotlin Subject classes' assertThat methods are nearly evenly split between:

  • a top-level ("package-scope?") function
  • a @JvmStatic method on a companion object

The former may be more likely to appear in autocomplete. The latter lets us produce more conventional Java-facing APIs, with FooSubject.assertThat and FooSubject.foos appearing on the instantiable FooSubject type, not some separate FooSubjectKt type. [edit: update: Yes, some recent Google-internal guidance suggests that we should recommend a companion object for exactly this reason, at least for anyone whose subject might be used from Java. This is despite external guidance to prefer a top-level function.] But I'm not up to date on this.

We should also consider the exposed Subject.Factory. I think that a top-level val might automatically result in a Java name like getFoos() instead of the foos() that we would recommend to produce expect.about(foos()).that(...). But maybe I'm wrong, and in either case, it may be overrideable.

cpovirk commented

(I notice that the Square's reflective-field-comparison subject currently exposes only a Subject.Factory without an assertThat method. It's an unusual case because it can function on any kind of object.)

cpovirk commented

Also, we've learned that users need to initialize their factory property to Subject.Factory(::FooSubject), rather than just ::FooSubject. That may be tricky, especially for inexperienced Kotlin users. (Maybe users aren't really "supposed to" have to write all that, but they currently do because of generics?)

cpovirk commented

It's pretty simple

...he says :) And now we're seeing some ways that Subject authors don't just face a technical choice (like top-level functions vs. companion object) but can actually cause problems for their users. Specifically, we're seeing problems with nullness. And while I've been hesitant to spend much time on adding nullness annotations for a testing library, we have a team interested in that for their own (somewhat unusual) reasons, and their work is likely to pay dividends for normal Kotlin users... and maybe also complicate things here and there; we'll see :)

(The big point so far is that Subject construction should accept null values: assertThat(foo: Foo?), createSubject(metadata: FailureMetadata, actual: Foo?), private constructor(metadata: FailureMetadata, actual: Foo?). Oh, and now that I write that, I notice that we should probably say something about that private constructor bit (vs. exposing a "normal" public constructor).)

[edit: And we might then nudge people toward defining private fun actualNonNull() = actual!!.]

cpovirk commented

I haven't run this by anyone yet, but just to write down at attempt to capture what I know so far, hopefully without any embarrassing mistakes [edit: OK, I've already had to edit it a few times :)]:

class FooSubject
private constructor(metadata: FailureMetadata, private val actual: Foo?) :
  Subject(metadata, actual) {
  ...

  companion object {
    @JvmStatic
    fun assertThat(foo: Foo?) = assertAbout(foos()).that(foo)

    @JvmStatic
    fun foos() = Factory(::FooSubject)
  }
}

[edit: companion object goes at the bottom by convention.]

[edit: I looked more at #660 (comment), and the main downside seems to be that it doesn't work work with @JvmStatic.]

cpovirk commented

In another context, we're discussing whether method references or lambdas are more idiomatic in Kotlin. However, even if we decide that lambdas are more idiomatic in a vacuum, I think I'd continue to suggest method references for Truth extensions, as I believe the comparison is:

fun foos() = Factory(::FooSubject)

vs.

fun foos(): Factory<FooSubject, Foo> = { FooSubject(it) }

Or wait, maybe it's this, which would be less bad? I should check sometime, but not now....

fun foos() = Factory { FooSubject(it) }

Now, if someone were using Explicit API Mode, then I think the two would be closer in length: We'd probably be comparing my initial lambda version with something like the following method-reference version (but I haven't tested it, including not having tested whether we would still need a second mention of "Factory" in this case):

fun foos(): Factory<FooSubject, Foo> = ::FooSubject