Problems with ListAssert and Wildcards #121

Open
jschneider opened this Issue Oct 27, 2012 · 15 comments

Comments

Projects
None yet
5 participants

I'd expected the following code to compile successfully:

List<String> strings0 = new ArrayList<String>();
List<? extends String> strings1 = new ArrayList<String>();

assertThat( strings0 ).isEqualTo( strings0 );
assertThat( strings0 ).isEqualTo( strings1 );
assertThat( strings1 ).isEqualTo( strings1 );
assertThat( strings1 ).isEqualTo( strings0 );
Contributor

joel-costigliola commented Oct 27, 2012

Well ... generics hell.

If you look at isEqualTo definition, you can see that it takes the object under assertion type as the parameter type
To be clear : assertThat("str") expects to get a String as isEqualTo parameter.

Note that, in your example, strings0 = strings1; does not compile because strings0 only accepts String but no subclasses of String.
This explains why assertThat( strings0 ).isEqualTo( strings1 ); does not compile : you are trying to compare incompatible types.

When it comes to generics hell, I often read Angelika Langer Generics FAQ.

Without having consulted Anglika's FAQ (which is bad I know), I would say that for assertThat( strings1 ).isEqualTo( strings1 );, you can't compare two List<? extends String> because they may contain different objects type (let's say List<MyStringSubclass> and List<YourStringSubclass>).
Same reason for the last one.

Owner

alexruiz commented Oct 28, 2012

I think that making FEST-Assert more 'type safe' by using generics is not
the right approach. In tests it is OK to relax type safety a bit. Sometimes
making things compile in tests can add more work than necessary. For
example, if I'm asserting that an instance of Person is equal to another
one, and the method that the returns the Person to verify returns it as
Entity, I would have to cast it to Person just to satisfy the test. I don't
see the point. The test is going to run anyway and it will tell us if it
passed or not.

In the case that Johannes showed, who cares if the lists are declared with
slightly different types? what matters is verifying that the elements are
equal.

I've been thinking about it for some time, even before Johannes' message. I
think reverting to the old behavior (in FEST-Assert) is the best to do.

Cheers,
-Alex

On Sat, Oct 27, 2012 at 4:13 PM, Joel Costigliola
notifications@github.comwrote:

Well ... generics hell.

If you look at isEqualTo definition, you can see that it takes the object
under assertion type as the parameter type
To be clear : assertThat("str") expects to get a String as isEqualToparameter.

Note that, in your example, strings0 = strings1; does not compile because
strings0 only accepts String but no subclasses of String.
This explains why assertThat( strings0 ).isEqualTo( strings1 ); does not
compile : you are trying to compare incompatible types.

When it comes to generics hell, I often read *Angelika Langer Generics FAQhttp://www.angelikalanger.com/GenericsFAQ/JavaGenericsFAQ.html
*.

Without having consulted Anglika's FAQ (which is bad I know), I would say
that for assertThat( strings1 ).isEqualTo( strings1 );, you can't compare
two List<? extends String> because they may contain different objects
type (let's say List and List).
Same reason for the last one.


Reply to this email directly or view it on GitHubhttps://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9840918.

Contributor

joel-costigliola commented Oct 28, 2012

I see your point Alex, in some other cases having generics is an
improvement, when writing Condition for example.
I think we should weight the pros and cons, and maybe ask in user list what
people prefer.

WDYT ?

Joel

On Sun, Oct 28, 2012 at 5:36 PM, Alex Ruiz notifications@github.com wrote:

I think that making FEST-Assert more 'type safe' by using generics is not
the right approach. In tests it is OK to relax type safety a bit.
Sometimes
making things compile in tests can add more work than necessary. For
example, if I'm asserting that an instance of Person is equal to another
one, and the method that the returns the Person to verify returns it as
Entity, I would have to cast it to Person just to satisfy the test. I
don't
see the point. The test is going to run anyway and it will tell us if it
passed or not.

In the case that Johannes showed, who cares if the lists are declared with
slightly different types? what matters is verifying that the elements are
equal.

I've been thinking about it for some time, even before Johannes' message.
I
think reverting to the old behavior (in FEST-Assert) is the best to do.

Cheers,
-Alex

On Sat, Oct 27, 2012 at 4:13 PM, Joel Costigliola
notifications@github.comwrote:

Well ... generics hell.

If you look at isEqualTo definition, you can see that it takes the
object
under assertion type as the parameter type
To be clear : assertThat("str") expects to get a String as
isEqualToparameter.

Note that, in your example, strings0 = strings1; does not compile
because
strings0 only accepts String but no subclasses of String.
This explains why assertThat( strings0 ).isEqualTo( strings1 ); does not
compile : you are trying to compare incompatible types.

When it comes to generics hell, I often read *Angelika Langer Generics
FAQhttp://www.angelikalanger.com/GenericsFAQ/JavaGenericsFAQ.html
*.

Without having consulted Anglika's FAQ (which is bad I know), I would
say
that for assertThat( strings1 ).isEqualTo( strings1 );, you can't
compare
two List<? extends String> because they may contain different objects
type (let's say List and List).
Same reason for the last one.


Reply to this email directly or view it on GitHub<
https://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9840918>.


Reply to this email directly or view it on GitHubhttps://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9847411.

Collaborator

tedyoung commented Oct 28, 2012

Yes, let's ask users, though since it involves generics, we'd need to
provide a few examples to show the impact of the changes.

I definitely agree, though, that writing Conditions in 1.x was a real pain
in terms of generics, so if the current 2.x improves that (I haven't tried
it yet), then I'm all for it.

;ted

On Sun, Oct 28, 2012 at 9:51 AM, Joel Costigliola
notifications@github.comwrote:

I see your point Alex, in some other cases having generics is an
improvement, when writing Condition for example.
I think we should weight the pros and cons, and maybe ask in user list
what
people prefer.

WDYT ?

Joel

On Sun, Oct 28, 2012 at 5:36 PM, Alex Ruiz notifications@github.com
wrote:

I think that making FEST-Assert more 'type safe' by using generics is
not
the right approach. In tests it is OK to relax type safety a bit.
Sometimes
making things compile in tests can add more work than necessary. For
example, if I'm asserting that an instance of Person is equal to another
one, and the method that the returns the Person to verify returns it as
Entity, I would have to cast it to Person just to satisfy the test. I
don't
see the point. The test is going to run anyway and it will tell us if it
passed or not.

In the case that Johannes showed, who cares if the lists are declared
with
slightly different types? what matters is verifying that the elements
are
equal.

I've been thinking about it for some time, even before Johannes'
message.
I
think reverting to the old behavior (in FEST-Assert) is the best to do.

Cheers,
-Alex

On Sat, Oct 27, 2012 at 4:13 PM, Joel Costigliola
notifications@github.comwrote:

Well ... generics hell.

If you look at isEqualTo definition, you can see that it takes the
object
under assertion type as the parameter type
To be clear : assertThat("str") expects to get a String as
isEqualToparameter.

Note that, in your example, strings0 = strings1; does not compile
because
strings0 only accepts String but no subclasses of String.
This explains why assertThat( strings0 ).isEqualTo( strings1 ); does
not
compile : you are trying to compare incompatible types.

When it comes to generics hell, I often read *Angelika Langer Generics
FAQhttp://www.angelikalanger.com/GenericsFAQ/JavaGenericsFAQ.html
*.

Without having consulted Anglika's FAQ (which is bad I know), I would
say
that for assertThat( strings1 ).isEqualTo( strings1 );, you can't
compare
two List<? extends String> because they may contain different objects
type (let's say List and List).
Same reason for the last one.


Reply to this email directly or view it on GitHub<

https://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9840918>.


Reply to this email directly or view it on GitHub<
https://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9847411>.


Reply to this email directly or view it on GitHubhttps://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9847570.

Owner

alexruiz commented Oct 28, 2012

Writing conditions is the best use case for generics. The thing is the way
we are doing conditions (a carry over from the 1.x branch.)

We were treating conditions as any other assertion, and I think the
Hamcrest way is the best:

assertThat(object, condition)

WDYT?

-Alex

On Sun, Oct 28, 2012 at 9:51 AM, Joel Costigliola
notifications@github.comwrote:

I see your point Alex, in some other cases having generics is an
improvement, when writing Condition for example.
I think we should weight the pros and cons, and maybe ask in user list
what
people prefer.

WDYT ?

Joel

On Sun, Oct 28, 2012 at 5:36 PM, Alex Ruiz notifications@github.com
wrote:

I think that making FEST-Assert more 'type safe' by using generics is
not
the right approach. In tests it is OK to relax type safety a bit.
Sometimes
making things compile in tests can add more work than necessary. For
example, if I'm asserting that an instance of Person is equal to another
one, and the method that the returns the Person to verify returns it as
Entity, I would have to cast it to Person just to satisfy the test. I
don't
see the point. The test is going to run anyway and it will tell us if it
passed or not.

In the case that Johannes showed, who cares if the lists are declared
with
slightly different types? what matters is verifying that the elements
are
equal.

I've been thinking about it for some time, even before Johannes'
message.
I
think reverting to the old behavior (in FEST-Assert) is the best to do.

Cheers,
-Alex

On Sat, Oct 27, 2012 at 4:13 PM, Joel Costigliola
notifications@github.comwrote:

Well ... generics hell.

If you look at isEqualTo definition, you can see that it takes the
object
under assertion type as the parameter type
To be clear : assertThat("str") expects to get a String as
isEqualToparameter.

Note that, in your example, strings0 = strings1; does not compile
because
strings0 only accepts String but no subclasses of String.
This explains why assertThat( strings0 ).isEqualTo( strings1 ); does
not
compile : you are trying to compare incompatible types.

When it comes to generics hell, I often read *Angelika Langer Generics
FAQhttp://www.angelikalanger.com/GenericsFAQ/JavaGenericsFAQ.html
*.

Without having consulted Anglika's FAQ (which is bad I know), I would
say
that for assertThat( strings1 ).isEqualTo( strings1 );, you can't
compare
two List<? extends String> because they may contain different objects
type (let's say List and List).
Same reason for the last one.


Reply to this email directly or view it on GitHub<

https://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9840918>.


Reply to this email directly or view it on GitHub<
https://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9847411>.


Reply to this email directly or view it on GitHubhttps://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9847570.

Collaborator

tedyoung commented Oct 28, 2012

I have to disagree -- but then, I also disagree with some of the other
changes to the fluent syntax (that Alex blogged about) that I haven't had
time to write about.

However, if 2.x makes writing new assertion classes easier (again, I
haven't looked at 2.x yet from this point of view), then conditions become
less important.

My concern is about discoverability, and one of the great things about a
fluent API is how discoverable things are: I just type
assertThat(actual).and I get a bunch of choices of things I can do. In
my environment, that's
just about the most important thing.

;ted

On Sun, Oct 28, 2012 at 10:24 AM, Alex Ruiz notifications@github.comwrote:

Writing conditions is the best use case for generics. The thing is the way
we are doing conditions (a carry over from the 1.x branch.)

We were treating conditions as any other assertion, and I think the
Hamcrest way is the best:

assertThat(object, condition)

WDYT?

-Alex

On Sun, Oct 28, 2012 at 9:51 AM, Joel Costigliola
notifications@github.comwrote:

I see your point Alex, in some other cases having generics is an
improvement, when writing Condition for example.
I think we should weight the pros and cons, and maybe ask in user list
what
people prefer.

WDYT ?

Joel

On Sun, Oct 28, 2012 at 5:36 PM, Alex Ruiz notifications@github.com
wrote:

I think that making FEST-Assert more 'type safe' by using generics is
not
the right approach. In tests it is OK to relax type safety a bit.
Sometimes
making things compile in tests can add more work than necessary. For
example, if I'm asserting that an instance of Person is equal to
another
one, and the method that the returns the Person to verify returns it
as
Entity, I would have to cast it to Person just to satisfy the test. I
don't
see the point. The test is going to run anyway and it will tell us if
it
passed or not.

In the case that Johannes showed, who cares if the lists are declared
with
slightly different types? what matters is verifying that the elements
are
equal.

I've been thinking about it for some time, even before Johannes'
message.
I
think reverting to the old behavior (in FEST-Assert) is the best to
do.

Cheers,
-Alex

On Sat, Oct 27, 2012 at 4:13 PM, Joel Costigliola
notifications@github.comwrote:

Well ... generics hell.

If you look at isEqualTo definition, you can see that it takes the
object
under assertion type as the parameter type
To be clear : assertThat("str") expects to get a String as
isEqualToparameter.

Note that, in your example, strings0 = strings1; does not compile
because
strings0 only accepts String but no subclasses of String.
This explains why assertThat( strings0 ).isEqualTo( strings1 ); does
not
compile : you are trying to compare incompatible types.

When it comes to generics hell, I often read *Angelika Langer
Generics
FAQhttp://www.angelikalanger.com/GenericsFAQ/JavaGenericsFAQ.html
*.

Without having consulted Anglika's FAQ (which is bad I know), I
would
say
that for assertThat( strings1 ).isEqualTo( strings1 );, you can't
compare
two List<? extends String> because they may contain different
objects
type (let's say List and
List).
Same reason for the last one.


Reply to this email directly or view it on GitHub<

https://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9840918>.


Reply to this email directly or view it on GitHub<

https://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9847411>.


Reply to this email directly or view it on GitHub<
https://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9847570>.


Reply to this email directly or view it on GitHubhttps://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9847938.

Contributor

joel-costigliola commented Oct 28, 2012

Writing custom assertions in 2.x is not yet simple enough.

We have #109 to ease that (but I can't work on it until Alex is done with its refactoring) and also the possibility to generate some with fest-assertion-generator and the corresponding maven plugin maven-fest-assertion-generator-plugin. There is also an eclipse plugin coming soon that will allow to do the same thing.

I agree with Ted on discoverability, that made me choose Fest over Hamcrest few years ago.

Owner

alexruiz commented Oct 28, 2012

Ted,

It is not about replacing the fluent API with conditions. It is about
removing conditions from the method chain.

Joel,

I think we will be able to make FEST semi-easy to extend (at least in
Java.) I'm really excited about the FEST Assertion Generator. It will fill
the gap nicely. This project will be our user's best friend :)

Cheers,
-Alex

On Sun, Oct 28, 2012 at 10:46 AM, Joel Costigliola <notifications@github.com

wrote:

Writing custom assertions in 2.x is not yet simple enough.

We have #109 #109 to
ease that (but I can't work on it until Alex is done with its refactoring)
and also the possibility to generate some with fest-assertion-generatorhttps://github.com/joel-costigliola/fest-assertion-generatorand the corresponding maven plugin
maven-fest-assertion-generator-pluginhttps://github.com/joel-costigliola/maven-fest-assertion-generator-plugin.
There is also an eclipse pluginhttps://github.com/joel-costigliola/fest-eclipse-plugincoming soon that will allow to do the same thing.

I agree with Ted on discoverability, that made me choose Fest over
Hamcrest few years ago.


Reply to this email directly or view it on GitHubhttps://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9848176.

BTW: I think the code example above compiles with fest-assert 1.x...

Owner

alexruiz commented Oct 29, 2012

Yes, it should, and that's the behavior I strongly think we should get back
to in 2.x.

On Mon, Oct 29, 2012 at 12:14 AM, Johannes Schneider <
notifications@github.com> wrote:

BTW: I think the code example above compiles with fest-assert 1.x...


Reply to this email directly or view it on GitHubhttps://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9857873.

Contributor

joel-costigliola commented Oct 29, 2012

I also think we should get back to that behavior but only keep strongly
typed Condition, which should be relatively easy, just a matter of changing
assertion parameters from generic type to Object.

On Mon, Oct 29, 2012 at 6:35 PM, Alex Ruiz notifications@github.com wrote:

Yes, it should, and that's the behavior I strongly think we should get
back
to in 2.x.

On Mon, Oct 29, 2012 at 12:14 AM, Johannes Schneider <
notifications@github.com> wrote:

BTW: I think the code example above compiles with fest-assert 1.x...


Reply to this email directly or view it on GitHub<
https://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9857873>.


Reply to this email directly or view it on GitHubhttps://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-9877937.

jsravn commented Nov 9, 2012

I have to agree w/ Alex here. We've been using fest-assert 1.x for the past year or so. One of my favorite things about fest-asserts is how it relaxes type safety, so you don't get the code-diarrhea that is endemic of java generics. Something which substantially detracts from the readability of test code and provides very little positive benefit.

I feel that 1.4 took a step in the wrong direction with the self referencing types and loss of IterableAssert, but the negative effects were pretty limited. It looks like 2.x so far takes it even more extreme, making it much more painful to use.

Owner

alexruiz commented Nov 10, 2012

Hi James,

I'm very curious to know your take on the self-referencing types. It made
code maintenance a lot easier for us. I'd like to know the downsides from
the user point of view.

Thanks!
-Alex

On Fri, Nov 9, 2012 at 12:58 PM, James Ravn notifications@github.comwrote:

I have to agree w/ Alex here. We've been using fest-assert 1.x for the
past year or so. One of my favorite things about fest-asserts is how it
relaxes type safety, so you don't get the code-diarrhea that is endemic of
java generics. Something which substantially detracts from the readability
of test code and provides very little positive benefit.

I feel that 1.4 took a step in the wrong direction with the self
referencing types and loss of IterableAssert, but the negative effects were
pretty limited. It looks like 2.x so far takes it even more extreme, to the
point of the library being very difficult to use for what should be simple
tests as the OP points out.


Reply to this email directly or view it on GitHubhttps://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-10244395.

jsravn commented Nov 11, 2012

Sorry, I should have been more specific. The self referencing types were fine. It was that #isEqualTo (and some other methods?) starting taking a parameterized type as their argument instead of Object. This led to issues where I couldn't do stuff like assertThat(someList).isEqualTo(someCollection), even if I knew for sure the collection was a list. This could occur for legacy api reasons, or whatever.

Owner

alexruiz commented Nov 11, 2012

Thanks James. The behavior you described is completely wrong and should be
fixed. 'isEqualTo' should take an Object. We'll fix that in 2.0.

Cheers,
-Alex

On Sat, Nov 10, 2012 at 4:47 PM, James Ravn notifications@github.comwrote:

Sorry, I should have been more specific. The self referencing types were
fine. It was that #isEqualTo (and some other methods?) starting taking a
parameterized type as their argument instead of Object. This led to issues
where I couldn't do stuff like
assertThat(someList).isEqualTo(someCollection), even if I knew for sure
the collection was a list. This could occur for legacy api reasons, or
whatever.


Reply to this email directly or view it on GitHubhttps://github.com/alexruiz/fest-assert-2.x/issues/121#issuecomment-10261869.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment