Re: [LTP] [PATCH v3 2/4] Add support for mixing C and shell code

[Petr Vorel <pvorel@suse.cz>]

Hi Cyril,

...
> diff --git a/testcases/lib/run_tests.sh b/testcases/lib/run_tests.sh
I hoped testcases/lib/run_tests.sh could be run in CI (and manually).

We have in the toplevel Makefile other test targets, could we add
testcases/lib/run_tests.sh to test-shell target?

1) Add test target to testcases/lib/tests/Makefile (similar to
metadata/tests/Makefile).

test:
	@echo == Run C - shell integration tests ===
	@./run_tests.sh

2) Add test target to testcases/lib/Makefile which would call test target in the
test/ directory (similar to metadata/Makefile).

test:
	$(MAKE) -C $(abs_srcdir)/tests/ test

3) Add testing to the toplevel test-shell target + build dependency to testcases/lib/.

test-shell: lib-all
ifneq ($(build),$(host))
	$(error running tests on cross-compile build not supported)
endif
	$(call _test,-s)
	$(MAKE) -C $(abs_top_builddir)/testcases/lib
	$(MAKE) -C $(abs_top_srcdir)/testcases/lib test

> --- /dev/null
> +++ b/testcases/lib/run_tests.sh
> @@ -0,0 +1,11 @@
> +#!/bin/sh
> +
> +testdir=$(realpath $(dirname $0))
Not sure if there should be some quotes.

> +export PATH=$PATH:$testdir:$testdir/tests/
Also quotes here.

...
> diff --git a/testcases/lib/tests/Makefile b/testcases/lib/tests/Makefile
> new file mode 100644
> index 000000000..5a5cf5310
> --- /dev/null
> +++ b/testcases/lib/tests/Makefile
> @@ -0,0 +1,11 @@
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (C) 2009, Cisco Systems Inc.
nit: Maybe LTP copyright? (copy paste error)
> +# Ngie Cooper, August 2009
nit: and you, or just replace Ngie?
> +
> +top_srcdir		?= ../../..
> +
> +include $(top_srcdir)/include/mk/testcases.mk
> +
> +INSTALL_TARGETS=
> +
> +include $(top_srcdir)/include/mk/generic_leaf_target.mk

The rest LGTM, but I'll have a deeper look today or tomorrow and try to play
with it.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp