diff mbox

[dpdk-dev,3/4] mk: add new test-run make rule

Message ID 20170215152632.25081-3-ferruh.yigit@intel.com (mailing list archive)
State Superseded, archived
Headers show

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Ferruh Yigit Feb. 15, 2017, 3:26 p.m. UTC
Since "make test" and "make test-build" does dependency resolving, they
check for all dependent components (lib and drivers) which takes a few
seconds.

This is a good feature during development, but if the target is just
running unit test, that step is unnecessary, it is possible to compile
onece and run unit test multiple times, without checking any code update

For this purpose, a new make rule "make test-run" added. Which just runs
the unit test, expects that unit test already compiled.

Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 mk/rte.sdkroot.mk | 4 ++--
 mk/rte.sdktest.mk | 4 ++--
 2 files changed, 4 insertions(+), 4 deletions(-)

Comments

Bruce Richardson Feb. 15, 2017, 5:07 p.m. UTC | #1
On Wed, Feb 15, 2017 at 03:26:31PM +0000, Ferruh Yigit wrote:
> Since "make test" and "make test-build" does dependency resolving, they
> check for all dependent components (lib and drivers) which takes a few
> seconds.
> 
> This is a good feature during development, but if the target is just
> running unit test, that step is unnecessary, it is possible to compile
> onece and run unit test multiple times, without checking any code update
> 
> For this purpose, a new make rule "make test-run" added. Which just runs
> the unit test, expects that unit test already compiled.
> 
> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> ---
Sorry, I just don't see the point of having an extra command to maintain
and document for the sake of a few seconds on a unit test run. How long
does the run itself take compared to the time to check dependencies? 

/Bruce
Ferruh Yigit Feb. 15, 2017, 5:41 p.m. UTC | #2
On 2/15/2017 5:07 PM, Bruce Richardson wrote:
> On Wed, Feb 15, 2017 at 03:26:31PM +0000, Ferruh Yigit wrote:
>> Since "make test" and "make test-build" does dependency resolving, they
>> check for all dependent components (lib and drivers) which takes a few
>> seconds.
>>
>> This is a good feature during development, but if the target is just
>> running unit test, that step is unnecessary, it is possible to compile
>> onece and run unit test multiple times, without checking any code update
>>
>> For this purpose, a new make rule "make test-run" added. Which just runs
>> the unit test, expects that unit test already compiled.
>>
>> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
>> ---
> Sorry, I just don't see the point of having an extra command to maintain
> and document for the sake of a few seconds on a unit test run. How long
> does the run itself take compared to the time to check dependencies? 

It is matter of choice, it does not take much time for "make test", but
still I thought it is handy to have a rule only to run the test.

I don't expect much maintenance cost with this, also I don't have strong
opinion to keep it.

> 
> /Bruce
>
Bruce Richardson Feb. 16, 2017, 9:08 a.m. UTC | #3
On Wed, Feb 15, 2017 at 05:41:08PM +0000, Ferruh Yigit wrote:
> On 2/15/2017 5:07 PM, Bruce Richardson wrote:
> > On Wed, Feb 15, 2017 at 03:26:31PM +0000, Ferruh Yigit wrote:
> >> Since "make test" and "make test-build" does dependency resolving, they
> >> check for all dependent components (lib and drivers) which takes a few
> >> seconds.
> >>
> >> This is a good feature during development, but if the target is just
> >> running unit test, that step is unnecessary, it is possible to compile
> >> onece and run unit test multiple times, without checking any code update
> >>
> >> For this purpose, a new make rule "make test-run" added. Which just runs
> >> the unit test, expects that unit test already compiled.
> >>
> >> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com>
> >> ---
> > Sorry, I just don't see the point of having an extra command to maintain
> > and document for the sake of a few seconds on a unit test run. How long
> > does the run itself take compared to the time to check dependencies? 
> 
> It is matter of choice, it does not take much time for "make test", but
> still I thought it is handy to have a rule only to run the test.
> 
> I don't expect much maintenance cost with this, also I don't have strong
> opinion to keep it.
> 
Yes, I suppose it isn't much maintenance cost indeed, so if nobody else
cares, I'm ok with it too.

/Bruce
Thomas Monjalon Feb. 16, 2017, 9:26 a.m. UTC | #4
2017-02-15 15:26, Ferruh Yigit:
> --- a/mk/rte.sdkroot.mk
> +++ b/mk/rte.sdkroot.mk
> @@ -92,8 +92,8 @@ default: all
>  config showconfigs showversion showversionum:
>  	$(Q)$(MAKE) -f $(RTE_SDK)/mk/rte.sdkconfig.mk $@
>  
> -.PHONY: test fast_test ring_test mempool_test perf_test coverage
> -test fast_test ring_test mempool_test perf_test coverage:
> +.PHONY: test test-run fast_test ring_test mempool_test perf_test coverage
> +test test-run fast_test ring_test mempool_test perf_test coverage:
>  	$(Q)$(MAKE) -f $(RTE_SDK)/mk/rte.sdktest.mk $@

"test" is a shortcut for build + basic tests.
I think test-run can be better named. It runs all the basic tests registered
in autotests. "test-all" would be wrong. What about "test-basic"?
Ferruh Yigit Feb. 16, 2017, 10:21 a.m. UTC | #5
On 2/16/2017 9:26 AM, Thomas Monjalon wrote:
> 2017-02-15 15:26, Ferruh Yigit:
>> --- a/mk/rte.sdkroot.mk
>> +++ b/mk/rte.sdkroot.mk
>> @@ -92,8 +92,8 @@ default: all
>>  config showconfigs showversion showversionum:
>>  	$(Q)$(MAKE) -f $(RTE_SDK)/mk/rte.sdkconfig.mk $@
>>  
>> -.PHONY: test fast_test ring_test mempool_test perf_test coverage
>> -test fast_test ring_test mempool_test perf_test coverage:
>> +.PHONY: test test-run fast_test ring_test mempool_test perf_test coverage
>> +test test-run fast_test ring_test mempool_test perf_test coverage:
>>  	$(Q)$(MAKE) -f $(RTE_SDK)/mk/rte.sdktest.mk $@
> 
> "test" is a shortcut for build + basic tests.
> I think test-run can be better named. It runs all the basic tests registered
> in autotests. "test-all" would be wrong. What about "test-basic"?

Right, "test-basic" matches more to existing rules (test-fast,
test-perf, ..), all has hidden "run" action implied, I will use
"test-basic" if there is no objection.

My concerns is "test-basic" "running basic tests without compilation"
may not be obvious for a newcomer.
Should I add a line to "make help" for "test" and "test-*" rules?

Like:
test      Compile tests and run basic unit tests
test-*    Run specific subset of the unit tests

thanks,
ferruh
Thomas Monjalon Feb. 16, 2017, 10:44 a.m. UTC | #6
2017-02-16 10:21, Ferruh Yigit:
> On 2/16/2017 9:26 AM, Thomas Monjalon wrote:
> > 2017-02-15 15:26, Ferruh Yigit:
> >> --- a/mk/rte.sdkroot.mk
> >> +++ b/mk/rte.sdkroot.mk
> >> @@ -92,8 +92,8 @@ default: all
> >>  config showconfigs showversion showversionum:
> >>  	$(Q)$(MAKE) -f $(RTE_SDK)/mk/rte.sdkconfig.mk $@
> >>  
> >> -.PHONY: test fast_test ring_test mempool_test perf_test coverage
> >> -test fast_test ring_test mempool_test perf_test coverage:
> >> +.PHONY: test test-run fast_test ring_test mempool_test perf_test coverage
> >> +test test-run fast_test ring_test mempool_test perf_test coverage:
> >>  	$(Q)$(MAKE) -f $(RTE_SDK)/mk/rte.sdktest.mk $@
> > 
> > "test" is a shortcut for build + basic tests.
> > I think test-run can be better named. It runs all the basic tests registered
> > in autotests. "test-all" would be wrong. What about "test-basic"?
> 
> Right, "test-basic" matches more to existing rules (test-fast,
> test-perf, ..), all has hidden "run" action implied, I will use
> "test-basic" if there is no objection.
> 
> My concerns is "test-basic" "running basic tests without compilation"
> may not be obvious for a newcomer.
> Should I add a line to "make help" for "test" and "test-*" rules?
> 
> Like:
> test      Compile tests and run basic unit tests
> test-*    Run specific subset of the unit tests

Yes it looks good.
diff mbox

Patch

diff --git a/mk/rte.sdkroot.mk b/mk/rte.sdkroot.mk
index 9c23987..0eb53fc 100644
--- a/mk/rte.sdkroot.mk
+++ b/mk/rte.sdkroot.mk
@@ -92,8 +92,8 @@  default: all
 config showconfigs showversion showversionum:
 	$(Q)$(MAKE) -f $(RTE_SDK)/mk/rte.sdkconfig.mk $@
 
-.PHONY: test fast_test ring_test mempool_test perf_test coverage
-test fast_test ring_test mempool_test perf_test coverage:
+.PHONY: test test-run fast_test ring_test mempool_test perf_test coverage
+test test-run fast_test ring_test mempool_test perf_test coverage:
 	$(Q)$(MAKE) -f $(RTE_SDK)/mk/rte.sdktest.mk $@
 
 test: test-build
diff --git a/mk/rte.sdktest.mk b/mk/rte.sdktest.mk
index 1cdb40b..6455b3e 100644
--- a/mk/rte.sdktest.mk
+++ b/mk/rte.sdktest.mk
@@ -46,14 +46,14 @@  DIR := $(shell basename $(RTE_OUTPUT))
 #
 # test: launch auto-tests, very simple for now.
 #
-.PHONY: test fast_test perf_test coverage
+.PHONY: test test-run fast_test perf_test coverage
 
 PERFLIST=ring_perf,mempool_perf,memcpy_perf,hash_perf,timer_perf
 coverage: BLACKLIST=-$(PERFLIST)
 fast_test: BLACKLIST=-$(PERFLIST)
 perf_test: WHITELIST=$(PERFLIST)
 
-test fast_test perf_test:
+test test-run fast_test perf_test:
 	@mkdir -p $(AUTOTEST_DIR) ; \
 	cd $(AUTOTEST_DIR) ; \
 	if [ -f $(RTE_OUTPUT)/app/test ]; then \