google / truth Goto Github PK

the code-gen jars are not tagged with @GwtIncompatible("java.lang.reflect.*") and are also not excluded from the gwt.xml configuration.

Hi.

Has it been considered that the ASSERT.that(list).has().allOf(foo, bar).inOrder() be changed to ASSERT.that(list).hasAllOf(foo, bar).inOrder()? And same for the rest of Has?

Or, is there a discussion where hasBlah() was voted out in favor of has().blah()?

The way it is the has() methods seems a little hiccupy as opposed to "fluent" to me. And it probably pose some discoverability issue to unfamiliar users because one has to type "has()." first in order to see the things he's really looking for.

Also, I have the habit of writing chained method calls one at a line, but the has() makes it odd to read:

ASSERT.that(list)
.has()
.allOf(foo, bar)
.inOrder();

Because has() and allOf() are parts of the same atomic clause of the sentence, with neither alone makes the sense.

Imho, it looks more natural as:

ASSERT.that(list)
.hasAllOf(foo, bar)
.inOrder();

Where each line states a single but atomic aspect of the assertion.

Someone just commented internally that this would be handy for testing AbstractFuture. [edit: Well, maybe not for that specifically, in light of #268. But it's useful for plenty of other things.] Someone else has evidently had the same thought, adding a project-local FutureSubject with the following API:

• isDone
• isPending
• hasValue(Object)
• failedWith(Class<? extends Throwable>)
• isFailed
• isCancelled

It is very easy to add and would be a great help for all osgi users. Here a example maven plugin config taken from the google guava pom.xml:

      <plugin>
        <groupId>org.apache.felix</groupId>
        <artifactId>maven-bundle-plugin</artifactId>
        <version>2.3.7</version>
        <executions>
          <execution>
            <id>bundle-manifest</id>
            <phase>process-classes</phase>
            <goals>
              <goal>manifest</goal>
            </goals>
          </execution>
        </executions>
        <configuration>
          <instructions>
            <Export-Package>!com.google.common.base.internal,com.google.common.*</Export-Package>
            <Import-Package>
              javax.annotation;resolution:=optional,
              sun.misc.*;resolution:=optional
            </Import-Package>
          </instructions>
        </configuration>
      </plugin>

Such objects are at least a little evil, but they exist in one or two Google-internal projects. If we currently support this, then we should continue to do so, or at least we should have a test that lets us know that we're breaking it so that we don't do so by mistake.

Given the frustration with trying to get maven push rights to org.junit.contrib, etc., Truth as a project should determine what it wants to do with java packages and maven groupIds etc.

My personal view is that we move to "org.truth" as both the maven groupId and the package id. truth.org (the domain) is squatted, but unused for software, and no one seems to be using hte package anywhere I could find in the world. Even if the project were never hosted at www.truth.org, it's irrelevant, though irksome.

It's also useful to look at what is junit-specific and what is not. Assume is, and Expect (as a Rule) is, but they could be implemented otherwise in other frameworks. Thoughts? Comments?

The authors who have contributed code not already under Google copyright (but who happen to also be Google employees) have all agreed to turn this project over to Google. It is primarily googlers who are active developers, and Google has a good github->internal syncing process, and in general this is going to be an actively supported project by Google. We feel Google is a good organizational steward of the project. It will continue to be an Apache 2.0 licensed project - in practice very little (including the project's coordinators) will change. What will change are:

1. The project will be under http://github.com/google/truth. We plan to migrate the repo, so issues, commit history, pull history, and all of the rest are preserved
2. Contributors will need to sign Google's Contributor License Agreement instead of the ad-hoc one created for Truth.

what may change (but is not clear yet is):

• Project may release future versions as com.google.truth:truth in the maven artifact(s)
• Project may (prior to a 1.0 release) rename the packages to com.google.testing.truth (though this is very much not certain - we're discussing.)

what won't change:

• fundamental purpose of the library, it's open-source nature and license, it's availability to the community

The params should all be @nullable.

to test for emptiness

Sometimes the hashCode() of a class implementing an interface is important and should be tested (for example for collections, Map.Entry). In such cases it would be nice to have Subject.hasHashCode() instead of having to write assertThat(obj.hashCode()).isEqualTo(h) because the former could provide a better failure message including the value of obj.toString().

That's just it - I make rookie mistakes.

It used to be present as the confusingly named hasContentsAnyOrder/hasContentsInOrder. The latter is still achievable with iteratesAs, but the former is not, and anyway, Christian prefers that they both be achievable through the same interface. exactly() is currently present in the codebase but commented out.

https://github.com/truth0/truth/blob/master/core/src/main/java/org/truth0/subjects/CollectionSubject.java

Special support for assertions on messages and causes (cause chains?) would be nice, plus then the failure messages could dump the whole exception which would be useful when things break.

I just ran into a case where i was doing:
ASSERT.that(expectedException.getMessage()).contains(....)

which failed because i had introduced a bug that caused a different exception to be thrown. If truth had been able to dump the whole exception (with stack traces), instead of just the mismatched message it would have been much easier to debug. (The stack trace of the cause pointed to the error, but that was hidden)

(I slipped a change behind Christian's back that did this a while back, but it's since been undone. I'm copying and pasting most of my original email, even though hasContentsAnyOrder is no longer the method affected. Now it's about IterableSubject.iteratesOver and the various has() options from CollectionSubject.

  @SuppressWarnings("unchecked") // varargs!
  public void testCartesianProduct_unrelatedTypes() {
    Set<Integer> x = set(1, 2);
    Set<String> y = set("3", "4");

    List<Object> exp1 = list((Object) 1, "3");
    List<Object> exp2 = list((Object) 1, "4");
    List<Object> exp3 = list((Object) 2, "3");
    List<Object> exp4 = list((Object) 2, "4");

    ASSERT.that(Sets.cartesianProduct(x, y)).hasContentsAnyOrder(exp1,
exp2, exp3, exp4);
  }

com/google/common/collect/SetsTest.java:779:
hasContentsAnyOrder(java.util.List<java.lang.Object&java.io.Serializable&java.lang.Comparable<?
extends java.lang.Object&java.io.Serializable&java.lang.Comparable<?>>>...)
in org.junit.contrib.truth.subjects.IterableSubject<capture#106 of ?
extends org.junit.contrib.truth.subjects.CollectionSubject<?,java.util.List<java.lang.Object&java.io.Serializable&java.lang.Comparable<?
extends java.lang.Object&java.io.Serializable&java.lang.Comparable<?>>>,java.util.Collection<java.util.List<java.lang.Object&java.io.Serializable&java.lang.Comparable<?
extends java.lang.Object&java.io.Serializable&java.lang.Comparable<?>>>>>,java.util.List<java.lang.Object&java.io.Serializable&java.lang.Comparable<?
extends java.lang.Object&java.io.Serializable&java.lang.Comparable<?>>>,java.util.Collection<java.util.List<java.lang.Object&java.io.Serializable&java.lang.Comparable<?
extends java.lang.Object&java.io.Serializable&java.lang.Comparable<?>>>>>
cannot be applied to (java.util.List<java.lang.Object>,java.util.List<java.lang.Object>,java.util.List<java.lang.Object>,java.util.List<java.lang.Object>)
    ASSERT.that(Sets.cartesianProduct(x, y)).hasContentsAnyOrder(exp1, exp2, exp3, exp4);

cartesianProduct() isn't returning a Set<List<Object>> it's returning a Set<List<Object&Serializable&Comparable<...>>. Luckily, I know the magic words (a <Object> on the cartesianProduct() call), but I suspect that many people won't get past the C++-template error messages.

I'm sure that you're familiar with this general problem from Hamcrest, FEST, etc., so I can believe that you have a good reason to use the restrictive types, but my initial experience has been scary :)

In the current release of truth, the following is a compile-time error:

new SubjectFactory<PrimitiveByteArraySubject, byte[]> {
  @Override public PrimitiveByteArraySubject getSubject(FailureStrategy fs, byte[] that) {
    return new PrimitiveByteArraySubject(fs, that);
  }
}

This is unfortunate because (until said subject is released) I'd like to assert_().about(byteArray()), but cannot.

This is because AbstractArraySubject<S extends AbstractArraySubject<S, T>, T> extends Subject<AbstractArraySubject<S, T>, T> and not Subject<S, T>; this is incompatible with the generic bounds of S in SubjectFactory. Changing the supertype to Subject<S, T> solves the problem, and doesn't appear to have any adverse effect.

I have implemented this (trivial) fix and signed a CLA, but before opening a pull request I'm wondering:
a) if the current "behavior" is intended, and
b) whether you the maintainers believe this is worth fixing (I do!).

I'm sure this isn't a new issue but I couldn't find an existing report:

assertThat(ImmutableList.of(1L)).containsExactly(1);

Results in

java.lang.AssertionError: Not true that <[1]> contains exactly <[1]>. It is missing <[1]> and has unexpected items <[1]>

In the situation where the output would be ambiguous, it would be nice if Truth would disambiguate by adding the "L" suffix as necessary.

This should be the last step in the 1.0 milestone. Ship it!

The font is too small. The comments are gray on a gray background. Consider using better font + colors

Here is a good example:

http://googlecloudplatform.github.io/gcloud-node/#/

There is assertThat(double[]) but not assertThat(double) which would be very useful to compare floating points.

Linked from http://google.github.io/truth/#more-detailed-usage-information

After upgrading to Truth 0.25, all my tests about collections and sets produce deprecation warnings because Truth.assertThat(Collection<T>) is now annotated with @Deprecated.

I now have to either use @SuppressWarnings or add an ugly explicit upcast to Iterable for each of these tests. Please consider removing either the annotation or the whole method.

I think that, if this doesn't exist, we will see people continue to write their assertions in multiple parts:

assertThat(foo).hasSize(5);
assertThat(foo).containsEntry(a, b);
assertThat(foo).containsEntry(c, d);
...

For example,

ASSERT.that(bool);

... will always pass, since it's not an error-creating part of the fluent chain. Someone needs to add something additional (isNull() or isTrue()) to "turn the crank". Java can't force this, but error-prone and similar analysis frameworks can detect this and make it a compile error. This is never what you want.

Hi.

Wondering if assertions on Collection and List can be applied to all Iterable's.

Most assertions from ListSubject and CollectionSubject seem to also make sense to iterables:

isEmpty()
isOrdered()
containsSequence()
has().exactly(...)

So how about flattening the class hierarchy and just have all these assertions for any Iterable?

Internally some implementations could simply copy the Iterable into a List. It's a test after all.

I believe it also makes the API surface smaller.

A comment from a pending CL to Guava, in which I write my own assertion:

  if (urlsFromLoader.size() != urlsFromClassPath.size()) {
    fail(String.format(
        "Number of URLs from system classloader (%s) and number of URLs from ClassPath (%s) "
            + "do not match",
        urlsFromLoader.size(), urlsFromClassPath.size()));
  }

I could just do assertThat(urlsFromClassPath.size()).named("URLs from ClassPath for resource " + resource).isEqualTo(urlsFromLoader.size()), but I have reservations:

snip other reservation

• I'm wondering how legitimate it is to say that the number of URLs from the ClassLoader is the "right" number. I'm not sure why -- I guess because it's computed, too, in a way complicated enough that it might be wrong? But that would still mean that it's "expected," just that we're uncertain. I guess that's why I want to give it a label, too -- to say that we expected the actual value to match the number from the ClassLoader, rather than "it should obviously be 3."

I'm not sure that I have a real point there, but it's at least possible that I have a scenario in which I really don't have a true "expected." Suppose that I have two pieces of code I want to keep in sync -- maybe two sets of constants in separate classes that need to match but can't for build-system reasons. It's a little weird to say that one is "expected" and the other "actual."

Do we really care? Probably not, especially since I have no idea how our API would accommodate this. But I'm filing the issue while it's on my mind. Maybe it will turn out to be important someday, and maybe someone will come up with an elegant solution. But I plan to forget about this entirely for a while :)

There are a couple of broken links on http://google.github.io/truth/usage:

• Under the heading "Super basics", the link "More examples are here" goes to http://google.github.io/detail, which gives a 404
• Under "More Comprehensive Examples", the link "can be found in the Example section" goes to http://google.github.io/detail as well.
• Under "Extensibility", the link "Extending Truth" goes to http://google.github.io/extension, which gives a 404

Example:

{ "a" : 1, "b" : 2 }

Right now there is no way to check if the map only contains key "a".

Add to CollectionSubject for consistency.

By default, failure messages show the underlying value (enclosed in <>). Subject#named sets customName to the user-provided string, and failure messages show that instead of the underlying value (enclosed in "").

I'd like a method which is like Subject#named, except that it prepends the user-provided string to the underlying value, instead of replacing it. This would be useful any time the stack trace isn't enough to show you exactly what asserted (and where the subject isn't boolean — with boolean, there's only one value which will make the test fail, so the underlying value isn't needed).

Example use cases:

1. When making an assertion in a loop. (In my example, I have a number of types enumerated by an enum, and I have a class which counts things classified by type, and I want to assert that the class returns the correct count for each type.)
2. When using an Expect instead of an ASSERT, so we don't get the stack trace.

The alternatives seem to be writing a custom Subject, which seems heavyweight, or making assertions about Pair.of(underlyingValue, label), which seems horrible.

I suspect the hardest bit of this is coming up with the right name for the method. It's a shame that labeled() is taken, as it would be perfect. I tentatively offer withNamePrefix as a clumsy-but-clear option.

I just saw this in a code review:

ASSERT.that(collection.containsAll(values));

What was meant was:

ASSERT.that(collection).containsAll(values);

(The error was a little less obvious in the real code, where "collection" and "values" were longer expressions.)

I don't really have a solution here.

Blah

closure-templates has a bunch of tests for how the compiler generates files. It would be nice if Truth had some support for asserting on local files via File objects.

e.g.
assertThat(file).exists()
assertThat(file).doesNotExist()
assertThat(file).contentEquals(otherFile);
assertThat(file).isDirectory();

others?
URL objects? java7 Paths?

I just saw some code like this (caveat: It was automatically generated from a migration from an old assertContains method, but I think the point stands equally well under those circumstances):

assertThat(ImmutableList.of(FOO, BAR, BAZ), contains(object.getValue());

If this comes up much, it would be nice for users to be able to write:

assertThat(object.getValue()).isIn(ImmutableList.of(FOO, BAR, BAZ));

But perhaps that's more useful with constants:

assertThat(promotion.getCountry()).isIn(SUPPORTED_COUNTRIES);

And users who don't have a constant would probably prefer:

assertThat(object.getValue()).isOneOf(FOO, BAR, BAZ)

Needless to say, this would require further research and discussion.

We probably want for it to treat a null "varargs" parameter as a single argument, rather than as a null array (similar to what methods like containsAllOf do by calling our helper accumulate method).

Integrate Truth with Google's automatic CLA checking infrastructure.

When an ASSERT fails, it dumps a stack trace. Expect is really nice because it allows lumping several closely-related tests together into a single test case, but if one fails it's difficult to see exactly which it was (see below). Including a stack trace from the point of failure would make this much easier. Has this been considered?

java.lang.AssertionError: All failed expectations:
  1. Not true that "foo" is "bar"
  2. Expected IllegalArgumentException parsing "baz"

    at org.truth0.Expect.evaluate(Expect.java:74)
    at junit.runners.ParentRunner.runLeaf(ParentRunner.java:263)
    at junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:68)

It just goes on from there into the test runner, etc, not indicating any lines in my test case, since the error isn't thrown until the end of the test.

How does Truth feel about adding things to ClassSubject?

There are assertions that I'd like to make using reflection (e.g. assertThat(someClass).isAnnotatedWith(Singleton.class);) that aren't currently supported without custom subjects. Is there any strong feeling for or against adding at least this assertion?

I haven't thought very deeply about inheritance, and there are probably arguments to be made in that area. If there isn't any opposition, I'll fork and get to work on it.

Thanks!

It would be nice if there was a way to test predicates on elements of a collection.
assertThat(myList).hasSomeWith(myPredicate); (for an any predicate test), and
assertThat(myList).hasEveryWith(myPredicate); (for an every predicate test).

Some potential assertions:

void isEmpty();

IntegerSubject size();

CollectionSubject keys();

CollectionSubject values();

MapSubject contains(key, value);

That way, I can do:

ASSERT.that(map).size().isEqualTo(10);
ASSERT.that(map).keys().hasAnyOrder(a, b);
ASSERT.that(map)
.contains(key1, value1)
.contains(key2, value2)

The other ones are all there for TestVerb but this one is conspicuously missing.

assertThat(someOptionalFoobarString).named("FroBar").hasValue(expected);

results in "Optional has an incorrect value. expected: but was:".

What happened to the custom name "FooBar"? Looks like it's simply ignored:

https://github.com/google/truth/blob/master/core/src/main/java/com/google/common/truth/OptionalSubject.java#L61-L64

  if (!getSubject().get().equals(expected)) {
    if (getSubject().get() instanceof String) {
      this.failureStrategy.failComparing("Optional<String> has an incorrect value.",
          (String) expected, (String) actual);

similar to #101

Need something like:

ASSERT.withMessage("message").that(myCollection).has().exactly(1, 2, 3);
ASSERT.about(type).withMessage("message").that(....)

Also, Truth error messages -- there's no real reason they can't be long. They should provide the following.

1. User-provided message.
2. Truth's generated description of what failed.
3. Full toString() of the "original" subject (perhaps truncated to XX,000 characters or whatever)
4. Stack trace (automatically from JUnit)

Due to an ambiguity (spec bug) in the JLS, there's a difference between how Eclipse and Sun's JDK on one hand compiles the following, vs. how OpenJDK's javac compiles it. Eclipse and Sun can handle this, including the type inference at play from the generic about() method.

public static final SubjectFactory<FooSubject, Foo> FOO = new FooSubjectFactory();
...
// assuming ASSERT is some AbstractVerb subclass
ASSERT.about(FOO).that(someFoo).does(somethingFooIsh);

This is the standard delegation form of extension for Truth. Unfortunately, the statement

ASSERT.about(FOO);

... itself fails to compile on OpenJDK, because it drops a piece of type inference along the way and, as it is, ends up presuming the output is Object. It works, however, if one forces the inference in assignment.

DelegatedVerb<FooSubject, Foo> FOO_ASSERT = ASSERT.about(FOO);
FOO_ASSERT.that(someFoo).does(somethingFooIsh);

This is frustrating, because it means that OpenJDK compilers will fail on the literate, convenient syntax for delegation, which means organizations that use OpenJDK internally as the only "approved" JDK can't use this syntax, as currently implemented. They can create a test-local FOO_ASSERT, or they can use extension, but the bugs logged with Oracle are circa-2009 and it looks like this is a long way out from fixed - even in Java7, since it requires JLS changes.

We can look at an alternative way to get this same effect. I'll think about how to accomplish it in a similarly convenient syntax.

Using thatEach with a custom SubjectFactory results in the compilation of a generated class, which forwards the proposition arguments to the original subject:

Iterable<Foo> data = iterable(new Foo(2 + 3), new Foo(2 + 4));
assert_().in(data).thatEach(FooSubject.FOO).matchesEither(new Foo(5), new Foo(6));

The generated code has a bug and the first argument Foo(5) will be passed to the original subject's method multiple times. All additional arguments will be dropped:

Not true that <Foo(6)> matches one of <Foo(5)> <Foo(5)>

In cases where the arguments have incompatible types, the generated code does not even compile.