atlas-engineer / prompter

Live-narrowing, fuzzy-matching, extensible prompt framework.
BSD 3-Clause "New" or "Revised" License
13 stars 1 forks source link

.github/workflows/tests: Enable ECL tests. #37

Open Ambrevar opened 10 months ago

Ambrevar commented 10 months ago

Needed for #36.

Rationale:

aadcg commented 10 months ago

I am still not convinced about the benefits for the reasons below. Nonetheless, I don't think this is very important so I'm leaving my thoughts as a note for future reference and reflections.


I think we need to distinguish two sets of goals: those of Nyxt and those of our libraries for their own sake. It should be noted that the success of the former is what makes the latter possible, as @jmercouris will certainly agree.

In light of the above and, regarding the CL implementation/OS CI matrix, I believe that GNU/Linux+SBCL would suffice for Nyxt's goals. If we think about the library alone, I see value when testing on multiple CL implementations (SBCL+CCL to be specific).

Regarding the argument that the CI useful for day-to-day work (such as #36), I'm afraid I still don't quite see it. The CI is triggered on push, so the experience differs from running a CL implementation locally - incremental development or the ability to inspect and debug test suite runs, etc. This is general observation on CI.

The following argument is subtle but important (while not completely related). Notice that the testing CI environment relies on the libraries' submodules. To ensure that it is faithful to Nyxt's testing environment, these need to be kept in sync (in turn, Nyxt's git submodules should be kept in sync with Guix). How do I, as a Nyxt reviewer, test changes made to a library? I run the library test suite within Nyxt's dev environment (nyxt-guix.el). I think that this shows that (1) a CI routine that is faithful to Nyxt requires maintenance (git submodules sync) while (2) those benefits could be achieved without that maintenance burden (by testing on Nyxt's dev env). Note that if we bump git submodule A in Nyxt, then we need to check which libraries also dependent on it and bump it accordingly. It could be automated but I am not sure about the value it would bring. My suggestion would be rather simple: accept that the CI library env differs from the Nyxt testing env. That could be achieved by (1) deleting all git submodules from our libraries and (2) add all of our libraires to Quicklisp. Roswell can load Quicklisp systems, so the CI would be able run the test suites without relying on the submodules and the extra work that it entails. Indeed, such a CI routine isn't faithful to Nyxt's dev environment but that is OK since the it would serve the scope of the library.

I also have doubts regarding the no maintenance argument (beyond the point above). The CI relies on Roswell (which, from my personal experience, is a can of worms) and Github Actions APIs. That does require some work at times.

Finally, I'd like to highlight some "soft" arguments. I believe we should be very good at doing one single thing alone before attempting to do many. Adding each source of complexity one step at a time seems like a reasonable strategy. Also, experimentation is key to reach success. If this change is indeed something that we are lacking, could we not wait until it becomes evident that we need to bring it back? It may be debatable but, in this concrete case and to my mind, I don't think we have reached that point to re-evaluate the decision.

Ambrevar commented 10 months ago
  1. I see great benefit in testing in a different environment: it widens the scope of tests. Example: race conditions may be highlighted while not showing in the CI.
  2. It allows for cross-platform testing (for me, that's Windows and macOS).