GNU bug report logs - #69405
[PATCH] .emacs.d/tree-sitter/ not used in tree-sitter tests

Previous Next

Package: emacs;

Reported by: Mattias Engdegård <mattias.engdegard <at> gmail.com>

Date: Mon, 26 Feb 2024 13:38:02 UTC

Severity: normal

Tags: patch

Done: Mattias Engdegård <mattias.engdegard <at> gmail.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 69405 in the body.
You can then email your comments to 69405 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to bug-gnu-emacs <at> gnu.org:
bug#69405; Package emacs. (Mon, 26 Feb 2024 13:38:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Mattias Engdegård <mattias.engdegard <at> gmail.com>:
New bug report received and forwarded. Copy sent to bug-gnu-emacs <at> gnu.org. (Mon, 26 Feb 2024 13:38:02 GMT) Full text and rfc822 format available.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Emacs Bug Report <bug-gnu-emacs <at> gnu.org>
Cc: Yuan Fu <casouri <at> gmail.com>
Subject: [PATCH] .emacs.d/tree-sitter/ not used in tree-sitter tests
Date: Mon, 26 Feb 2024 14:36:39 +0100
[Message part 1 (text/plain, inline)]
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.

[test-treesit-dir.diff (application/octet-stream, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69405; Package emacs. (Mon, 26 Feb 2024 14:26:03 GMT) Full text and rfc822 format available.

Message #8 received at 69405 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: casouri <at> gmail.com, 69405 <at> debbugs.gnu.org
Subject: Re: bug#69405: [PATCH] .emacs.d/tree-sitter/ not used in tree-sitter
 tests
Date: Mon, 26 Feb 2024 15:59:12 +0200
> Cc: Yuan Fu <casouri <at> gmail.com>
> From: Mattias Engdegård <mattias.engdegard <at> 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.




Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69405; Package emacs. (Mon, 26 Feb 2024 16:29:01 GMT) Full text and rfc822 format available.

Message #11 received at 69405 <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: casouri <at> gmail.com, 69405 <at> debbugs.gnu.org
Subject: Re: bug#69405: [PATCH] .emacs.d/tree-sitter/ not used in tree-sitter
 tests
Date: Mon, 26 Feb 2024 17:26:43 +0100
[Message part 1 (text/plain, inline)]
26 feb. 2024 kl. 14.59 skrev Eli Zaretskii <eliz <at> 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.)

[test-treesit-dir-2.diff (application/octet-stream, attachment)]

Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69405; Package emacs. (Mon, 26 Feb 2024 17:11:03 GMT) Full text and rfc822 format available.

Message #14 received at 69405 <at> debbugs.gnu.org (full text, mbox):

From: Eli Zaretskii <eliz <at> gnu.org>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: casouri <at> gmail.com, 69405 <at> debbugs.gnu.org
Subject: Re: bug#69405: [PATCH] .emacs.d/tree-sitter/ not used in tree-sitter
 tests
Date: Mon, 26 Feb 2024 18:52:41 +0200
> From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
> Date: Mon, 26 Feb 2024 17:26:43 +0100
> Cc: 69405 <at> debbugs.gnu.org,
>  casouri <at> gmail.com
> 
> 26 feb. 2024 kl. 14.59 skrev Eli Zaretskii <eliz <at> 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.




Reply sent to Mattias Engdegård <mattias.engdegard <at> gmail.com>:
You have taken responsibility. (Mon, 26 Feb 2024 18:16:01 GMT) Full text and rfc822 format available.

Notification sent to Mattias Engdegård <mattias.engdegard <at> gmail.com>:
bug acknowledged by developer. (Mon, 26 Feb 2024 18:16:01 GMT) Full text and rfc822 format available.

Message #19 received at 69405-done <at> debbugs.gnu.org (full text, mbox):

From: Mattias Engdegård <mattias.engdegard <at> gmail.com>
To: Eli Zaretskii <eliz <at> gnu.org>
Cc: 69405-done <at> debbugs.gnu.org, Yuan Fu <casouri <at> gmail.com>
Subject: Re: bug#69405: [PATCH] .emacs.d/tree-sitter/ not used in tree-sitter
 tests
Date: Mon, 26 Feb 2024 19:07:30 +0100
26 feb. 2024 kl. 17.52 skrev Eli Zaretskii <eliz <at> 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!





Information forwarded to bug-gnu-emacs <at> gnu.org:
bug#69405; Package emacs. (Mon, 26 Feb 2024 21:36:02 GMT) Full text and rfc822 format available.

Message #22 received at 69405-done <at> debbugs.gnu.org (full text, mbox):

From: Yuan Fu <casouri <at> gmail.com>
To: Mattias Engdegård <mattias.engdegard <at> gmail.com>
Cc: 69405-done <at> debbugs.gnu.org, Eli Zaretskii <eliz <at> gnu.org>
Subject: Re: bug#69405: [PATCH] .emacs.d/tree-sitter/ not used in tree-sitter
 tests
Date: Mon, 26 Feb 2024 13:34:02 -0800

> On Feb 26, 2024, at 10:07 AM, Mattias Engdegård <mattias.engdegard <at> gmail.com> wrote:
> 
> 26 feb. 2024 kl. 17.52 skrev Eli Zaretskii <eliz <at> 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



bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Wed, 27 Mar 2024 11:24:05 GMT) Full text and rfc822 format available.

This bug report was last modified 1 year and 140 days ago.

Previous Next


GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.