all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* bug#69405: [PATCH] .emacs.d/tree-sitter/ not used in tree-sitter tests
@ 2024-02-26 13:36 Mattias Engdegård
  2024-02-26 13:59 ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Mattias Engdegård @ 2024-02-26 13:36 UTC (permalink / raw)
  To: 69405; +Cc: Yuan Fu

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

The .emacs.d/tree-sitter/ directory isn't searched when running tests non-interactively. The Makefile sets HOME to /nonexistent, which is correct but has the unfortunate side-effect of not running all the tests properly even with the compiled grammars installed.

The attached patch adds an environment variable, EMACS_TREE_SITTER_DIR, and sets it in the Makefile. I didn't bother documenting it because it's only intended for use in our own tests.


[-- Attachment #2: test-treesit-dir.diff --]
[-- Type: application/octet-stream, Size: 1892 bytes --]

diff --git a/src/treesit.c b/src/treesit.c
index d86ab501187..a2eeb89c93d 100644
--- a/src/treesit.c
+++ b/src/treesit.c
@@ -605,6 +605,17 @@ treesit_load_language (Lisp_Object language_symbol,
     = Fexpand_file_name (concat2 (build_string ("tree-sitter/"), lib_base_name),
 			 Fsymbol_value (Quser_emacs_directory));
   treesit_load_language_push_for_each_suffix (lib_name, &path_candidates);
+
+  /* Then push the directory of EMACS_TREE_SITTER_DIR, if any.
+     It is mainly intended for testing purposes, where HOME isn't available.  */
+  const char *envdir = getenv ("EMACS_TREE_SITTER_DIR");
+  if (envdir && *envdir)
+    {
+      Lisp_Object lib_name = Fexpand_file_name (lib_base_name,
+						build_string (envdir));
+      treesit_load_language_push_for_each_suffix (lib_name, &path_candidates);
+    }
+
   /* Then push paths from treesit-extra-load-path.  */
   Lisp_Object tail;
 
diff --git a/test/Makefile.in b/test/Makefile.in
index 720f5c7ff8c..5e7f1ac444b 100644
--- a/test/Makefile.in
+++ b/test/Makefile.in
@@ -118,6 +118,9 @@ emacs =
 # startup.el must be updated too.
 TEST_HOME = /nonexistent
 
+# Allow tests using tree-sitter to access grammars installed locally.
+TESTENV = HOME=$(TEST_HOME) EMACS_TREE_SITTER_DIR=$(HOME)/.emacs.d/tree-sitter
+
 test_module_dir := src/emacs-module-resources
 
 .PHONY: all check
@@ -178,7 +181,7 @@ testloadfile =
 
 %.log: %.elc
 	$(AM_V_GEN)${MKDIR_P} $(dir $@)
-	$(AM_V_at)HOME=$(TEST_HOME) $(emacs) \
+	$(AM_V_at)$(TESTENV) $(emacs) \
 	  -l ert ${ert_opts} -l $(testloadfile) \
 	  $(TEST_RUN_ERT)
 
@@ -335,7 +338,7 @@ .PHONY:
 ## summarizing step from running when there is an error.
 check-doit:
 ifeq ($(TEST_INTERACTIVE), yes)
-	HOME=$(TEST_HOME) $(emacs) \
+	$(TESTENV) $(emacs) \
 	  -l ert ${ert_opts} \
 	  $(patsubst %,-l %,$(if $(findstring $(TEST_LOAD_EL),yes),$ELFILES,$(ELFILES:.el=))) \
 	  $(TEST_RUN_ERT)

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#69405: [PATCH] .emacs.d/tree-sitter/ not used in tree-sitter tests
  2024-02-26 13:36 bug#69405: [PATCH] .emacs.d/tree-sitter/ not used in tree-sitter tests Mattias Engdegård
@ 2024-02-26 13:59 ` Eli Zaretskii
  2024-02-26 16:26   ` Mattias Engdegård
  0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2024-02-26 13:59 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: casouri, 69405

> Cc: Yuan Fu <casouri@gmail.com>
> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Mon, 26 Feb 2024 14:36:39 +0100
> 
> The .emacs.d/tree-sitter/ directory isn't searched when running tests non-interactively. The Makefile sets HOME to /nonexistent, which is correct but has the unfortunate side-effect of not running all the tests properly even with the compiled grammars installed.
> 
> The attached patch adds an environment variable, EMACS_TREE_SITTER_DIR, and sets it in the Makefile. I didn't bother documenting it because it's only intended for use in our own tests.

Can't we do that in the test harness, instead of introducing
test-suite dependencies into the built Emacs binary?  For example, how
about adding to tree-sitter-extra-load-path in treesit-tests.el
instead?

Thanks.





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#69405: [PATCH] .emacs.d/tree-sitter/ not used in tree-sitter tests
  2024-02-26 13:59 ` Eli Zaretskii
@ 2024-02-26 16:26   ` Mattias Engdegård
  2024-02-26 16:52     ` Eli Zaretskii
  0 siblings, 1 reply; 6+ messages in thread
From: Mattias Engdegård @ 2024-02-26 16:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: casouri, 69405

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

26 feb. 2024 kl. 14.59 skrev Eli Zaretskii <eliz@gnu.org>:

> Can't we do that in the test harness, instead of introducing
> test-suite dependencies into the built Emacs binary?  For example, how
> about adding to tree-sitter-extra-load-path in treesit-tests.el
> instead?

Yes, that's probably a better idea. We don't even need anything in the test suites if we set `treesit-extra-load-path` in the Makefile, as in this patch.

(We could set an environment variable instead, but then it would need decoding in each tree-sitter test as well, and the change to the Makefile wouldn't really be simpler.)


[-- Attachment #2: test-treesit-dir-2.diff --]
[-- Type: application/octet-stream, Size: 557 bytes --]

diff --git a/test/Makefile.in b/test/Makefile.in
index 720f5c7ff8c..3cbdbec4414 100644
--- a/test/Makefile.in
+++ b/test/Makefile.in
@@ -92,6 +92,10 @@ TEST_RUN_ERT =
 # Additional settings for ert.
 ert_opts =
 
+# Supply a path to local tree-sitter installations, as we run tests
+# without a valid HOME.
+ert_opts += --eval "(setq treesit-extra-load-path '(\"$(HOME)/.emacs.d/tree-sitter\"))"
+
 # Maximum length of lines in ert backtraces; nil for no limit.
 # (if empty, use the default ert-batch-backtrace-right-margin).
 TEST_BACKTRACE_LINE_LENGTH =

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* bug#69405: [PATCH] .emacs.d/tree-sitter/ not used in tree-sitter tests
  2024-02-26 16:26   ` Mattias Engdegård
@ 2024-02-26 16:52     ` Eli Zaretskii
  2024-02-26 18:07       ` Mattias Engdegård
  0 siblings, 1 reply; 6+ messages in thread
From: Eli Zaretskii @ 2024-02-26 16:52 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: casouri, 69405

> From: Mattias Engdegård <mattias.engdegard@gmail.com>
> Date: Mon, 26 Feb 2024 17:26:43 +0100
> Cc: 69405@debbugs.gnu.org,
>  casouri@gmail.com
> 
> 26 feb. 2024 kl. 14.59 skrev Eli Zaretskii <eliz@gnu.org>:
> 
> > Can't we do that in the test harness, instead of introducing
> > test-suite dependencies into the built Emacs binary?  For example, how
> > about adding to tree-sitter-extra-load-path in treesit-tests.el
> > instead?
> 
> Yes, that's probably a better idea. We don't even need anything in the test suites if we set `treesit-extra-load-path` in the Makefile, as in this patch.

Right, thanks.  This is much better.

> (We could set an environment variable instead, but then it would need decoding in each tree-sitter test as well, and the change to the Makefile wouldn't really be simpler.)

The question is how important is it to support cases where the
directory is not under ~/.emacs.d/tree-sitter/, but somewhere else,
perhaps created specifically for running the tests without affecting
the production sessions?  If that's not very important, then
supporting just the HOME case should be enough.





^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#69405: [PATCH] .emacs.d/tree-sitter/ not used in tree-sitter tests
  2024-02-26 16:52     ` Eli Zaretskii
@ 2024-02-26 18:07       ` Mattias Engdegård
  2024-02-26 21:34         ` Yuan Fu
  0 siblings, 1 reply; 6+ messages in thread
From: Mattias Engdegård @ 2024-02-26 18:07 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: 69405-done, Yuan Fu

26 feb. 2024 kl. 17.52 skrev Eli Zaretskii <eliz@gnu.org>:

> The question is how important is it to support cases where the
> directory is not under ~/.emacs.d/tree-sitter/, but somewhere else,
> perhaps created specifically for running the tests without affecting
> the production sessions?  If that's not very important, then
> supporting just the HOME case should be enough.

Yes, let's start with the HOME case, since that's what is being actively inhibited by the Makefile.
Pushed to master. Maybe this will lead to more people running tree-sitter tests (I certainly will).
Thank you!






^ permalink raw reply	[flat|nested] 6+ messages in thread

* bug#69405: [PATCH] .emacs.d/tree-sitter/ not used in tree-sitter tests
  2024-02-26 18:07       ` Mattias Engdegård
@ 2024-02-26 21:34         ` Yuan Fu
  0 siblings, 0 replies; 6+ messages in thread
From: Yuan Fu @ 2024-02-26 21:34 UTC (permalink / raw)
  To: Mattias Engdegård; +Cc: 69405-done, Eli Zaretskii



> On Feb 26, 2024, at 10:07 AM, Mattias Engdegård <mattias.engdegard@gmail.com> wrote:
> 
> 26 feb. 2024 kl. 17.52 skrev Eli Zaretskii <eliz@gnu.org>:
> 
>> The question is how important is it to support cases where the
>> directory is not under ~/.emacs.d/tree-sitter/, but somewhere else,
>> perhaps created specifically for running the tests without affecting
>> the production sessions?  If that's not very important, then
>> supporting just the HOME case should be enough.
> 
> Yes, let's start with the HOME case, since that's what is being actively inhibited by the Makefile.
> Pushed to master. Maybe this will lead to more people running tree-sitter tests (I certainly will).
> Thank you!
> 

Thanks. I totally agree.

Yuan




^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2024-02-26 21:34 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-26 13:36 bug#69405: [PATCH] .emacs.d/tree-sitter/ not used in tree-sitter tests Mattias Engdegård
2024-02-26 13:59 ` Eli Zaretskii
2024-02-26 16:26   ` Mattias Engdegård
2024-02-26 16:52     ` Eli Zaretskii
2024-02-26 18:07       ` Mattias Engdegård
2024-02-26 21:34         ` Yuan Fu

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.