Hi Stefan

I do realize that check.sh will be a very good addition. I am very much 
interested in contributing to it.
I will write "make check-build" target and submit patch.

Thanks
Kabeer

------- Original Message -------
Sender : Stefan Schmidt<ste...@osg.samsung.com>
Date : May 08, 2015 12:21 (GMT+05:30)
Title : Re: [E-devel] [EGIT] [core/efl] master 03/55: autotools: enable make 
check per individual modules.

Hello.

On 08/05/15 06:20, Kabeer Ahmed Khan wrote:
>
>     Hi
>
>     On 07/05/15 19:06, stefan schmidt wrote:
>     >Hello.
>     >
>     >On 07/05/15 10:06, kabeer khan wrote:
>     >> cedric pushed a commit to branch master.
>     >>
>     >>
>     
> http://git.enlightenment.org/core/efl.git/commit/?id=35119e7bfdc7c13c2041293
>     f3d0b2ebe1fb7c313
>     >>
>     >> commit 35119e7bfdc7c13c2041293f3d0b2ebe1fb7c313
>     > >Author: kabeer khan
>     > >Date:   Wed Apr 15 16:58:11 2015 +0200
>     >>
>     >>    autotools: enable make check per individual modules.
>     > >
>     > >     Currently make check runs tests of whole EFL.Enabled running
>     > >     of tests of individual modules by make check-
>     > >
>     >
>     >The idea of being able to just check specific areas of code you are
>     >working on is nice.
>     >I have some problems with this implementation though. Some comments 
> below.
>     >>      Signed-off-by: kabeer khan
>     >>     Signed-off-by: Cedric BAIL
>     >> ---
>     >>   Makefile.am                     | 69
>     +++++++++++++++++++++++++++++++++++++++++
>     >>   src/Makefile_Ecore.am           |  5 +++
>     >>   src/Makefile_Ecore_Audio_Cxx.am |  6 ++++
>     >>   src/Makefile_Ecore_Con.am       |  6 ++++
>     >>   src/Makefile_Ecore_Cxx.am       | 12 +++++++
>     >>   src/Makefile_Ector.am           |  6 ++++
>     >>   src/Makefile_Edje.am            |  6 ++++
>     >>   src/Makefile_Edje_Cxx.am        |  6 ++++
>     >>   src/Makefile_Eet.am             |  6 ++++
>     >>   src/Makefile_Eet_Cxx.am         |  7 +++++
>     >>   src/Makefile_Eeze.am            |  7 +++++
>     >>   src/Makefile_Efreet.am          |  6 ++++
>     >>   src/Makefile_Eina.am            |  6 ++++
>     >>   src/Makefile_Eina_Cxx.am        |  6 ++++
>     >>   src/Makefile_Eio.am             |  6 ++++
>     >>   src/Makefile_Eldbus.am          |  6 ++++
>     >>   src/Makefile_Eldbus_Cxx.am      |  6 ++++
>     >>   src/Makefile_Emile.am           |  6 ++++
>     >>   src/Makefile_Eo.am              | 45 +++++++++++++++++++++++++++
>     >>   src/Makefile_Eolian.am          |  6 ++++
>     >>   src/Makefile_Eolian_Cxx.am      |  6 ++++
>     >>   src/Makefile_Evas.am            |  6 ++++
>     >>   src/Makefile_Evas_Cxx.am        |  6 ++++
>     >>  23 files changed, 247 insertions(+)
>     >>
>     >> diff --git a/Makefile.am b/Makefile.am
>     >> index a756511..4bc6def 100644
>     >> --- a/Makefile.am
>     >> +++ b/Makefile.am
>     >> @@ -432,6 +432,75 @@ endif
>     >>   if EFL_ENABLE_COVERAGE
>     >>   @$(MAKE) $(AM_MAKEFLAGS) lcov-report
>     >>   endif
>     >> +
>     >> +check: override DISABLE_SUBTESTS = 1
>     >> +check:
>     >> + ifeq($(DISABLE_SUBTESTS), 1)
>     >> + make check-recursive
>     >> + endif
>     >I stumbled over this when make check failed (jenkins as well as locally
>     >for me). After make check is done I hit this:
>     >
>     
> >===========================================================================
>     =
>     >Testsuite summary for efl 1.14.99.30492
>     
> >===========================================================================
>     =
>     ># TOTAL: 31
>     ># PASS:  31
>     ># SKIP:  0
>     ># XFAIL: 0
>     ># FAIL:  0
>     ># XPASS: 0
>     ># ERROR: 0
>     
> >===========================================================================
>     =
>     >Making check in data
>     >make[1]: Nothing to be done for 'check'.
>     >Making check in doc
>     >Making check in previews
>      >  CC       preview_text_filter.o
>       > CCLD     preview_text_filter
>     >Making check in po
>     >ifeq(1, 1)
>     >/bin/sh: -c: line 0: syntax error near unexpected token `1,'
>     >/bin/sh: -c: line 0: `ifeq(1, 1)'
>     >Makefile:2951: recipe for target 'check' failed
>     >make: *** [check] Error 1
>     >To be honest I do not understand what this tries to do. Setting
>     >DISABLE_SUBTESTS to 1 here will always have the condition being true, or
>     >not?
>     DISABLE_SUBTESTS is just a flag which prevents make check-recursive when
>     test for individual modules is run. This code is failing because rules for
>     make check-preview_text_filter is not defined. Yes, this is a major issue
>     with this approach as new rules needs to be defined for each new module.

Well in the current approach it would have let the check running twice. 
It already did run through but failed the second time with the above 
error message.


>
>     >That means make check will be run twice. I just checked by removing the
>     >if condition.
>     >
>     >> +check-eina:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eina
>     >> +check-eina-cxx:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eina-cxx
>     >> +check-ecore:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-ecore
>     >> +check-ecore-audio-cxx:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-ecore-audio-cxx
>     >> +check-ecore-con:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-ecore-con
>     >> +check-ecore-cxx:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-ecore-cxx
>     >> +check-ecore-cxx-compile:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-ecore-cxx-compile
>     >> +check-ector:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-ector
>     >> +check-edje:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-edje
>     >> +check-edje-cxx:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-edje-cxx
>     >> +check-eet:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eet
>     >> +check-eet-cxx:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eet-cxx
>     >> +check-eeze:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eeze
>     >> +check-efreet:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-efreet
>     >> +check-eio:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eio
>     >> +check-eldbus:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eldbus
>     >> +check-eldbus-cxx:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eldbus-cxx
>     >> +check-emile:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-emile
>     >> +check-eo:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eo
>     >> +check-eo-composite-object:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eo-composite-object
>     >> +check-eo-constructors:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eo-constructors
>     >> +check-eo-function-overrides:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eo-function-overrides
>     >> +check-eo-interface:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eo-interface
>     >> +check-eo-mixin:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eo-mixin
>     >> +check-eo-text-access:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eo-text-access
>     >> +check-eo-signals:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eo-signals
>     >> +check-eo-children:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eo-children
>     >> +check-eolian:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eolian
>     >> +check-eolian-cxx:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-eolian-cxx
>     >> +check-evas:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-evas
>     >> +check-evas-cxx:
>     >> + $(MAKE) $(AM_MAKEFLAGS) -C src check-evas-cxx
>     >>   endif
>     >>
>     >This makes the whole approach quite inflexible. Tom brought this up in a
>     >recent discussion. One have to add these extra check rules for all new
>     >modules, etc.
>     >
>     >His proposal is to have one make rule that compiles all the tests and a
>     >small shell script check.sh which can run the test suites more flexible
>     >by figuring out things by the given parameters. E.g.
>     >
>     >make check-build #compile all test suites
>     >
>     .>/check.sh evas
>     .>/check.sh evas textblock
>     .>/check.sh eo composite
>     >...
>
>     This will give more granularity. I didn't knew that this was planned. I
>     would have known if this was discussed in mailing list.


This was after you already did the implementation and already landed in 
Cedric's next branch. It sparked the discussion. Your work was already 
done at that point and let to some discussion about it. :) These thing 
happen, sometimes one have to write code to throw it away for something 
different. Happened to all of us. Not feeling nice at that point but it 
normally turned out into something better. :)

Tom said he will write the needed check.sh script once we have a make 
target that only builds the test suites but not run them. That would be 
make check-build. Are you interest to write this? If not let me know and 
I will have a look myself.

regards
Stefan Schmidt

------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel
------------------------------------------------------------------------------
One dashboard for servers and applications across Physical-Virtual-Cloud 
Widest out-of-the-box monitoring support with 50+ applications
Performance metrics, stats and reports that give you Actionable Insights
Deep dive visibility with transaction tracing using APM Insight.
http://ad.doubleclick.net/ddm/clk/290420510;117567292;y
_______________________________________________
enlightenment-devel mailing list
enlightenment-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/enlightenment-devel

Reply via email to