Subject: Re: bug#76446: [Patch Debbugs v3] Infrastructure Improvements

From: Morgan Smith <[email-masked]>
Date: Tue, 25 Feb 2025 13:17:52 -0500

Michael Albinus <[email-masked]> writes:

> Morgan Smith <[email-masked]> writes:
>
> Hi Morgan,
>
>> I'm still not 100% sure what the ideal way to send in patches is.  Here
>> is the entire v3 patch series
>
> This is fine. Since most of the changes are not controversial, I've
> committed all your patches to the debbugs repo (plus some very minor
> cleanups from me).

I am still curious.  The guix people seem to prefer 'git send-email' but
I was told not to do that by some emacs devel people.

> The last point I like to discuss is the Makefile. Attached is a patch I
> propose. It changes the handling of the tests more like Emacs does. Now
> we have different calls:
>
> --8<---------------cut here---------------start------------->8---
> # make check
> # make debbugs-tests
> # make debbugs-gnu-tests
> --8<---------------cut here---------------end--------------->8---
>
> What's missing is the handling of SELECTORs like Emacs does. This we
> could add later.
>
> WDYT?

I started looking at what emacs does.  It looks complicated.  I don't
entirely understand what they are doing.  However, I would argue that
what is currently in the debbugs makefile is quite powerful and I would
be interested in what use cases your solution provides.

Are you trying to improve performance of running the tests?  Here are the
performance numbers for running the current tests on my laptop:

make clean
time make build
0.590s
time make check
0.472s
time make check TESTS=get-status
0.148s
time make check TESTS=no-tests
0.122s
time emacs -Q --batch
0.033s

As we can see, if we only selectivly load the test files instead of loading all
of the test files, we can maybe save 1/10 of a second.  I personally am not
that worried about this.

Are you trying to selectively run tests so we can debug tests one at a time?
That is already entirely doable with the current setup.  Running 'make check
TESTS=get-status' only runs a singular test.

The patch you have provided actually removes the ability to run one test at a
time as we'd have to run entire files.

I'm not certain what the SELECTORs do but perhaps you're thinking that
in the future we would have groups of tests (like "expensive" or
"network") that we could select.  In the interest of not
over-engineering on hypotheticals, I would say that is a future problem.
One that I would likely solve by putting those test files in
sub-directories labeled "expensive" and "network".

>
> Best regards, Michael.
>
> diff --git a/Makefile b/Makefile
> index da0764ea36..bb69ebe9da 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1,13 +1,11 @@
>  EMACS ?= emacs
>  MAKEINFO ?= makeinfo
>
> -# regex of tests to run
> -TESTS=.*
> -
>  SOURCE=$(wildcard *.el)
>  TESTSOURCE=$(wildcard test/*.el)
>  TARGET=$(filter-out debbugs-pkg.elc,$(patsubst %.el,%.elc,$(SOURCE)))
>  TESTTARGET=$(patsubst %.el,%.elc,$(TESTSOURCE))
> +TESTS=$(patsubst test/%.el,%,$(wildcard test/*-tests.el))
>
>  INFOMANUALS=debbugs.info debbugs-ug.info
>
> @@ -30,10 +28,14 @@ doc: $(INFOMANUALS)
>  build: $(TARGET)
>
>  checkdoc: $(SOURCE) $(TESTSOURCE)
> -	@$(EMACS) -Q --batch -l resources/debbugs-checkdoc-config.el $(foreach file,$^,"--eval=(checkdoc-file \"$(file)\")")
> +	@$(EMACS) -Q --batch -l resources/debbugs-checkdoc-config.el \
> +	  $(foreach file,$^,"--eval=(checkdoc-file \"$(file)\")")
> +
> +check: $(TESTS)
>
> -check: build $(TESTTARGET)
> -	@$(EMACS) -Q --batch -L . -L ./test $(foreach file,$(TESTSOURCE), -l $(file)) --eval '(ert-run-tests-batch-and-exit "$(TESTS)")'
> +%-tests: build $(TESTTARGET)
> +	@$(EMACS) -Q --batch -L . -L ./test -l $@ \
> +	  --eval '(ert-run-tests-batch-and-exit t)'
>
>  clean:
>  	-rm -f $(TARGET) $(TESTTARGET) $(INFOMANUALS)