Skip to content

Commit

Permalink
Initial support for doctest.
Browse files Browse the repository at this point in the history
  • Loading branch information
@doublep
doublep committed Apr 24, 2024
1 parent 76b08f7 commit 9f5387f
Show file tree
Hide file tree
Showing 6 changed files with 163 additions and 20 deletions.
75 changes: 75 additions & 0 deletions eldev-doctest.el
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
;;; eldev-doctest.el --- Elisp development tool -*- lexical-binding: t -*-

;;; Copyright (C) 2024 Paul Pogonyshev

;; This program 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.

;; This program 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 this program. If not, see https://www.gnu.org/licenses.

;;; Code:

(require 'eldev)


(defvar doctest-message-level)
(defvar doctest-after-every-test-functions)
(defvar doctest-after-all-tests-hook)

(declare-function doctest-files "doctest")
(declare-function doctest-state "doctest")


(defvar eldev--test-doctest-concise-expected nil)


(defun eldev-test-doctest-preprocess-selectors (selectors)
(when selectors
(eldev-warn "Doctest currently doesn't support selectors; they will be ignored")))

(defun eldev-run-doctest-tests (files &optional environment)
"Run Doctest tests in specified FILES."
(when eldev-test-stop-on-unexpected
(eldev-warn "Option `--stop-on-unexpected' (`-s') is not supported with Doctest framework"))
;; Unlike with other testing frameworks, the feature here is not required by the test
;; files: those files are the same as program's source.
(eldev--require-external-feature 'doctest)
(eldev--test-load-files files)
(eldev-bind-from-environment environment (doctest-message-level eldev--test-doctest-concise-expected)
(let ((doctest-after-every-test-functions doctest-after-every-test-functions)
(doctest-after-all-tests-hook doctest-after-all-tests-hook))
(when eldev--test-doctest-concise-expected
(push (lambda (params)
(let ((state (doctest-state)))
(eldev-test-runner-concise-tick (and (eq (cdr (assq 'result params)) 'failure)
;; Only force a number if Doctest is going to print something.
(eq doctest-message-level 'info))
(cdr (assq 'total state)))))
doctest-after-every-test-functions)
(push (lambda ()
(let* ((state (doctest-state))
(num-total (cdr (assq 'total state))))
(unless (= eldev--test-runner-concise-num-reported num-total)
(eldev-test-runner-concise-tick t num-total))))
doctest-after-all-tests-hook))
(doctest-files files)))
(let ((state (doctest-state)))
(setf eldev-test-num-passed (+ eldev-test-num-passed (cdr (assq 'passed state)))
eldev-test-num-failed (+ eldev-test-num-failed (cdr (assq 'failed state))))
;; Even if unsupported by the framework, at least update the variable so that we can
;; stop before running further test types.
(when eldev-test-stop-on-unexpected
(setf eldev-test-stop-on-unexpected (- eldev-test-stop-on-unexpected (assq 'failed state))))))


(provide 'eldev-doctest)

;;; eldev-doctest.el ends here
71 changes: 51 additions & 20 deletions eldev.el
View file
Original file line number Diff line number Diff line change
Expand Up @@ -321,6 +321,7 @@ Since 0.5")

(defvar eldev-known-tool-packages
'((buttercup :version "1.24" :archive melpa)
(doctest :archive melpa)
(ecukes :version "0.6.18" :archive melpa)
;; Need GNU ELPA for `let-alist' on older Emacs versions.
(package-lint :version "0.14" :archives (melpa gnu-elpa))
Expand Down Expand Up @@ -4040,6 +4041,9 @@ At least one of options `--file' and `--open' is required."
(declare-function eldev-count-buttercup-tests "eldev-buttercup" (selectors))
(declare-function eldev-run-buttercup-tests "eldev-buttercup" (selectors &optional environment))

(declare-function eldev-test-doctest-preprocess-selectors "eldev-doctest" (selectors))
(declare-function eldev-run-doctest-tests "eldev-doctest" (files &optional environment))

(declare-function eldev-test-ecukes-preprocess-selectors "eldev-ecukes" (selectors))
(declare-function eldev-run-ecukes-tests "eldev-ecukes" (feature-files selectors &optional environment))

Expand Down Expand Up @@ -4174,6 +4178,13 @@ Should normally be specified only from command line.")
(run-tests . (lambda (selectors _files _runner environment)
(eldev-run-buttercup-tests selectors environment)))
(profiling-self . t)))
(doctest . ((fileset-base . main)
(file-description . "doctest `.el' file%s")
(packages . ((:tool doctest)))
(require . eldev-doctest)
(preprocess-selectors . eldev-test-doctest-preprocess-selectors)
(run-tests . (lambda (_selectors files _runner environment)
(eldev-run-doctest-tests files environment)))))
(ecukes . ((detect . (lambda () t)) ; if `.feature' files are found, then they must be for Ecukes
(fileset . "*.feature")
(file-description . "test `.feature' file%s")
Expand Down Expand Up @@ -4254,6 +4265,15 @@ for details."
:profiling-self t
(eldev--do-test 'ecukes parameters))

(eldev-defcommand eldev-test-doctest (&rest parameters)
"Run project's Doctest regression/unit tests. See command `test'
for details."
:parameters "[SELECTOR...]"
:aliases doctest
:category testing
:hidden-if (or (<= (length (eldev-listify eldev-test-framework)) 1) (not (memq 'doctest eldev-test-framework)))
(eldev--do-test 'doctest parameters))

(defun eldev--do-test (possible-frameworks parameters)
(eldev-load-project-dependencies 'test)
(if possible-frameworks
Expand All @@ -4266,7 +4286,8 @@ for details."
(when unused
(signal 'eldev-error `(:hint ,(eldev-format-message "%s" (eldev-message-enumerate '("Used framework:" "Used frameworks:") (eldev-listify eldev-test-framework)))
"This project doesn't use %s" ,(eldev-message-enumerate "test framework" (nreverse unused)))))))
(setf possible-frameworks (mapcar #'car eldev-test-known-frameworks)))
;; Exclude non-autodetectable frameworks, e.g. Doctest.
(setf possible-frameworks (mapcar #'car (eldev-filter (assq 'detect it) eldev-test-known-frameworks))))
;; Prepare `eldev-test-stop-on-unexpected' for the actual test functions.
(let ((eldev-test-stop-on-unexpected (when eldev-test-stop-on-unexpected
(if (and (integerp eldev-test-stop-on-unexpected) (> eldev-test-stop-on-unexpected 0))
Expand All @@ -4286,11 +4307,12 @@ for details."
(push selector filter-patterns)
(push selector selectors)))
(dolist (framework possible-frameworks)
(let* ((fileset (or (eldev-test-get-framework-entry framework 'fileset) "*.el"))
(entry (cdr (assoc fileset filesets))))
(let* ((fileset (cons (or (eldev-test-get-framework-entry framework 'fileset-base) 'test)
(or (eldev-test-get-framework-entry framework 'fileset) "*.el")))
(entry (assoc fileset filesets)))
(if entry
(push framework (car entry))
(push (cons fileset (list `(,framework) (or (eldev-test-get-framework-entry framework 'file-description) "test `.el' file%s") t)) filesets))))
(push framework (cadr entry))
(push (list fileset (list framework) (or (eldev-test-get-framework-entry framework 'file-description) "test `.el' file%s") t) filesets))))
(setf selectors (nreverse selectors)
filesets (nreverse filesets))
(unwind-protect
Expand All @@ -4301,6 +4323,7 @@ for details."
(unless (or (and eldev-test-stop-on-unexpected (< eldev-test-stop-on-unexpected 0))
(and (eq pass 'count) (>= num-matched-tests eldev-test-expect-at-least)))
(let* ((fileset (car entry))
(std-el-fileset (equal fileset '(test . "*.el")))
(used-by-frameworks (nth 0 (cdr entry)))
(file-description (nth 1 (cdr entry)))
(files (nth 2 (cdr entry)))
Expand All @@ -4314,8 +4337,10 @@ for details."
(if fileset
(push fileset framework-filesets)
(setf disregard-framework-filesets t))))
(setf files (eldev-find-and-trace-files `(:and ,(eldev-standard-fileset 'test) ,fileset ,@(when (and framework-filesets (not disregard-framework-filesets))
`((:or ,@(nreverse framework-filesets)))))
(setf files (eldev-find-and-trace-files `(:and ,(eldev-standard-fileset (car fileset))
,(cdr fileset)
,@(when (and framework-filesets (not disregard-framework-filesets))
`((:or ,@(nreverse framework-filesets)))))
file-description))
(when filter-patterns
(setf files (eldev-filter-files files (reverse filter-patterns)))
Expand All @@ -4325,18 +4350,10 @@ for details."
;; this `when' is important.
(when files
(setf found-any-files t)
(when (and (equal fileset "*.el") (not files-looked-up))
;; Require `.el' files and automatically install used test frameworks.
(when (and std-el-fileset (not files-looked-up))
(eldev-autoinstalling-implicit-dependencies t
(dolist (file files)
(let* ((absolute-without-el (replace-regexp-in-string (rx ".el" eos) "" (expand-file-name file eldev-project-dir) t t))
(already-loaded (eldev-any-p (assoc (concat absolute-without-el it) load-history) load-suffixes)))
(if already-loaded
(eldev-trace "Not loading file `%s': already `require'd by some other file" file)
(eldev-named-step nil (eldev-format-message "loading test file `%s'" file)
(eldev-trace "%s..." (eldev-current-step-name t))
;; Loading the test file can results in evaluation, which might use `eldev-backtrace'.
(eldev-backtrace-notch 'eldev
(load absolute-without-el nil t nil t))))))))
(eldev--test-load-files files)))
(let* ((runner-name (or eldev-test-runner 'simple))
(runner (or (cdr (assq runner-name eldev--test-runners))
(signal 'eldev-error `(:hint ("Check output of `%s test --list-runners'" ,(eldev-shell-command t)) "Unknown test runner `%s'" ,runner-name))))
Expand All @@ -4349,7 +4366,7 @@ for details."
(and (eq pass 'count) (>= num-matched-tests eldev-test-expect-at-least)))
(let ((already-prepared (memq framework used-frameworks)))
(unless already-prepared
(unless (equal fileset "*.el")
(unless std-el-fileset
(eldev--test-maybe-install-framework framework "Installing package(s) of testing framework `%s'..."))
(unless (or (null supported) (memq framework (eldev-listify supported)))
(signal 'eldev-error `(:hint ("Run `%s test --list-runners' for more information" ,(eldev-shell-command t))
Expand Down Expand Up @@ -4399,6 +4416,18 @@ for details."
(when any-frameworks-failed
(signal 'eldev-quit 1)))))

(defun eldev--test-load-files (files)
(dolist (file files)
(let* ((absolute-without-el (replace-regexp-in-string (rx ".el" eos) "" (expand-file-name file eldev-project-dir) t t))
(already-loaded (eldev-any-p (assoc (concat absolute-without-el it) load-history) load-suffixes)))
(if already-loaded
(eldev-trace "Not loading file `%s': already `require'd by some other file" file)
(eldev-named-step nil (eldev-format-message "loading test file `%s'" file)
(eldev-trace "%s..." (eldev-current-step-name t))
;; Loading the test file can results in evaluation, which might use `eldev-backtrace'.
(eldev-backtrace-notch 'eldev
(load absolute-without-el nil t nil t)))))))

(defvar byte-compiler-error-flag)
(defun eldev--test-autoinstalling-framework (enabled callback)
;; If framework is specified explicitly, ensure it is installed first. Otherwise
Expand Down Expand Up @@ -4641,7 +4670,9 @@ be silenced."
(eldev--test-ert-concise-expected . t)))
(`buttercup
`((buttercup-reporter-batch-quiet-statuses . (skipped disabled pending passed))
(eldev--test-buttercup-concise-expected . t))))
(eldev--test-buttercup-concise-expected . t)))
(`doctest
`((eldev--test-doctest-concise-expected . t))))
(eldev-test-runner-simple-environment framework)))

(defun eldev-test-runner-concise-tick (force-number &optional num-executed num-planned)
Expand Down
14 changes: 14 additions & 0 deletions test/integration/doctest.el
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
;; -*- lexical-binding: t -*-

(require 'test/common)


(ert-deftest eldev-test-doctest-project-m-1 ()
;; Two tests, all pass.
(eldev--test-run "project-m" ("test")
(should (string-match-p "2 passed" stdout))
(should (string-match-p "0 failed" stdout))
(should (= exit-code 0))))


(provide 'test/integration/doctest)
1 change: 1 addition & 0 deletions test/project-m/Eldev
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
(setf eldev-test-framework 'doctest)
7 changes: 7 additions & 0 deletions test/project-m/project-m-advanced.el
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
(defun project-m-hello-to (whom)
"Doctest me:
>> (project-m-hello-to \"world\")
=> \"Hello, world!\""
(format "Hello, %s!" whom))

(provide 'project-m-advanced)
15 changes: 15 additions & 0 deletions test/project-m/project-m.el
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
;;; project-m.el --- Simple test project with no dependencies that uses `doctest' -*- lexical-binding: t -*-

;; Version: 1.0
;; Homepage: https://example.com/
;; Package-Requires: ((emacs "24"))

(defun project-m-hello ()
"Doctest me:
>> (project-m-hello)
=> \"Hello\""
"Hello")

(provide 'project-m)

;;; project-m.el ends here

9 comments on commit 9f5387f

@doublep
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@suhail-singh, @ag91: It took kinda long, but now there is some support for doctest in Eldev. Unlike with other frameworks, its use must be declared explicitly (see test/project-m/Eldev), because there is now good way to autodetect it. If you can, please try this out and tell me what's missing, wrong or buggy.

@suhail-singh
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@doublep thank you for the update.
will there be an Eldev release in the coming days which would include this functionality?

@doublep
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, within next week or so. But it would be nice if you could evaluate the additions before the release, maybe something is badly missing. See instructions to upgrade Eldev to an unreleased version.

@suhail-singh
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some areas of improvement based on limited testing:

  1. It would help if the doctest summary noted, at the very least, that the
    pass / fail statistics related to doctest tests. Other test runners such
    as ert don't do this, however, they note the name of the test that is being
    executed. Even something as simple as Running doctest tests would help.

  2. A failing doctest should result in a non-zero exit code for eldev test.
    This doesn't seem to happen, at least when the eldev-test-framework is set
    to '(ert doctest).

@suhail-singh
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@doublep ^

@doublep
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@suhail-singh:
I think point 1. is rather for @ag91 to improve in the library directly.

Point 2 is valid, I apparently forgot this. Fixed now in 822e0d4 (and a test added).

@ag91
Copy link

@ag91 ag91 commented on 9f5387f Apr 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would help if the doctest summary noted, at the very least, that the
pass / fail statistics related to doctest tests. Other test runners such
as ert don't do this, however, they note the name of the test that is being
executed. Even something as simple as Running doctest tests would help

I guess this only for non-interactive run, so a message in doctest-files should do the trick, no?

@doublep
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ag91: I don't know, maybe would be better to just prepend "Doctest summary: " to the summary string in all cases? Currently it just looks a bit out-of-context, like "what is passed/failed, I forgot what I did three seconds ago?"

Or maybe @suhail-singh could clarify what he meant. (I think here you need @, otherwise notifications probably don't get sent).

@suhail-singh
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... a message in doctest-files should do the trick, no?

@ag91 yes, i think that would be sufficient

Please sign in to comment.