Email or username:

Password:

Forgot your password?
Top-level
graywolf

@civodul I mean, if you, as one of the maintainers, would like to take the code and put it into guile, I would be happy to re-license it :) But I am skeptical anyone has time to review it all, even if there would be interest.

FWIW my test-error works (correctly, with all of #t, #f, 'symbol, &programming-error and programming-error?). I suspect implementing it is much easier now in 2024 then when the code was written (guile-2.0 was considered cutting edge based on the comments).

I am obviously biased, but I think the implementation *can* be fairly readable: git.wolfsden.cz/guile-wolfsden ; At the same time I am still fairly new to Scheme, so I am sure it can be improved in many places.

Picking subset of the API that I would verify correct is workable approach, but test-error in particular *is* useful. (And even test-assert, the very basic test, is "wrong", it evaluates test-name twice. But that particular bug probably does not matter too much.)

Regarding the spec, while there are things I would like to change or specify more clearly, the interface as likely intended (which sadly is not the same as "as written") *mostly* works, albeit being bit simplistic.

My biggest gripe is that all of it is top level, so does not play well with REPL (all tests run on file load, which sucks). Solved by adding non-standard define-test:

(define-test "let us test"
(test-assert "foo" #t)
(test-assert "bar" #t))

Will see how I will feel about it after few hundred more tests written.

6 comments
Ludovic Courtès

@graywolf Your implementation looks *much* nicer! Were you able to test it against existing test suites like that of Guix or maybe some smaller guile-* package?

If there are no known regressions, I’d be in favor of integrating it (as LGPLv3+).

graywolf

@civodul Thanks :)

I did not test it with other projects (I did not expect non-me people to use it), but if it is heading in the direction of possible inclusion into Guile proper, I will find the time to do that. I will try with Guix, since that is probably the biggest project using it. I hope I will be able to get to it over the weekend.

Agree, as I said, I have no issues with re-licensing it.

graywolf

@civodul So quick testing suggests that it mostly works, there are few `definition in expression context` errors. I do not know enough about scheme to say if the implementation is wrong, or if the test is doing something it should not.

Specification says just "This evaluates the expression.", so it might be the test at fault?

Aaaanyway, changing the expansion in the implementation to `(let () expression)' instead of just `expression' seems to resolve it, and I am willing to do that, since it seems useful.

I will do more testing on Sunday.

@civodul So quick testing suggests that it mostly works, there are few `definition in expression context` errors. I do not know enough about scheme to say if the implementation is wrong, or if the test is doing something it should not.

Specification says just "This evaluates the expression.", so it might be the test at fault?

graywolf

@civodul

tl;dr: It looks good.

Long version:

So after few required compatibility tweaks I found that 4 extra tests fail with my implementation compared to the guile's on. Diff:

--- /tmp/log.guile 2024-08-11 07:33:03.383038595 +0000
+++ /tmp/log.wolfsden 2024-08-11 17:52:05.230543705 +0000
@@ -88,7 +88,7 @@
PASS: tests/elm.scm
PASS: tests/elm.scm
PASS: tests/elpa.scm
-PASS: tests/file-systems.scm
+FAIL: tests/file-systems.scm
PASS: tests/gem.scm
PASS: tests/gexp.scm
PASS: tests/git.scm
@@ -129,7 +129,7 @@
PASS: tests/scripts.scm
PASS: tests/search-paths.scm
PASS: tests/services.scm
-PASS: tests/services/file-sharing.scm
+FAIL: tests/services/file-sharing.scm
PASS: tests/services/configuration.scm
PASS: tests/services/configuration.scm
PASS: tests/services/configuration.scm
@@ -147,7 +147,7 @@
PASS: tests/store-database.scm
PASS: tests/store-deduplication.scm
PASS: tests/store-roots.scm
-PASS: tests/store.scm
+FAIL: tests/store.scm
PASS: tests/substitute.scm
PASS: tests/swh.scm
SKIP: tests/syscalls.scm
@@ -195,10 +195,10 @@
Testsuite summary for GNU Guix 1.3.0.63387-de714
============================================================================
# TOTAL: 2454
-# PASS: 2421
+# PASS: 2417
# SKIP: 25
# XFAIL: 2
-# FAIL: 6
+# FAIL: 10
# XPASS: 0
# ERROR: 0
============================================================================

Three of those extra fails are due to using string in test-error, in the spirit of:

(test-error "transmission-password-hash, salt value too long"
(transmission-password-hash
"transmission"
(make-string (+ %transmission-salt-length 1) #\a)))

I do not plan to include a special support for string error-type. It can (in the test file) be replaced with #t and the test will start passing. (I think it is likely the test author of the test just confused test-name and error-type in test-error.)

The 4th is this test:

(test-equal "store-path-package-name #f"
#f
(store-path-package-name
"/foo/bar/283gqy39v3g9dxjy26rynl0zls82fmcg-guile-2.0.7"))

Which relies on bug in guile's implementation. The specification says that if expression raises an exception, the test should fail. Guile just returns #f instead. So the test passes, since #f equals #f. This test should be switched to test-error.

Of course, this is something that should probably be double checked by someone else.

@civodul

tl;dr: It looks good.

Long version:

So after few required compatibility tweaks I found that 4 extra tests fail with my implementation compared to the guile's on. Diff:

--- /tmp/log.guile 2024-08-11 07:33:03.383038595 +0000
+++ /tmp/log.wolfsden 2024-08-11 17:52:05.230543705 +0000
@@ -88,7 +88,7 @@
PASS: tests/elm.scm
PASS: tests/elm.scm
PASS: tests/elpa.scm
-PASS: tests/file-systems.scm
+FAIL: tests/file-systems.scm
PASS: tests/gem.scm
PASS: tests/gexp.scm
PASS: tests/git.scm
@@ -129,7 +129,7 @@

Ludovic Courtès

@graywolf Nice. Looks like these two Guix tests you mention should be fixed/adjusted, actually.

That it reveals issues with tests rather than with the framework itself looks like a good sign!

Ludovic Courtès

@graywolf In the past, when incorporating third-party code in Guile, such as SRFIs, we’d usually try to keep the upstream part unchanged and just add a thin layer around it, the idea being that it would make it easier to keep in sync with upstream.

Clearly that hasn’t been of much use with SRFI-64.

Go Up