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

Document how to write a Truth extension under Kotlin #661

Open
cpovirk opened this issue Feb 4, 2020 · 8 comments
Open

Document how to write a Truth extension under Kotlin #661

cpovirk opened this issue Feb 4, 2020 · 8 comments
Labels
P3 not scheduled type=documentation Documentation that is other than for an API type=enhancement Make an existing feature better

Comments

@cpovirk
Copy link
Member

cpovirk commented Feb 4, 2020

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 cpovirk added type=other Miscellaneous activities not covered by other type= labels P3 not scheduled type=documentation Documentation that is other than for an API and removed type=other Miscellaneous activities not covered by other type= labels labels Feb 4, 2020
@cpovirk
Copy link
Member Author

cpovirk commented Feb 6, 2020

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
Copy link
Member Author

cpovirk commented Feb 7, 2020

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
Copy link
Member Author

cpovirk commented Mar 6, 2020

(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.)

@raghsriniv raghsriniv added the type=enhancement Make an existing feature better label Apr 19, 2021
@cpovirk
Copy link
Member Author

cpovirk commented Jul 26, 2022

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
Copy link
Member Author

cpovirk commented Feb 16, 2023

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
Copy link
Member Author

cpovirk commented Mar 2, 2023

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
Copy link
Member Author

cpovirk commented Mar 27, 2023

@cpovirk
Copy link
Member Author

cpovirk commented May 3, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P3 not scheduled type=documentation Documentation that is other than for an API type=enhancement Make an existing feature better
Projects
None yet
Development

No branches or pull requests

2 participants