Created attachment 136630 [details]
Set of patches

These patches set the test suite to a working state again.

The following has been changed:
 - Some old cruft removed
 - Tests adapted to Gstreamer# api changes
 - Removed ConsoleUi.cs and adapted the Makefile to use nunit-console.

With these patches there are three failing tests. Two of these were real crashers that prevented nunit from reporting the results, so I disabled those with the [Ignore] attribute.

- PadTest.TestPushUnlinked
   Fails with a System.AccessViolationException.
   This might be related to the part #585551 that isn't yet solved, 
   as it also throws this exception.

- PadTest.TestFuncAssigning
   This crashes without any further information.

- PipelineTest.TestBusAddWatch
   This is the same crash as #585541.

Some comments:

0001 - You remove some test cases but they do more than just testing the
       refcounts (i.e. TestAddRemovePad)
     - Some tests were commented out before, maybe we can activate them again
       and remove the refcount checks and they're useful ;)

0002 - I'm not sure but maybe the Dispose() calls could expose
       owernship/refcount issues because we might dispose an instance
       that we don't own anymore. Or does nunit make sure that the GC is
       called at the end of every test?

0003 - The Bin test that iterates over the children could also use the
       Elements, etc properties that provide an IEnumerable instead of using
       the ChildProxy interface. But this is of course fine too :)

0005 - What exactly is the problem with the Application.Deinit() calls?

And a general comment, IMHO we shouldn't ignore crashing tests as this way they might be forgotten :)

Good work otherwise... I'll handle the comments above unless you want to provide a new patch :)

Ok, they're all committed now :)

Arg, a mid-air collision.

Anyway, below is my response to your comments.
I was planning on making a new patch series, but if you applied my patches as they where (they don't show up on fdo yet), I can address your issues with a separate follow-up patch.

Maarten

================================================================================

0001
  Yeah, I removed test quite liberally.
  I suppose I can rescue some of what remains after removing the Refcount calls 
  by adding a check for the number of pads on an element, for instance.
  Btw, is there a specific reason the Element.Numpads property is gone now?

0002
  I just figured that the tests should be written the same as one would write 
  an application, i.e. without the need for explicit cleanup.
  I'm not very familiar with the internals of nunit, so I don't know whether or
  not it GCs after every test.

0003
  Yes, that certainly is the more cleaner way of accessing the child elements 
  of a bin. In general however, I have the feeling that this tests depends to 
  much on the implentation details of GstBin, or is the order of child elements 
  always garanteed to be the same as the order in which they were added?

0005
  This could very well be another idiosyncrasy of Windows, but after the first 
  DeInit, all subsequent Init()s failed, causing a lot of errors due to a 
  non-initialized gstreamer. I figured it wouldn't be really nescessary to have 
  a DeInit after every test, so I removed it. But I should make a test case out 
  of it and file a bug, if appropriate.

Well, marking the tests with the IgnoreAttribute is preferred above commenting
them out of course. Note that on my machine these two tests not only failed,
but also caused the entire test suite to crash. By ignoring them I at least get
a summary of the results, which shows the ignored tests as well, so they can't
be forgotten.

(In reply to comment #5)
> Arg, a mid-air collision.
> 
> Anyway, below is my response to your comments.
> I was planning on making a new patch series, but if you applied my patches as
> they where (they don't show up on fdo yet), I can address your issues with a
> separate follow-up patch.

Great, I've pushed all changes now :)
 ================================================================================
> 
> 0001
>   Yeah, I removed test quite liberally.
>   I suppose I can rescue some of what remains after removing the Refcount calls 
>   by adding a check for the number of pads on an element, for instance.
>   Btw, is there a specific reason the Element.Numpads property is gone now?

Ok, would be great if you could attach a new patch which adds all those tests again then :)

There's no specific reason but I've never used this in C or C# :) If you think the numpads, numsinkpads, numsrcpads fields of the GstElement instance struct are useful just file a bug with a patch to add them as readonly properties.

> 0002
>   I just figured that the tests should be written the same as one would write 
>   an application, i.e. without the need for explicit cleanup.
>   I'm not very familiar with the internals of nunit, so I don't know whether or
>   not it GCs after every test.

Well, let's keep it as is then (without Dispose() calls).

> 0003
>   Yes, that certainly is the more cleaner way of accessing the child elements 
>   of a bin. In general however, I have the feeling that this tests depends to 
>   much on the implentation details of GstBin, or is the order of child elements 
>   always garanteed to be the same as the order in which they were added?

Not garantueed by the API but currently it will always be the case ;) I guess that patch should be changed a bit then...

> 0005
>   This could very well be another idiosyncrasy of Windows, but after the first 
>   DeInit, all subsequent Init()s failed, causing a lot of errors due to a 
>   non-initialized gstreamer. I figured it wouldn't be really nescessary to have 
>   a DeInit after every test, so I removed it. But I should make a test case out 
>   of it and file a bug, if appropriate.

Oh, that's intentional. I thought nunit calls every test in a separate process like check does. After DeInit() you must not call Init() again. DeInit() is just for cleaning everything up before the process exits (and is IMHO absolutely useless).

> Well, marking the tests with the IgnoreAttribute is preferred above commenting
> them out of course. Note that on my machine these two tests not only failed,
> but also caused the entire test suite to crash. By ignoring them I at least get
> a summary of the results, which shows the ignored tests as well, so they can't
> be forgotten.

Ok, fine by me then :) I've enabled all tests now though and none of them is crashing for me now (after some fixes). If anything breaks for you please file bugs with backtraces, etc :)

See http://www.mono-project.com/Debugging