make check should include the unit test cases about ginger HOT 5 CLOSED

Actually 'make check-local' just verifies code integrity (pep8 and stuff) if you're considering Kimchi as a base. It is 'make check' that should run the code integrity check of check-local and the unit tests too.

from ginger.

Daniel,

I just cross checked, make check and make check-local both execute the test cases in case of wok framework.and kimchi plugin.

bash-4.3# pwd
/home/Abhiram18/kimchi/tests

content of Makefile.am has
check-local:
$(MKDIR_P) $(top_srcdir)/data/screenshots
./run_tests.sh

On the same lines i was assuming i had to update the Makefile.am in ginger tests(ginger/tests/makefile.am) with below content,
check-local:
$(MKDIR_P) $(top_srcdir)/data/screenshots
./run_tests.sh

Let me know if this is fine?

from ginger.

We're talking about different files. I was talking about make check-local at project root. For example, in Kimchi:

[danielhb@arthas kimchi]$ make check-local
contrib/check_i18n.py ./i18n.py 
Checking for invalid i18n string...
Checking for invalid i18n string successfully
find . -path './.git' -prune -type f -o \
    -name '*.py' -o -name '*.py.in'  | xargs /usr/bin/pyflakes | \
    while read LINE; do echo "$LINE"; false; done
/usr/bin/pep8 --version
1.6.2
/usr/bin/pep8 --filename '*.py,*.py.in' --exclude="*config.py,*i18n.py,*tests/test_config.py" .
[danielhb@arthas kimchi]$  

Note that the tests aren't run after firing "make check-local" at Kimchi root. Same thing with wok.

However, I just verified that the change you've proposed (without the /data/screenshots line because we do not have that in Ginger) in tests/Makefile.am allows 'make check' from Ginger root to run the unit tests as well. So go ahead and make the change. Pay attention to the TAB indentation at Makefile.am, otherwise it will fail.

And, if I may suggest, you can enhance the 'check-local' target at Ginger root to do the same steps as Kimchi/Wok. As far as I just checked here it is simply a matter of adding the line

contrib/check_i18n.py $(I18N_FILES)

At the start of the check-local target at Makefile.am of Ginger root (disclaimer: I've not tested it, just speculating). You can add this change in this same patch/issue if you like.

Daniel

from ginger.

@abhiramkulkarni let me know if you're still interested in doing this enhancement

from ginger.

Will be working on this issue in coming weeks

from ginger.