Re: StreamsEqual issue(s)

Hi Gert, 

> > > I also noticed NUnit is changing the position in the 
> stream (to the 
> > > start of the stream), and doesn't change it back 
> afterwards. Should 
> > > an Assert by itself be allowed to change the state of a stream?
> >
> > In some cases, NUnit actually creates the stream by opening a file. 
> > But in others, it may be passed in as an argument.
> > Theoretically, we should not change it or at least document that it 
> > may be changed. I'll look at it.
> 
> It the stream is both readable and seekable, then I think 
> NUnit should store the initial position, and reposition back 
> at that position after comparing the content of the stream.

That makes sense - I'll put it into the head anyway.
 
> > > I'm also not sure if NUnit should actually read these 
> streams. What 
> > > will you do if the stream is readable and not seekable? 
> You'd change 
> > > the position in the stream (by positioning at the 
> beginning and by 
> > > reading it), and you can't change it back to the origin position.
> >
> > I can't think of any good use case for wanting to assert on the 
> > equality of two non-seekable streams. Can you? Maybe I should just 
> > throw an InvalidArgumentException.
> 
> No, but if you aim for backward compatibility then this is 
> not the best approach.

My logic so far has been that it's OK to change behavior if it
only impacts something that should not have been acceptable in
the first place. Since NUnit is a /test/ framework, and isn't
(normally) shipping as part of the software being tested, pointing
out a problem is a good thing.

> But well, the change from Stream.Equals to actually comparing 
> the content of the streams isn't either.
> 
> I guess this does not apply to only Streams. I leave it up to 
> your good judgement to decide which approach to take.

In fact, it applies anyplace where we have added new behavior
to AreEqual(object, object). There's a tradeoff but I think it's
largely in favor of making such changes when they are useful
because,

1) They don't generally break anything but a test for reference
equality.

2) There's a preferred way to do that.

> > > Apart from this, I wondered why NUnit is not always checking for 
> > > reference equality first? Why bothering checking the 
> content of two 
> > > streams, it they are actually the same?
> >
> > But why is the user bothering to use the Assert in that case? 
> > He should be using AreSame rather than AreEqual. OTOH, I 
> can imagine 
> > odd cases where it would not be possible to know in 
> advance, so it's 
> > worth a check.
> 
> He should indeed be using AreSame, but this specific test 
> even dated from before Assert.AreSame existed (the fixture 
> derived from Assertion, and used AssertEquals to compare the 
> streams). There are actually quite a few tests in Mono that 
> use this (long-time) deprecated construct.

So maybe the first patch is to upgrade the tests to NUnit 2.2. :-)

> > I wasn't planning to do any more development in the 2.4 
> code line, but 
> > I guess I could do a fix for mono if there turn out to be 
> any issues 
> > that require it. Alternatively, you may want to look at 2.5, which 
> > will get active maintenance after it is released.
> 
> I don't think you should bother fixing this for 2.4. I'll 
> even need to check with Miguel if he wants to consider 
> upgrading NUnit.

I suggest running your same tests with the 2.5 alpha to see if
there are any further problems. I suspect there will not be, but
I can easily fix any incompatibilities there.

In fact, fixing anything that made it easier to use NUnit in
Mono would be a big priority.

BTW, do you think I should have some sort of install for linux?
Up to now it has been totally left to the packagers.

Charlie
 
> Perhaps he prefers to keep things as is.
> 
> Gert
> 
> 



-------------------------------------------------------------------------
This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
Build the coolest Linux based applications with Moblin SDK & win great prizes
Grand prize is a trip for two to an Open Source event anywhere in the world
http://moblin-contest.org/redirect.php?banner_id=100&url=/