On Tue, 3 May 2016 11:19:35 +0200 Stefan Schmidt <[email protected]> said:

> Hello.
> 
> On 03/05/16 09:19, Stefan Schmidt wrote:
> > Hello.
> >
> > On 02/05/16 21:49, Larry de Oliveira Lira Jr wrote:
> >> Hi
> >> I think you don't have a dbus-daemon running, in the current
> >> eldbus-test (eldbus_test_eldbus_init.c) I found:
> >>
> > I have it running and the eldbus test cases worked fine before your
> > promise change. After your change they crash.
> >
> > Marcel told me on IRC that he had a look at the backtrace of the crash
> > and ecore_main_loop_begin pointed to 0x0 because ecore was not linked
> > correctly.
> >
> >>   57 /* TODO: This test assumes an existing dbus-daemon running. It
> >>   58  * shouldn't do this, instead we should launch dbus-daemon ourselves
> >>   59  * and create our private instance, use it and then kill it
> >>   60  * afterwards.
> >>   61  */
> >>
> >> and almost of eldbus test is disabled/commented
> >>
> >> I think the best is deactivate this eldbus tests added by me, until I
> >> found a best solution
> >>
> > You did not add new tests but changed some that already have been there
> > and are now longer working now. Deactivating them is no solution. Please
> > get this fixed.
> >
> > This is not even all this commit broke. You use ecore_promise_then and
> > ecore_promise_value_get here while these function do not exist (its
> > eina_ not ecore_) and the compiler happily warns you about it...
> >
> > Please have a look and get these whole thing fixed. I feel close to
> > reverting it but I want to give a chance first to get it sorted out.
> 
> I just had to discover that this very patch also disabled the test cases 
> for eio!
> 
> Disabling tests for getting new stuff in is not acceptable. I get more 
> and more the feeling that this patch was put in to early.
> 
> Felipe, if you push things for other people you should do a full review 
> and push back on shortcuts.
> 
> /me starts to get grumpy as is commit pops up on every strange thing I 
> looked at today.

you're right stefan. disabling just to get new stuff in is wrong. we've had
problems with tests where on one machine they always succeed and on another
they fail randomly and we can't figure it out because they work for developers.
these are just cases we have to live with until we finally find out why...

but disabling tests just to avoid fixing something is wrong.

i get is that new things come in and don't have tests yet - they may never have
tests because of their nature (because the tests are insanely horrible to write
or run). that's ok. not brilliant but ok. turning off tests that we have ...
unless the tests themselves just were invalid/wrong and unable to be sanely
run/tested... is not good.

> regards
> Stefan Schmidt
> 
> ------------------------------------------------------------------------------
> Find and fix application performance issues faster with Applications Manager
> Applications Manager provides deep performance insights into multiple tiers of
> your business applications. It resolves application problems quickly and
> reduces your MTTR. Get your free trial!
> https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
> _______________________________________________
> enlightenment-devel mailing list
> [email protected]
> https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
> 


-- 
------------- Codito, ergo sum - "I code, therefore I am" --------------
The Rasterman (Carsten Haitzler)    [email protected]


------------------------------------------------------------------------------
Find and fix application performance issues faster with Applications Manager
Applications Manager provides deep performance insights into multiple tiers of
your business applications. It resolves application problems quickly and
reduces your MTTR. Get your free trial!
https://ad.doubleclick.net/ddm/clk/302982198;130105516;z
_______________________________________________
enlightenment-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to