From 1da0201b657a91166974082f00b0f1252851313d Mon Sep 17 00:00:00 2001
From: Josh B <30708316+joshbax189@users.noreply.github.com>
Date: Mon, 18 Nov 2024 21:23:33 +1300
Subject: [PATCH] Fix: eask options should not be passed to buttercup (#281)

* Add more tests for buttercup command

* Update 'test buttercup' to filter options

* Update 'test buttercup' command doco

* Add a comment about buttercup warning and check full paths too
---
 cmds/test/buttercup.js                        |  8 ++---
 lisp/test/buttercup.el                        | 13 +++++++-
 test/commands/test/buttercup/run.sh           | 23 +++++++++++++-
 .../buttercup/test-fail/buttercup-test.el     | 31 +++++++++++++++++++
 .../{test => test-ok}/buttercup-test.el       |  0
 5 files changed, 69 insertions(+), 6 deletions(-)
 mode change 100644 => 100755 test/commands/test/buttercup/run.sh
 create mode 100644 test/commands/test/buttercup/test-fail/buttercup-test.el
 rename test/commands/test/buttercup/{test => test-ok}/buttercup-test.el (100%)

diff --git a/cmds/test/buttercup.js b/cmds/test/buttercup.js
index f2d37696..79e17344 100644
--- a/cmds/test/buttercup.js
+++ b/cmds/test/buttercup.js
@@ -17,15 +17,15 @@
 
 "use strict";
 
-exports.command = ['buttercup [files..]'];
+exports.command = ['buttercup [directories..]'];
 exports.desc = 'Run buttercup tests';
 exports.builder = yargs => yargs
   .positional(
-    '[files..]', {
-      description: 'files you want buttercup to run on',
+    '[directories..]', {
+      description: 'directories containing buttercup tests, must be children of the current directory',
       type: 'array',
     });
 
 exports.handler = async (argv) => {
-  await UTIL.e_call(argv, 'test/buttercup', argv.files);
+  await UTIL.e_call(argv, 'test/buttercup', argv.directories);
 };
diff --git a/lisp/test/buttercup.el b/lisp/test/buttercup.el
index 658b1401..675b2a1d 100644
--- a/lisp/test/buttercup.el
+++ b/lisp/test/buttercup.el
@@ -22,7 +22,18 @@
   ;; Start Testing
   (require 'buttercup)
   ;; Propose fix from https://github.com/jorgenschaefer/emacs-buttercup/pull/217
-  (let ((load-path (cons "." load-path)))
+  (let* ((load-path (cons "." load-path))
+         ;; this does not include options
+         (args (eask-args)))
+    ;; buttercup-run-discover uses command-line-args-left not command-line-args
+    (setq command-line-args-left args)
+    ;; Seems like buttercup-run-discover only works on directories that are children of
+    ;; the current directory.
+    ;; When given a parent directory it always fails with "No suites found", even if there are tests.
+    ;; Since this is a bit confusing, we warn the user specifically.
+    ;; See discussion https://github.com/emacs-eask/cli/pull/281
+    (when-let ((bad-arg (seq-find (lambda (x) (not (file-in-directory-p x default-directory))) args)))
+      (error "Buttercup cannot run in parent directory: %s" bad-arg))
     (buttercup-run-discover)))
 
 ;;; test/buttercup.el ends here
diff --git a/test/commands/test/buttercup/run.sh b/test/commands/test/buttercup/run.sh
old mode 100644
new mode 100755
index 286a47de..4b0c53fe
--- a/test/commands/test/buttercup/run.sh
+++ b/test/commands/test/buttercup/run.sh
@@ -26,4 +26,25 @@ echo "Test command 'buttercup'..."
 cd $(dirname "$0")
 
 eask install-deps --dev
-eask test buttercup
+if eask test buttercup; then
+    # this runs all tests, so should error
+    echo "expected error"
+    exit 1
+fi
+
+# buttercup takes directories as arguments
+eask test buttercup ./test-ok
+if eask test buttercup ./test-ok ./test-fail; then
+    echo "expected error"
+    exit 1
+fi
+
+# buttercup does not take options
+eask test buttercup --no-color ./test-ok
+
+# Because load-path is manually set, cannot refer to parent directories.
+# Note this does work if you do ../buttercup/test-ok/, but not for any other directory.
+if eask test buttercup ../ert/ 2>&1 | grep 'No suites defined'; then
+    echo "expected error"
+    exit 1
+fi
diff --git a/test/commands/test/buttercup/test-fail/buttercup-test.el b/test/commands/test/buttercup/test-fail/buttercup-test.el
new file mode 100644
index 00000000..b492fbe1
--- /dev/null
+++ b/test/commands/test/buttercup/test-fail/buttercup-test.el
@@ -0,0 +1,31 @@
+;;; buttercup-test.el --- Test the command buttercup  -*- lexical-binding: t; -*-
+
+;; Copyright (C) 2022-2024  the Eask authors.
+
+;; 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/>.
+
+;;; Commentary:
+
+;; Tests for the command buttercup
+
+;;; Code:
+
+(require 'buttercup)
+(require 'debug)
+
+(describe "A failing suite"
+  (it "contains a spec with a false expectation"
+    (expect t :to-be nil)))
+
+;;; buttercup-test.el ends here
diff --git a/test/commands/test/buttercup/test/buttercup-test.el b/test/commands/test/buttercup/test-ok/buttercup-test.el
similarity index 100%
rename from test/commands/test/buttercup/test/buttercup-test.el
rename to test/commands/test/buttercup/test-ok/buttercup-test.el