| Message ID | cover.1718548296.git.suhail@bayesians.ca |
|---|---|
| Headers | show |
| Series | gnu: emacs-org: Update to 9.7.4. | expand |
Hello, I'm the author of org-super-agenda, some of whose tests have been bypassed in this patch series. Suhail, I appreciate your making the package build on Guix, but have these failing tests been reported as bugs upstream? You mention in a code comment an obsolete variable, but I don't know anything about that, and I can't find a bug reported on the tracker about it. Please, if you know about a bug, and especially if you know about a solution, report it upstream so it can be fixed. I maintain a number of Emacs/Org packages, and keeping up with the changes in Org, and trying to maintain compatibility across versions which have incompatibilities, is trying, especially if users don't report them (I don't always run the latest release of Org). Thanks, Adam
Adam Porter <adam@alphapapa.net> writes: > Suhail, I appreciate your making the package build on Guix, Hello, Adam. Though I am not a user, I appreciate you authoring the package and releasing and supporting it for the benefit of others. > but have these failing tests been reported as bugs upstream? I have not made any bug reports regarding the failing tests yet. I don't know if others have (or rather didn't, before I started writing this email). I did try and see if a newer commit from org-super-agenda was immune from the test failures, but didn't have any luck. It's not clear to me whether the test failures point to bug(s) in org-super-agenda, bug(s) in its test suite, bug(s) in Org mode, or (less likely, but still possible) false positive(s) arising due to the tests not being designed necessarily to run on Guix. I was planning to look into it further, but since I don't personally use org-super-agenda it's not been a priority. Regardless, for the benefit of others, I am glad that you pay attention to this mailing list and are now aware. > You mention in a code comment an obsolete variable, but I don't know > anything about that, and I can't find a bug reported on the tracker > about it. The "obsolete" variable in question is `org-super-agenda-retain-sorting'. The tests making use of the older user option are `org-super-agenda-test--with-retained-sorting' and `org-super-agenda-test--without-retained-sorting'. In commit `27aa0db4243b835494f4836b82d0fbf3c5be7392' on 2021-09-28, it seems you renamed that option to `org-super-agenda-keep-order'. The tests were not updated. The FIXME comment in the former test notes: #+begin_src emacs-lisp ;; FIXME: This seems to be slightly broken on Org 9.6.6. Maybe <https://github.com/alphapapa/org-super-agenda/issues/207>? #+end_src The bug #207 seems to have been first noted about 3 years ago, is currently open and assigned to you, and targeted for the 1.4 milestone. > Please, if you know about a bug, Not being a user of the package, my awareness is limited to the presence of test failures. I am not sure if you have CI configured for the project, but if you have and aren't seeing these failures, it may be worth your while to understand where those differences stem from. If you do and do see the same failures, your awareness and understanding of the issues probably exceeds my own limited understanding, so please excuse most of this email. If you don't have CI configured, you may want to consider doing so. Whether these failures are false positives or not, is not clear to me. If I had spent more time analyzing it, I am sure eventually I would have understood it and either added to the existing issues, or opened new ones, or submitted one or more patch/pull request upstream. Alas, there's only so much time in the day and we must all prioritize. > and especially if you know about a solution, As noted in my patch, my attempt to naively rename `org-super-agenda-retain-sorting' to `org-super-agenda-keep-order' did not fix the failures I observed. The actual fix, whatever it is, isn't one I'm aware of. > report it upstream so it can be fixed. That _is_ the general protcol ;) > I maintain a number of Emacs/Org packages, and keeping up with the > changes in Org, and trying to maintain compatibility across versions > which have incompatibilities, is trying, especially if users don't > report them (I don't always run the latest release of Org). I agree user reports are invaluable. In fact, I'd go further and say that user time is invaluable. However, given that their time is precious, it seems wasteful to rely on them for something that can be adequately addressed by a properly configured CI system (at least in this instance). In my candid opinion, their time is better served reporting on usability concerns that aren't (yet) covered by tests. Regards,
Hi Suhail, On 6/23/24 00:27, Suhail Singh wrote: > >> but have these failing tests been reported as bugs upstream? > > I have not made any bug reports regarding the failing tests yet. I > don't know if others have (or rather didn't, before I started > writing this email). I did try and see if a newer commit from > org-super-agenda was immune from the test failures, but didn't have > any luck. > > It's not clear to me whether the test failures point to bug(s) in > org-super-agenda, bug(s) in its test suite, bug(s) in Org mode, or > (less likely, but still possible) false positive(s) arising due to > the tests not being designed necessarily to run on Guix. I was > planning to look into it further, but since I don't personally use > org-super-agenda it's not been a priority. Regardless, for the > benefit of others, I am glad that you pay attention to this mailing > list and are now aware. I don't subscribe to nor watch the Guix lists; I don't have enough personal bandwidth for that. I only noticed this report because I was trying to understand a problem with building Emacs 29.4 and was looking for related issues on the tracker. >> You mention in a code comment an obsolete variable, but I don't >> know anything about that, and I can't find a bug reported on the >> tracker about it. > > The "obsolete" variable in question is > `org-super-agenda-retain-sorting'. The tests making use of the > older user option are `org-super-agenda-test--with-retained-sorting' > and `org-super-agenda-test--without-retained-sorting'. In commit > `27aa0db4243b835494f4836b82d0fbf3c5be7392' on 2021-09-28, it seems > you renamed that option to `org-super-agenda-keep-order'. The tests > were not updated. The FIXME comment in the former test notes: > > #+begin_src emacs-lisp ;; FIXME: This seems to be slightly broken on > Org 9.6.6. Maybe > <https://github.com/alphapapa/org-super-agenda/issues/207>? > #+end_src > > The bug #207 seems to have been first noted about 3 years ago, is > currently open and assigned to you, and targeted for the 1.4 > milestone. Thanks for your thorough investigation. The problem you've described is simply that, when I renamed a symbol in a contributor's code before merging it, I overlooked to rename it in the associated test as well--and this is the first I've heard about it. The issue mentioned in the FIXME is related to the functionality, but is not about the test itself, and the test does not appear to fail there due to the incorrect symbol (IOW the test would fail anyway due to the "real" problem). >> Please, if you know about a bug, > > Not being a user of the package, my awareness is limited to the > presence of test failures. I am not sure if you have CI configured > for the project, but if you have and aren't seeing these failures, it > may be worth your while to understand where those differences stem > from. If you do and do see the same failures, your awareness and > understanding of the issues probably exceeds my own limited > understanding, so please excuse most of this email. If you don't > have CI configured, you may want to consider doing so. >> I maintain a number of Emacs/Org packages, and keeping up with the >> changes in Org, and trying to maintain compatibility across >> versions which have incompatibilities, is trying, especially if >> users don't report them (I don't always run the latest release of >> Org). > > I agree user reports are invaluable. In fact, I'd go further and > say that user time is invaluable. However, given that their time is > precious, it seems wasteful to rely on them for something that can > be adequately addressed by a properly configured CI system (at least > in this instance). In my candid opinion, their time is better > served reporting on usability concerns that aren't (yet) covered by > tests. As you can see in the respective git repositories, there are extensive lints and tests, and they are run on GitHub CI using my bespoke makem.sh system for linting and testing Emacs Lisp packages, across a variety of Emacs versions. I also use this system extensively to test changes locally before pushing, and I have for years (the whole reason I wrote makem.sh 5 years ago, because catching these kinds of problems after the fact is utterly impractical to deal with). Nevertheless, due to the fact that multiple versions of Emacs and of Org are supported by these packages, it is not always possible for the lints and tests to pass cleanly on all supported versions at the same time--and that can cause actual problems, such as this test having a wrong symbol name, to be obscured, which appears to be the case here. IOW, for some projects, the lints and/or tests are always going to fail on some combination of Emacs/Org versions, even though the project in question may actually work on it. So a "properly configured CI system" is not a panacea; software being software, there will always be holes in the various layers, and when they align... > Whether these failures are false positives or not, is not clear to > me. If I had spent more time analyzing it, I am sure eventually I > would have understood it and either added to the existing issues, or > opened new ones, or submitted one or more patch/pull request > upstream. Alas, there's only so much time in the day and we must all > prioritize. All I ask is that, if you know about an obvious problem like a symbol name being wrong, that you drop me a line, either as an issue on the tracker or via email. When you mentioned an "obsolete symbol," I thought you meant one defined in Org itself (because, strictly speaking, the symbol in org-super-agenda was never in a released version, having been renamed before it was even merged, so it was not "obsolete", a term that has a specific meaning, especially in Emacs-related contexts; and Org itself frequently obsoletes symbols across versions). So I was wondering what new Org symbol I needed to add compatibility code for in org-super-agenda. >> and especially if you know about a solution, > > As noted in my patch, my attempt to naively rename > `org-super-agenda-retain-sorting' to `org-super-agenda-keep-order' > did not fix the failures I observed. The actual fix, whatever it is, > isn't one I'm aware of. That would be issue #207, and it appears to be one of those things that changes across Org versions, making it hard or impossible to fix in a way that comprehensively passes testing (especially since I've no control over the versions of Emacs and Org used in Guix). It's also a minor bug, essentially a cosmetic issue, which has made it a low priority to work on with my limited time, which I'm sure you can appreciate. Anyway, as a longtime user of and contributor to distributions that package upstream software for downstream users, such as Debian and Guix, I appreciate the role of the "middleman" packager/maintainer. Also being an upstream maintainer, I appreciate the challenge from that perspective as well. The best way to smooth over these issues is to have good communication in both directions. So as an upstream developer, I try to make changes small, atomic, and well-documented, and to make bug-fix releases on stable branches, to make it easier for downstream maintainers to incorporate such changes between major releases. My only request as such an upstream maintainer is that, when a downstream maintainer finds a problem, that it be reported to me, especially when it may indicate an underlying bug rather than a downstream compatibility issue (e.g. one specific to Guix; but note that I'm keen to help compatibility with Guix as well). Thanks, Adam
Adam Porter <adam@alphapapa.net> writes: > IOW, for some projects, the lints and/or tests are always going to fail > on some combination of Emacs/Org versions, even though the project in > question may actually work on it. That's understandable. When I encounter such situations, I find it helpful to separate tests where "failure is intolerable" from others where "failure, while not ideal, is acceptable". You may find it helpful to make such a distinction as well. > That would be issue #207, and it appears to be one of those things > that changes across Org versions, making it hard or impossible to fix > in a way that comprehensively passes testing (especially since I've no > control over the versions of Emacs and Org used in Guix). It's also a > minor bug, essentially a cosmetic issue, which has made it a low > priority to work on with my limited time, which I'm sure you can > appreciate. It seems that #207 isn't one that will be fixed in the near future and that that's acceptable (for users of org-super-agenda). It's unclear whether the failure in test `org-super-agenda-test--auto-parent' also corresponds to the same bug, or a distinct issue. Regardless, if one or more of the failing tests are acceptable failures, it would help Guix packagers if upstream was updated to provide a command/invocation that does not fail when such category of tests don't succeed. It would obviate having to ignore such failing tests in Guix and reduce the maintenance burden of downstream packagers.