Expand and externalize our docs on writing your own Subject

[cpovirk]

We have some docs internally. Other things for us to cover:

• Don't throw AssertionError directly: Call this.fail():
  • fail() generates the appropriate type of failure (e.g., AssumptionViolatedException when someone uses assume().
  • fail() uses the user's failure message and subject name.
  • fail() has message to disambiguate objects with the same toString() but different classes.
  • fail() comes in overloads that help you construct the message.
• Similarly, don't delegate sub-object assertions to assertThat(...): Delegate to check().that(...). (But note bug Do calls to Subject.check() lose the failureMessage? #199.) This preserves the failureStrategy.
• Once you call fail(), don't do anything else. If someone is using expect() (or some custom FailureStrategy that logs failures -- I think Google has at least one), fail() won't throw, so execution will continue.
  • This could result in multiple separate failure reports ("size is different," "missing element a," "missing element b"). (Sometimes that could be a good thing, but if it's a good thing, then we should do it for assert(), too, by putting everything in one fail() call!)
  • Worse, it could result in exceptions: If we fail() if the sizes of two collections differ but then continue to iterate over them in parallel, we're likely to throw NoSuchElementException. This defeats the purpose of expect() and might(?) obscure the real error.
  • We could maybe "solve" the former problem by wrapping the FailureStrategy in some kind of failOnlyTheFirstTimeStrategy, but this doesn't help with the second problem.
  • Related question: What happens with expect() and chained assertions like containsExactly().inOrder()? I guess that we need to return the "inOrder() always succeeds" object, which we should maybe stop calling IN_ORDER and documenting as though it means that. It's possible that the failOnlyTheFirstTimeStrategy approach could be useful here, but I'm wary of it.
• Distinguish between "check failed" and "bug in your test: check is bogus" (issue Policy describing the difference between "check failed" and "could not evaluate check" #207).
• Use failComparing() when substring/subsequence matches are helpful to highlight.
• [If we ever require you to override named() (issue Consider removing self type <S> from Subject #202), we should document that.]

[cpovirk]

• How to expose your SubjectFactory: FooSubject.factory()? FooSubject.foo()? FooSubject.foos()? And/or do you expose your own assertThat method, which can be static imported alongside other overloads so long as they don't overlap (like if you wrote your own StringSubject) or conflict (like if you created a SerializableSubject that could apply to, say, ImmutableList as well as `IterableSubject could). (We've had a little discussion on this internally but haven't reached a conclusion.)

[cpovirk]

• Anything necessary for your subject to work with thatEach? Or, as we've discussed from time to time, we could remove thatEach or substantially change how it works.

[cgruber]

I'm taking the assignment, and will start to do the harmonization/sync infrastructure work, and when I'm done, I'll flip it to @kluever to massage the content.

[cpovirk]

• At some point, we should decide whether to recommend that all assertion methods be named for a singular subject (hasValues) or whether some can be plural (haveValues). I'm still a fan of singular, since things like isEqualTo will always be around, but someone (Kevin?) felt pretty strongly in the other direction.

[cpovirk]

• If you provide a chaining API for asserting about subobjects, how should you name the chaining methods? We have MultimapSubject.valuesForKey... so probably like that?

[cpovirk]

• Discourage people from saying (for example) "AtomicLongMap is basically a Map, so let's either (a) have assertThat(atomicLongMap) return a MapSubject or (b) make AtomicLongMapSubject extend MapSubject."

Both produce weird behavior for most of the methods inherited from Subject:

• isAnyOf
• isEqualTo
• isInstanceOf
• isNoneOf
• isNotIn
• isNotInstanceOf
• isNotEqualTo
• isNotNull
• isNotSameAs
• isNull
• isSameAs

We could conceivably override those. We'd also need to put in special logic for null, something like:

return atomicMap == null 
    ? new AtomicLongMapSubject(null, null) 
    ? new AtomicLongMapSubject(atomicMap.asMap(), atomicMap); 

But at that point, we might as well just inherit only from Subject and delegate our implementations to an internal MapSubject where helpful.

[cpovirk]

• Document the unchecked cast necessary if you want to extend IterableSubject and implement a corresponding SubjectFactory. Or make the cast unnecessary by zigging or zagging with named(). Or make it all code generated, as Christian has discusses.

[cpovirk]

RE: SubjectFactory naming (#208 (comment)):

I might prefer singular names:

• assertAbout(optionals()) sounds a little weird to me. (This might be unique to Optional, which I guess I still don't fully see as a sensible noun. But likely I'll get there someday :))
• "optional()" seems easier to work into other possible APIs:

Compare:

assertThatEvery(optional()).in(list).isPresent();
assertThatSome(optional()).in(list).isPresent();

vs.

assertThatAll(optionals()).in(list).isPresent(); // "all... is"
assertThatAtLeastOneOfThe(optionals()).in(list).isPresent(); // yuck

On the other hand, I sort of prefer the sound of "assertAbout(strings())" to "assertAbout(string())." But I do think that the latter is probably clearer: You're asserting only about one string.

[cpovirk]

In favor of plural names: "string()" sounds kind of weird by itself -- like it's a specific string? -- whereas "strings()" sounds more like it's about the type of strings in general.

[cpovirk]

Slightly more reasonable but still kind of messy:

assertThatEachOfThe(strings())...
assertThatOneOfThe(strings())...