Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

make check should include the unit test cases #31

Closed
abhiramkulkarni opened this issue Oct 28, 2015 · 5 comments
Closed

make check should include the unit test cases #31

abhiramkulkarni opened this issue Oct 28, 2015 · 5 comments
Assignees
Milestone

Comments

@abhiramkulkarni
Copy link
Member

Whenever make check-local is executed , the unit test cases also should be executed.
This helps in doing some basic test case execution for any patch being proposed.

@danielhb
Copy link
Contributor

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.

@danielhb danielhb changed the title make check-local should include the unit test cases make check should include the unit test cases Oct 29, 2015
@abhiramkulkarni
Copy link
Member Author

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?

@danielhb
Copy link
Contributor

danielhb commented Nov 3, 2015

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

@danielhb
Copy link
Contributor

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

@danielhb danielhb added this to the Backlog milestone Dec 24, 2015
@abhiramkulkarni
Copy link
Member Author

Will be working on this issue in coming weeks

@danielhb danielhb assigned danielhb and unassigned abhiramkulkarni Jul 7, 2016
@danielhb danielhb modified the milestones: Ginger 2.3, Backlog Jul 7, 2016
danielhb added a commit that referenced this issue Jul 8, 2016
This patch adds the 'check-local' target in tests/Makefile.am
to allow the unit tests to be ran when issuing 'make check'
from Ginger root.

Signed-off-by: Daniel Henrique Barboza <danielhb@linux.vnet.ibm.com>
@danielhb danielhb closed this as completed Jul 8, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants