Restructure Makefile to make it easier to exclude tests

[manphiz]

• Include all test files in test-x-rkt-files using wildcard
• Drop the rule to build on ./test/racket directory

More on motivation: ./test/racket/hash-lang-test.rkt fails when running under Debian sbuild as it tries to open a display :0, and it cannot work under sbuild. Restructuring the Makefile this way makes it easier to exclude a test using filter-out.

[greghendershott]

There are different styles for Racket tests, and different ways to use raco test to run each.

1. One way, often preferred in "modern" Racket, is to put tests in a test submodule, often within the same file as the code being tested. (Some people like the tests adjacent. Others might think it's clutter. Anyway it's a choice.) For this, we use raco test -x, which means, only run the test submodule, and if no such submodule, do nothing at all.

2. Another way, is that the entire file is tests. For this, we use raco test.

So the proposed change won't work.

To the extent this PR is motivated by #738, perhaps we can fix that directly, and close this PR?

If there really does need to be a way to exclude tests via the Makefile: I'm not sure how that could/should work, but glad to discuss more.

[greghendershott]

p.s. Although I think the raco test -x shorthand is familiar to most Racketeers, it might be clearer if the Makefile spelled it out more explicitly -- which would be something like raco test --submodule test --no-run-if-absent.

(Where --submodule test is making explicit the default, and --no-run-if-absent is the long form of the -x flag.)

Maybe that should be in a comment next to the shorthand, or vice versa?

[manphiz]

Greg Hendershott ***@***.***> writes:

There are different styles for Racket tests, and different ways to use `raco test` to run each. 1. One way, often preferred in "modern" Racket, is to put tests in a `test` submodule, often within the same file as the code being tested. (Some people like the tests adjacent. Others might think it's clutter. Anyway it's a choice.) For this, we use `raco test -x`, which means, only run the `test` submodule. 2. Another way, is that the entire file is tests. For this, we use `raco test`. So the proposed change won't work.

Ack. So what is the recommended way to skip individual tests in both 1 and 2 settings?

To the extent this PR is motivated by #738, perhaps we can fix that directly, and close this PR?

There are also other tests not working in Debian sbuild environment. This is currently how I skip another failing test[1] and #738[2]

If there really does need to be a way to exclude tests via the `Makefile`: I'm not sure how that could/should work, but glad to discuss more.

Personally I don't insist on using Makefile as long as there is a way to skip tests :)

-- Reply to this email directly or view it on GitHub: #739 (comment) You are receiving this because you authored the thread. Message ID: ***@***.***>

[1] https://salsa.debian.org/emacsen-team/racket-mode/-/commit/e5f840cbc155140ab38c234bbf851901ea92ab55 [2] https://salsa.debian.org/emacsen-team/racket-mode/-/commit/db68f875350a7be5f81864f70b9dd92491694f85

…

-- Regards, Xiyue Deng

[greghendershott]

So far I've preferred to keep any conditional with the test itself, not external in the Makefile.

(That filter-out is an exception where it wasn't possible (AFAICT), not meant to be a role model example.)

More on motivation: ./test/racket/hash-lang-test.rkt fails when running under Debian sbuild as it tries to open a display :0, and it cannot work under sbuild. Restructuring the Makefile this way makes it easier to exclude a test using filter-out.

In this case, does the following work for you on buildd?

Although I may have made a mistake, the intent is to:

• skip the tests and exit with 0 on a headless server with no DISPLAY env var.
• and, belt+suspenders, if loading a function from the Racket framework library errors, also skip the tests and exit 0.

modified   test/racket/hash-lang-test.rkt
@@ -1,7 +1,6 @@
 #lang racket/base
 
 (require rackunit
-         framework
          racket/class
          racket/dict
          racket/async-channel
@@ -11,6 +10,17 @@
          "../../racket/lang-info.rkt"
          "../../racket/util.rkt")
 
+(unless (getenv "DISPLAY")
+  (displayln "DISPLAY not available; SKIPPING hash-lang tests")
+  (exit 0))
+
+(define racket:text%
+  (with-handlers ([exn:fail?
+                   (λ _
+                     (displayln "racket:text% from framework not available; SKIPPING hash-lang tests")
+                     (exit 0))])
+    (dynamic-require 'framework 'racket:text%)))
+
 (define hash-lang%
   (with-handlers ([exn:fail:filesystem:missing-module?
                    (λ _

---

(And for #738 I think a slightly different, but also localized check. I'll follow up there, on that issue.)

[greghendershott]

OK, on a minimal branch I'm working on:

• Adding a new GitHub Actions job that

  1. installs only Minimal Racket
  2. manually installs the packages recommended to users here
  3. runs the tests without using xvfb-run

  This should be closer to what you have and are doing with buildd -- however note you might need to add 2.

• Fixing any issues that arise. This includes the two you've already reported, plus one it caught with the commands that analyze requires. The general theme is, a test should itself skip running when it detects conditions it knows about. And, it should display a "skipping blah tests (because)" message.

And this new GHA job will presumably help us avoid creating more such problems, going forward.

Again, for now this is still just on that minimal topic branch. If you'd like to review before I merge, let me know, otherwise I'll aim to merge within a day or two.

[manphiz]

Greg Hendershott ***@***.***> writes:

OK, on a `minimal` branch I'm working on: - Adding a new GitHub Actions job that 1. installs only Minimal Racket 2. manually installs the packages recommended to users [here](https://www.racket-mode.com/#Minimal-Racket) 3. runs the tests _without_ using `xvfb-run` This should be closer to what you have and are doing with `buildd` -- however note you might need to add 2. - Fixing any issues that arise. This includes the two you've already reported, plus one it caught with the commands that analyze requires. The general theme is, a test should itself skip running when it detects conditions it knows about. And, it should display a "skipping blah tests (because)" message. And this new GHA job will presumably help us avoid creating more such problems, going forward. Again, for now this is still just on that `minimal` topic branch. If you'd like to review before I merge, let me know, otherwise I'll aim to merge within a day or two.

Tried out the minimal branch and it seems to work well: scribble and hash-lang tests are properly skipped. Looking forward to the merging!

…

-- Reply to this email directly or view it on GitHub: #739 (comment) You are receiving this because you authored the thread. Message ID: ***@***.***>

-- Regards, Xiyue Deng

[greghendershott]

Merged!