https://blogs.gentoo.org/ago/2020/07/04/gentoo-tinderbox/ Issue: app-emacs/compat-28.1.1.3 fails tests. Discovered on: amd64 (internal ref: ci)
Created attachment 786590 [details] build.log build log and emerge --info
As of now I couldn't reproduce.
(In reply to Maciej Barć from comment #2) > As of now I couldn't reproduce. These tests compare values from compat's json-parse-string to the native JSON parser, any Emacs with USE=-json will fail here. A lot of the tests are similar (comparing the compat implementation with the real thing), so I think it would be best to RESTRICT=test or delete the tests as they don't provide a lot of value.
Deleting ALL test is a bad idea, because there are still some other test provide some value. We can always add emacs[json] to BDEPEND :) ...which I think is the right thing to do!
The bug has been closed via the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=9b80c6ce439ef6013dfa1ec1868576d1acaad322 commit 9b80c6ce439ef6013dfa1ec1868576d1acaad322 Author: Maciej Barć <xgqt@gentoo.org> AuthorDate: 2022-06-21 17:46:26 +0000 Commit: Maciej Barć <xgqt@gentoo.org> CommitDate: 2022-06-21 17:48:05 +0000 app-emacs/compat: add emacs[json] required for tests Closes: https://bugs.gentoo.org/853322 Signed-off-by: Maciej Barć <xgqt@gentoo.org> app-emacs/compat/compat-28.1.1.3.ebuild | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-)
(In reply to Maciej Barć from comment #4) > We can always add emacs[json] to BDEPEND :) ...which I think is the right > thing to do! No, this is a terrible idea because it limits the user's choice. Please revert and find a different solution.
(In reply to Ulrich Müller from comment #6) > (In reply to Maciej Barć from comment #4) > > We can always add emacs[json] to BDEPEND :) ...which I think is the right > > thing to do! > > No, this is a terrible idea because it limits the user's choice. Please > revert and find a different solution. For example, check for "has_version app-editors/emacs[json(-)]" in src_test() and skip tests if this condition is not fulfilled.
(In reply to Ulrich Müller from comment #7) > For example, check for "has_version app-editors/emacs[json(-)]" in > src_test() and skip tests if this condition is not fulfilled. Thinking about it, this won't work, because the eselected Emacs version may not have the feature. For the same reason, the BDEPEND won't work either. So presumably, a runtime check in src_test is needed here. (Look at app-editors/emacs-daemon-0.22 for an example.)
(In reply to Ulrich Müller from comment #6) > (In reply to Maciej Barć from comment #4) > > We can always add emacs[json] to BDEPEND :) ...which I think is the right > > thing to do! > > No, this is a terrible idea because it limits the user's choice. Please > revert and find a different solution. No, it does not limit the user's choice because it is just for tests. Maybe I wasn't clear enough, I meant BDEPEND="test? ( thing )".
The bug has been closed via the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=98307eefb98e8ec4fa76501aa3bfb877ff59de96 commit 98307eefb98e8ec4fa76501aa3bfb877ff59de96 Author: Maciej Barć <xgqt@gentoo.org> AuthorDate: 2022-06-21 19:35:49 +0000 Commit: Maciej Barć <xgqt@gentoo.org> CommitDate: 2022-06-21 19:36:11 +0000 app-emacs/compat: check for native JSON parsing support Closes: https://bugs.gentoo.org/853322 Signed-off-by: Maciej Barć <xgqt@gentoo.org> app-emacs/compat/compat-28.1.1.3.ebuild | 16 ++++++++++++++++ 1 file changed, 16 insertions(+)
If you think "test?" BDEPEND is useless now you can drop it. In my opinion it should stay; I'm sure I have seen something similar some time ago, where it wasn't 100% clear the test dep is used but it was still being pulled in.
(In reply to Maciej Barć from comment #11) > If you think "test?" BDEPEND is useless now you can drop it. > > In my opinion it should stay; I'm sure I have seen something similar some > time ago, where it wasn't 100% clear the test dep is used but it was still > being pulled in. It's correct. Because of slotted Emacs, a BDEPEND is necessary but not sufficient.
(In reply to Sam James from comment #12) > (In reply to Maciej Barć from comment #11) > > If you think "test?" BDEPEND is useless now you can drop it. > > > > In my opinion it should stay; I'm sure I have seen something similar some > > time ago, where it wasn't 100% clear the test dep is used but it was still > > being pulled in. > > It's correct. Because of slotted Emacs, a BDEPEND is necessary but not > sufficient. ... but please add a "die" in your new check, because otherwise we'll get a false-positive on tests passing.
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=cb8fad5ea441ea9c5a8e940fad76b2a07fb32155 commit cb8fad5ea441ea9c5a8e940fad76b2a07fb32155 Author: Maciej Barć <xgqt@gentoo.org> AuthorDate: 2022-06-21 19:47:11 +0000 Commit: Maciej Barć <xgqt@gentoo.org> CommitDate: 2022-06-21 19:47:11 +0000 app-emacs/compat: add missing die to avoid false-positive Bug: https://bugs.gentoo.org/853322 Signed-off-by: Maciej Barć <xgqt@gentoo.org> app-emacs/compat/compat-28.1.1.3.ebuild | 1 + 1 file changed, 1 insertion(+)
The bug has been referenced in the following commit(s): https://gitweb.gentoo.org/repo/gentoo.git/commit/?id=e8701186083295b4136c3e2d45048b1f9265eb1e commit e8701186083295b4136c3e2d45048b1f9265eb1e Author: Ulrich Müller <ulm@gentoo.org> AuthorDate: 2022-06-21 20:00:30 +0000 Commit: Ulrich Müller <ulm@gentoo.org> CommitDate: 2022-06-21 20:00:30 +0000 app-emacs/compat: Drop BDEPEND on emacs[json] Bug: https://bugs.gentoo.org/853322#c11 Signed-off-by: Ulrich Müller <ulm@gentoo.org> app-emacs/compat/compat-28.1.1.3.ebuild | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-)
(In reply to Sam James from comment #13) > ... but please add a "die" in your new check, because otherwise we'll get a > false-positive on tests passing. IMHO this is just wrong. We don't want failing tests for reasons that have nothing to do with the package.
