* Re: [Emacs-diffs] emacs-25 f5c762c: Additional changes for "make check-expensive" [not found] ` <E1aGNHT-0007u5-Su@vcs.savannah.gnu.org> @ 2016-01-12 17:43 ` Phillip Lord 2016-01-12 17:49 ` John Wiegley 2016-01-12 19:03 ` Michael Albinus 0 siblings, 2 replies; 8+ messages in thread From: Phillip Lord @ 2016-01-12 17:43 UTC (permalink / raw) To: emacs-devel; +Cc: Michael Albinus Michael Albinus <Michael.Albinus@gmx.de> writes: > diff --git a/test/automated/Makefile.in b/test/automated/Makefile.in > index 43e3905..48920ef 100644 > --- a/test/automated/Makefile.in > +++ b/test/automated/Makefile.in > @@ -87,9 +87,9 @@ WRITE_LOG = > $@ 2>&1 || { stat=ERROR; cat $@; }; echo $$stat: $@ > ## to change this; bug#17848 - if that gets done, this can be simplified). > ## > ## Beware: it approximates 'no-byte-compile', so watch out for false-positives! > -SELECTOR_DEFAULT=(not (tag :expensive-test)) > +SELECTOR_DEFAULT=(quote (not (tag :expensive-test))) > SELECTOR_EXPENSIVE=nil > -SELECTOR=${SELECTOR_DEFAULT} > +SELECTOR= Michael I think that there is a problem with this commit, in that the default selector is only used for "make check". By default "make check-maybe" runs all tests (including the expensive ones). So, you have to do make check-maybe SELECTOR="(quote (not (tag :expensive-test)))" I think it makes more sense for check-maybe to skip expensive tests, unless told otherwise, as "make check-maybe" is a good candidate for use pre-commit. Something like this should work: SELECTOR_DEFAULT=(quote (not (tag :expensive-test))) SELECTOR_EXPENSIVE=nil SELECTOR= ifeq ($(SELECTOR),undefined) SELECTOR_ACTUAL=$(SELECTOR_DEFAULT) else SELECTOR_ACTUAL=$(SELECTOR) endif (and changing all subsequent uses of SELECTOR to SELECTOR_ACTUAL). which would preserve the use of "SELECTOR" for use on the command line. Make sense? (er, pun not intended). Phil ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Emacs-diffs] emacs-25 f5c762c: Additional changes for "make check-expensive" 2016-01-12 17:43 ` [Emacs-diffs] emacs-25 f5c762c: Additional changes for "make check-expensive" Phillip Lord @ 2016-01-12 17:49 ` John Wiegley 2016-01-12 18:55 ` Michael Albinus 2016-01-12 19:03 ` Michael Albinus 1 sibling, 1 reply; 8+ messages in thread From: John Wiegley @ 2016-01-12 17:49 UTC (permalink / raw) To: Phillip Lord; +Cc: Michael Albinus, emacs-devel [-- Attachment #1: Type: text/plain, Size: 627 bytes --] >>>>> Phillip Lord <phillip.lord@russet.org.uk> writes: > I think that there is a problem with this commit, in that the default > selector is only used for "make check". By default "make check-maybe" runs > all tests (including the expensive ones). So, you have to do Hi Phillip, we need to get all of this straightened out, let me know so that I can reflect the same changes into master, in order to repair what was lost of this work during yesterdays' merge. -- John Wiegley GPG fingerprint = 4710 CF98 AF9B 327B B80F http://newartisans.com 60E1 46C4 BD1A 7AC1 4BA2 [-- Attachment #2: signature.asc --] [-- Type: application/pgp-signature, Size: 629 bytes --] ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Emacs-diffs] emacs-25 f5c762c: Additional changes for "make check-expensive" 2016-01-12 17:49 ` John Wiegley @ 2016-01-12 18:55 ` Michael Albinus 2016-01-14 22:15 ` Phillip Lord 0 siblings, 1 reply; 8+ messages in thread From: Michael Albinus @ 2016-01-12 18:55 UTC (permalink / raw) To: Phillip Lord; +Cc: emacs-devel John Wiegley <jwiegley@gmail.com> writes: > Hi Phillip, we need to get all of this straightened out, let me know so that I > can reflect the same changes into master, in order to repair what was lost of > this work during yesterdays' merge. I believe Philip is not speaking about a problem of the merge from yesterday. It's about my original commit to the emacs-25 branch. Best regards, Michael. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Emacs-diffs] emacs-25 f5c762c: Additional changes for "make check-expensive" 2016-01-12 18:55 ` Michael Albinus @ 2016-01-14 22:15 ` Phillip Lord 0 siblings, 0 replies; 8+ messages in thread From: Phillip Lord @ 2016-01-14 22:15 UTC (permalink / raw) To: Michael Albinus; +Cc: emacs-devel Michael Albinus <michael.albinus@gmx.de> writes: > John Wiegley <jwiegley@gmail.com> writes: > >> Hi Phillip, we need to get all of this straightened out, let me know so that I >> can reflect the same changes into master, in order to repair what was lost of >> this work during yesterdays' merge. > > I believe Philip is not speaking about a problem of the merge from > yesterday. It's about my original commit to the emacs-25 branch. Yep, no worries about the merge. Michael's change went in a while back. Phil ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Emacs-diffs] emacs-25 f5c762c: Additional changes for "make check-expensive" 2016-01-12 17:43 ` [Emacs-diffs] emacs-25 f5c762c: Additional changes for "make check-expensive" Phillip Lord 2016-01-12 17:49 ` John Wiegley @ 2016-01-12 19:03 ` Michael Albinus 2016-01-14 8:13 ` Michael Albinus 2016-01-14 23:04 ` Phillip Lord 1 sibling, 2 replies; 8+ messages in thread From: Michael Albinus @ 2016-01-12 19:03 UTC (permalink / raw) To: Phillip Lord; +Cc: emacs-devel phillip.lord@russet.org.uk (Phillip Lord) writes: > Michael Hi Philip, > I think that there is a problem with this commit, in that the default > selector is only used for "make check". By default "make check-maybe" > runs all tests (including the expensive ones). So, you have to do > > make check-maybe SELECTOR="(quote (not (tag :expensive-test)))" > > I think it makes more sense for check-maybe to skip expensive tests, > unless told otherwise, as "make check-maybe" is a good candidate for use > pre-commit. I see. Before touching the Makefile, we shall agree how all the targets shall behave. I would say, that "check" and "check-maybe" shall skip the expensive tests. "check-expensive", "<foo>", and "<foo>.log" shall run all tests. > Phil Best regards, Michael. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Emacs-diffs] emacs-25 f5c762c: Additional changes for "make check-expensive" 2016-01-12 19:03 ` Michael Albinus @ 2016-01-14 8:13 ` Michael Albinus 2016-01-14 23:04 ` Phillip Lord 1 sibling, 0 replies; 8+ messages in thread From: Michael Albinus @ 2016-01-14 8:13 UTC (permalink / raw) To: Phillip Lord; +Cc: emacs-devel Michael Albinus <michael.albinus@gmx.de> writes: > I see. Before touching the Makefile, we shall agree how all the targets > shall behave. I would say, that "check" and "check-maybe" shall skip the > expensive tests. "check-expensive", "<foo>", and "<foo>.log" shall run > all tests. I've committed the patch to the emacs-25 branch. Best regards, Michael. ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Emacs-diffs] emacs-25 f5c762c: Additional changes for "make check-expensive" 2016-01-12 19:03 ` Michael Albinus 2016-01-14 8:13 ` Michael Albinus @ 2016-01-14 23:04 ` Phillip Lord 2016-01-15 8:45 ` Michael Albinus 1 sibling, 1 reply; 8+ messages in thread From: Phillip Lord @ 2016-01-14 23:04 UTC (permalink / raw) To: Michael Albinus; +Cc: emacs-devel Michael Albinus <michael.albinus@gmx.de> writes: > phillip.lord@russet.org.uk (Phillip Lord) writes: > >> Michael > > Hi Philip, > >> I think that there is a problem with this commit, in that the default >> selector is only used for "make check". By default "make check-maybe" >> runs all tests (including the expensive ones). So, you have to do >> >> make check-maybe SELECTOR="(quote (not (tag :expensive-test)))" >> >> I think it makes more sense for check-maybe to skip expensive tests, >> unless told otherwise, as "make check-maybe" is a good candidate for use >> pre-commit. > > I see. Before touching the Makefile, we shall agree how all the targets > shall behave. I would say, that "check" and "check-maybe" shall skip the > expensive tests. "check-expensive", "<foo>", and "<foo>.log" shall run > all tests. Sorry for slow reply! I would say that, yes, both check and check-maybe should skip expensive tests by default. I think this actually contradicts the GNU coding standards, but possibly it's these that need updating for very slow tests. I'd also agree about <foo>, <foo>.log (on master <foo>.log is lisp/<foo.log>, and lisp/<foo> also exists). However, I also think that these should also respond to setting SELECTOR on the command line. The problem with this patch: +SELECTOR_DEFAULT = (quote (not (tag :expensive-test))) +SELECTOR_EXPENSIVE = nil +SELECTOR = +check-maybe: + @${MAKE} check-doit SELECTOR="${SELECTOR_DEFAULT}" + is that "make check-maybe SELECTOR=nil" doesn't actually run the expensive tests as it should. That was the reason for my original, rather more complicated, suggestion. I've attached a complete patch below (actually tested this time!), which I think works. Would this make sense to you? Phil diff --git a/test/automated/Makefile.in b/test/automated/Makefile.in index 152e601..2534a65 100644 --- a/test/automated/Makefile.in +++ b/test/automated/Makefile.in @@ -89,7 +89,13 @@ WRITE_LOG = > $@ 2>&1 || { stat=ERROR; cat $@; }; echo $$stat: $@ ## Beware: it approximates 'no-byte-compile', so watch out for false-positives! SELECTOR_DEFAULT = (quote (not (tag :expensive-test))) SELECTOR_EXPENSIVE = nil -SELECTOR = +ifndef SELECTOR +SELECTOR_ACTUAL=$(SELECTOR_DEFAULT) +else +SELECTOR_ACTUAL=$(SELECTOR) +endif + + %.log: ${srcdir}/%.el @if grep '^;.*no-byte-compile: t' $< > /dev/null; then \ loadfile=$<; \ @@ -100,7 +106,7 @@ SELECTOR = echo Testing $$loadfile; \ stat=OK ; \ $(emacs) -l ert -l $$loadfile \ - --eval "(ert-run-tests-batch-and-exit ${SELECTOR})" ${WRITE_LOG} + --eval "(ert-run-tests-batch-and-exit ${SELECTOR_ACTUAL})" ${WRITE_LOG} ELFILES = $(sort $(wildcard ${srcdir}/*.el)) LOGFILES = $(patsubst %.el,%.log,$(notdir ${ELFILES})) @@ -123,7 +129,7 @@ $(foreach test,${TESTS},$(eval $(call test_template,${test}))) ## Rerun all default tests. check: mostlyclean - @${MAKE} check-doit SELECTOR="${SELECTOR_DEFAULT}" + @${MAKE} check-doit SELECTOR="${SELECTOR_ACTUAL}" ## Rerun all default and expensive tests. .PHONY: check-expensive @@ -133,7 +139,7 @@ check-expensive: mostlyclean ## Only re-run default tests whose .log is older than the test. .PHONY: check-maybe check-maybe: - @${MAKE} check-doit SELECTOR="${SELECTOR_DEFAULT}" + @${MAKE} check-doit SELECTOR="${SELECTOR_ACTUAL}" ## Run the tests. .PHONY: check-doit ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [Emacs-diffs] emacs-25 f5c762c: Additional changes for "make check-expensive" 2016-01-14 23:04 ` Phillip Lord @ 2016-01-15 8:45 ` Michael Albinus 0 siblings, 0 replies; 8+ messages in thread From: Michael Albinus @ 2016-01-15 8:45 UTC (permalink / raw) To: Phillip Lord; +Cc: emacs-devel phillip.lord@russet.org.uk (Phillip Lord) writes: Hi Phillip, > I've attached a complete patch below (actually tested this time!), which > I think works. Would this make sense to you? Yes, go for it. Please describe it also somewhere. > Phil Best regards, Michael. ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2016-01-15 8:45 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- [not found] <20160105084751.30334.99051@vcs.savannah.gnu.org> [not found] ` <E1aGNHT-0007u5-Su@vcs.savannah.gnu.org> 2016-01-12 17:43 ` [Emacs-diffs] emacs-25 f5c762c: Additional changes for "make check-expensive" Phillip Lord 2016-01-12 17:49 ` John Wiegley 2016-01-12 18:55 ` Michael Albinus 2016-01-14 22:15 ` Phillip Lord 2016-01-12 19:03 ` Michael Albinus 2016-01-14 8:13 ` Michael Albinus 2016-01-14 23:04 ` Phillip Lord 2016-01-15 8:45 ` Michael Albinus
Code repositories for project(s) associated with this public inbox https://git.savannah.gnu.org/cgit/emacs.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).