unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Stefan Kangas <stefankangas@gmail.com>
To: Lars Ingebrigtsen <larsi@gnus.org>
Cc: emacs-devel@gnu.org
Subject: Re: Tests and linting not coupled to one file
Date: Sun, 21 Feb 2021 04:37:33 -0600	[thread overview]
Message-ID: <CADwFkmkjhVFDSN-j4fKSMZYtB2P4fNhnL3jxXwa-j-QEiTwwKQ@mail.gmail.com> (raw)
In-Reply-To: <87sgb4lkty.fsf@gnus.org>

[-- Attachment #1: Type: text/plain, Size: 1202 bytes --]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Stefan Kangas <stefankangas@gmail.com> writes:
>
>> That's a fair point.  Likely they would not be aware of "test/misc" at
>> first.  OTOH we could document it in "test/file-organization.org".
>>
>> Here's three scenarios where adding a test to the existing files might
>> not make sense:
>
> [...]
>
>> That said, if we can find a way to shoe-horn the stuff we need into the
>> existing test suite without it being overly ugly, it's perfectly fine by
>> me.  Perhaps that would be less ugly than adding a new "test/misc"
>> directory.
>
> There's certainly something to be said for consistency here: If we're
> looking for a test of something in foo.el, we look in
> lisp/foo-tests.el.  And as you point out, cross-cutting linter stuff
> doesn't really fit into that scheme at all.
>
> So upon further consideration, I agree with you that adding a test/misc
> directory would be better in the long term.

(That was in September.)

How does the attached patch look?  It runs all custom tests
automatically - all of them marked as expensive, and two of them marked
as expected to fail.  We will probably want to fix up the failing tests
at some point, too.

[-- Attachment #2: 0001-Run-admin-cus-tests.el-tests-from-test-suite.patch --]
[-- Type: text/x-diff, Size: 11005 bytes --]

From 414e939e3275630f33bdebb90c95bf9f578d93ae Mon Sep 17 00:00:00 2001
From: Stefan Kangas <stefan@marxist.se>
Date: Fri, 19 Feb 2021 12:31:56 +0100
Subject: [PATCH] Run admin/cus-tests.el tests from test suite

* test/Makefile.in (SUBDIRS): Run tests in new directory "misc",
intended for tests not belonging to any one file.
* test/misc/test-custom-deps.el:
* test/misc/test-custom-libs.el:
* test/misc/test-custom-noloads.el:
* test/misc/test-custom-opts.el: New files.
* test/lisp/custom-tests.el (custom--test-local-option): Move test to
above new file test-custom-opts.el.

* admin/cus-test.el: Document running tests from regular test suite.
* test/file-organization.org (Test Files): Document new test directory
"misc" for tests not belonging to any one file.
---
 admin/cus-test.el                |  7 +++++
 test/Makefile.in                 |  2 +-
 test/file-organization.org       |  4 +++
 test/lisp/custom-tests.el        | 11 --------
 test/misc/test-custom-deps.el    | 42 +++++++++++++++++++++++++++++
 test/misc/test-custom-libs.el    | 46 ++++++++++++++++++++++++++++++++
 test/misc/test-custom-noloads.el | 45 +++++++++++++++++++++++++++++++
 test/misc/test-custom-opts.el    | 39 +++++++++++++++++++++++++++
 8 files changed, 184 insertions(+), 12 deletions(-)
 create mode 100644 test/misc/test-custom-deps.el
 create mode 100644 test/misc/test-custom-libs.el
 create mode 100644 test/misc/test-custom-noloads.el
 create mode 100644 test/misc/test-custom-opts.el

diff --git a/admin/cus-test.el b/admin/cus-test.el
index afd5f4ceae..30b5f65561 100644
--- a/admin/cus-test.el
+++ b/admin/cus-test.el
@@ -37,6 +37,13 @@
 ;;
 ;;   src/emacs -batch -l admin/cus-test.el -f cus-test-noloads
 ;;
+;; or as a part of the test suite with
+;;
+;;   make -C test test-custom-opts
+;;   make -C test test-custom-deps
+;;   make -C test test-custom-libs
+;;   make -C test test-custom-noloads
+;;
 ;; in the emacs source directory.
 ;;
 ;; For interactive use: Load this file.  Then
diff --git a/test/Makefile.in b/test/Makefile.in
index ff228d1261..48bbe8712b 100644
--- a/test/Makefile.in
+++ b/test/Makefile.in
@@ -257,7 +257,7 @@ define test_template
 $(foreach test,${TESTS},$(eval $(call test_template,${test})))
 
 ## Get the tests for only a specific directory.
-SUBDIRS = $(sort $(shell find lib-src lisp src -type d ! -path "*resources*" -print))
+SUBDIRS = $(sort $(shell find lib-src lisp misc src -type d ! -path "*resources*" -print))
 
 define subdir_template
   .PHONY: check-$(subst /,-,$(1))
diff --git a/test/file-organization.org b/test/file-organization.org
index 7cf5b88d6d..d1f92da432 100644
--- a/test/file-organization.org
+++ b/test/file-organization.org
@@ -43,6 +43,10 @@ Similarly, tests of features implemented in C should reside in
 ~-tests.el~ added to the base-name of the tested source file.  Thus,
 tests for ~src/fileio.c~ should be in ~test/src/fileio-tests.el~.
 
+Some tests do not belong to any one particular file.  Such tests
+should be put in the ~misc~ directory and be given a descriptive name
+that does /not/ end with ~-tests.el~.
+
 There are also some test materials that cannot be run automatically
 (i.e. via ert).  These should be placed in ~/test/manual~; they are
 not run by the "make check" command and its derivatives.
diff --git a/test/lisp/custom-tests.el b/test/lisp/custom-tests.el
index 10854c71d5..09f79c1a08 100644
--- a/test/lisp/custom-tests.el
+++ b/test/lisp/custom-tests.el
@@ -145,17 +145,6 @@ custom-test-show-comment-preserves-changes
                                (widget-apply field :value-to-internal origvalue)
                                "bar"))))))
 
-(defconst custom-test-admin-cus-test
-  (expand-file-name "admin/cus-test.el" source-directory))
-
-(declare-function cus-test-opts custom-test-admin-cus-test)
-
-(ert-deftest check-for-wrong-custom-types ()
-  :tags '(:expensive-test)
-  (skip-unless (file-readable-p custom-test-admin-cus-test))
-  (load custom-test-admin-cus-test)
-  (should (null (cus-test-opts t))))
-
 (ert-deftest custom-test-enable-theme-keeps-settings ()
   "Test that enabling a theme doesn't change its settings."
   (let* ((custom-theme-load-path `(,(ert-resource-directory)))
diff --git a/test/misc/test-custom-deps.el b/test/misc/test-custom-deps.el
new file mode 100644
index 0000000000..f072adddcb
--- /dev/null
+++ b/test/misc/test-custom-deps.el
@@ -0,0 +1,42 @@
+;;; test-custom-deps.el --- Test custom deps  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2021 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; The command `cus-test-deps' loads all (!) custom dependencies and
+;; reports about load errors.
+
+;;; Code:
+
+(require 'ert)
+
+(defconst custom-test-admin-cus-test
+  (expand-file-name "admin/cus-test.el" source-directory))
+
+(declare-function cus-test-deps custom-test-admin-cus-test)
+(defvar cus-test-deps-errors)  ; from admin/cus-tests.el
+
+(ert-deftest test-custom-deps ()
+  :tags '(:expensive-test)
+  (skip-unless (file-readable-p custom-test-admin-cus-test))
+  (load custom-test-admin-cus-test)
+  (cus-test-deps)
+  (should-not cus-test-deps-errors))
+
+;;; test-custom-deps.el ends here
diff --git a/test/misc/test-custom-libs.el b/test/misc/test-custom-libs.el
new file mode 100644
index 0000000000..70f043d129
--- /dev/null
+++ b/test/misc/test-custom-libs.el
@@ -0,0 +1,46 @@
+;;; test-custom-libs.el --- Test custom loads  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2021 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; This file runs for all libraries with autoloads separate emacs
+;; processes of the form "emacs -batch -l LIB".
+
+;;; Code:
+
+(require 'ert)
+
+(defconst custom-test-admin-cus-test
+  (expand-file-name "admin/cus-test.el" source-directory))
+
+(declare-function cus-test-libs custom-test-admin-cus-test)
+(defvar cus-test-libs-errors)  ; from admin/cus-tests.el
+
+;; FIXME: Currently fails for:
+;;        - lisp/term/ns-win.el
+;;        - lisp/org/org-num.el
+(ert-deftest test-custom-libs ()
+  :tags '(:expensive-test)
+  :expected-result :failed ; FIXME: See above.
+  (skip-unless (file-readable-p custom-test-admin-cus-test))
+  (load custom-test-admin-cus-test)
+  (cus-test-libs t)
+  (should-not cus-test-libs-errors))
+
+;;; test-custom-deps.el ends here
diff --git a/test/misc/test-custom-noloads.el b/test/misc/test-custom-noloads.el
new file mode 100644
index 0000000000..e999fe2abb
--- /dev/null
+++ b/test/misc/test-custom-noloads.el
@@ -0,0 +1,45 @@
+;;; test-custom-deps.el --- Test custom noloads  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2021 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; The command `cus-test-noloads' returns a list of variables which
+;; are somewhere declared as custom options, but not loaded by
+;; `custom-load-symbol'.
+
+;;; Code:
+
+(require 'ert)
+
+(defconst custom-test-admin-cus-test
+  (expand-file-name "admin/cus-test.el" source-directory))
+
+(declare-function cus-test-noloads custom-test-admin-cus-test)
+(defvar cus-test-vars-not-cus-loaded)  ; from admin/cus-tests.el
+
+;; FIXME: Multiple failures here.
+(ert-deftest custom-test-load ()
+  :tags '(:expensive-test)
+  :expected-result :failed ; FIXME: See above.
+  (skip-unless (file-readable-p custom-test-admin-cus-test))
+  (load custom-test-admin-cus-test)
+  (cus-test-noloads)
+  (should-not cus-test-vars-not-cus-loaded))
+
+;;; test-custom-deps.el ends here
diff --git a/test/misc/test-custom-opts.el b/test/misc/test-custom-opts.el
new file mode 100644
index 0000000000..fa6b9e66ae
--- /dev/null
+++ b/test/misc/test-custom-opts.el
@@ -0,0 +1,39 @@
+;;; test-custom-opts.el --- Test custom opts  -*- lexical-binding:t -*-
+
+;; Copyright (C) 2021 Free Software Foundation, Inc.
+
+;; This file is part of GNU Emacs.
+
+;; GNU Emacs is free software: you can redistribute it and/or modify
+;; it under the terms of the GNU General Public License as published by
+;; the Free Software Foundation, either version 3 of the License, or
+;; (at your option) any later version.
+
+;; GNU Emacs is distributed in the hope that it will be useful,
+;; but WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+;; GNU General Public License for more details.
+
+;; You should have received a copy of the GNU General Public License
+;; along with GNU Emacs.  If not, see <https://www.gnu.org/licenses/>.
+
+;;; Commentary:
+
+;; The command `cus-test-opts' tests many (all?) custom options.
+
+;;; Code:
+
+(require 'ert)
+
+(defconst custom-test-admin-cus-test
+  (expand-file-name "admin/cus-test.el" source-directory))
+
+(declare-function cus-test-opts custom-test-admin-cus-test)
+
+(ert-deftest check-for-wrong-custom-opts ()
+  :tags '(:expensive-test)
+  (skip-unless (file-readable-p custom-test-admin-cus-test))
+  (load custom-test-admin-cus-test)
+  (should (null (cus-test-opts t))))
+
+;;; test-custom-opts.el ends here
-- 
2.30.0


  reply	other threads:[~2021-02-21 10:37 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200925124646.29315.84502@vcs0.savannah.gnu.org>
     [not found] ` <20200925124647.83150209D4@vcs0.savannah.gnu.org>
2020-09-25 13:11   ` Tests and linting not coupled to one file (was: Re: master 664927b: Add an expensive test for defcustom types) Stefan Kangas
2020-09-25 13:46     ` Tests and linting not coupled to one file Lars Ingebrigtsen
2020-09-25 15:36       ` Stefan Kangas
2020-09-26 13:38         ` Lars Ingebrigtsen
2021-02-21 10:37           ` Stefan Kangas [this message]
2021-02-21 13:18             ` Lars Ingebrigtsen
2021-02-21 19:34               ` Stefan Kangas

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/emacs/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CADwFkmkjhVFDSN-j4fKSMZYtB2P4fNhnL3jxXwa-j-QEiTwwKQ@mail.gmail.com \
    --to=stefankangas@gmail.com \
    --cc=emacs-devel@gnu.org \
    --cc=larsi@gnus.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).