Add OptionalSubject.hasValueThat() (which returns a Subject)

[moaxcp]

Would it be possible to add something similary to Subject.isSameAs to the OptionalSubject? In my api I have been switching methods to return Optional and I'm noticing that some of the asserts would be much easier if OptionalSubject had a method like isSameAs. It should assert that the value is the same instance as another value.

[kluever]

If we added this, we'd probably want to name it similar to the existing OptionalSubject.hasValue(), which means we would end up with hasValueThatIsSameAs(), which is a mouthful.

And at that point, you're probably better off just calling .get() on the subject:

assertThat(optional).hasValueThatIsSameAs(value); assertThat(optional.get()).isSameAs(value);

And FWIW, I do see several hundreds results for ".get()).isSameAs(" inside of Google's codebase, so it does seem like a useful API to have, but getting a reasonably short name that is consistent with hasValue() is going to be difficult.

Happy to leave this request open for more discussion though.

[moaxcp]

For now I am calling get() but to prevent the test from erroring with NoSuchElementExceptions I'm also asserting on isPresent().

assertThat(optional).isPresent();
assertThat(optional.get()).isSameAs(value);

This helps fail rather than error with an unexpected exception.

I would rather do something like

assertThat(optional).hasValueThatIsSameAs(value);

even if it is verbose.

[kluever]

What would be the harm in omitting the isPresent() check? The test will still fail since an exception will be thrown.

[moaxcp]

The test will still fail. When an unexpected exception is thrown junit reports it as an error instead of a failure. In general I dont like unexpected errors but maybe it is ok in this case. My worry is the message is not as descriptive with an error vs failure.

[moaxcp]

What if hasValue returned a Subject?

assertThat(optional).hasValue(object).isSameAs(object);

Or another idea is to add hasValueThat which returns a Subject. I noticed something similar used for exceptions.

assertThat(optional).hasValueThat().isSameAs(object);

[jbduncan]

I personally love the idea of assertThat(optional).hasValueThat().isSameAs(object);. :)

[ronshapiro]

Those assertions typically don't work well for anything that isn't an Object type. Otherwise we need the subject factory and passing that in gets awkward.

On Sun, Oct 28, 2018, 5:27 PM Jonathan Bluett-Duncan < [email protected] wrote:

I personally love the idea of assertThat(optional).hasValueThat().isSameAs(object);. :)

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/google/truth/issues/493#issuecomment-433743072, or mute the thread https://github.com/notifications/unsubscribe-auth/AAwY3XbYggydA0F-tr5KcULvcKGQis5cks5upiFcgaJpZM4XyqgF .

[moaxcp]

I think I see what you are saying. OptionalSubject uses Optional<?> so when actual().get() is called it returns an Object type. Is it possible for OptionalSubject to be generic? I think it could work for supported types at least.

public final class OptionalSubject<T> extends Subject<OptionalSubject, Optional<T>> {

This way when the StandardSubjectBuilder is called with that(actual().get()) it will return the correct type of Subject.

For extensions, I have an idea that may be terrible but I want to mention it.

Could a conversion method be added to Subject?

assertThat(optional).hasValueThat().as(employees()).hasUsername("kak");

From what I can tell the Subject already has metadata and actual. Could these be passed to a builder?

Also it may read better with a singlular factory.

assertThat(optional).hasValueThat().as(employee()).hasUsername("kak");

Having a plural factory makes sense when using .about(employees()) but I don't see why a singular factory couldn't be added for this case.

[kluever]

The hasValueThat() proposal seems reasonable to me, but I'll need to bring this through our internal API review process, and do some additional research into the number of potential users. I'll try to get to that next week.

Thanks for the proposal!

[cpovirk]

Related: https://github.com/google/truth/issues/315

[cpovirk]

And FWIW, I do see several hundreds results for ".get()).isSameAs(" inside of Google's codebase, so it does seem like a useful API to have

Without doing any research, I would guess that most of these could use isEqualTo (but isSameAs works OK because they're using enums, exceptions, small integers, etc.). (This kind of assertion is why we renamed isSameAs to isSameInstanceAs.)

Still, there may well be value in this. I just learned that internally that there was an agreement to add assertThat(optional).hasValueThat() to return a plain Subject last year, so the particular case of isSameAs might become possible soon.