Fix tests

[christopherlam]

Tests were passing incorrectly.

This branch removes the srfi-64 test runner customisation, using the default test-runner-simple. The test output is less nice, but it does not incorrectly signal test passes.

I'm not sure of the effect upon the guile coverage tests.

@jralls

[jralls]

Is this related to Arch Linux passing CI on #1926?

[christopherlam]

Is this related to Arch Linux passing CI on #1926?

Yes

[christopherlam]

@jralls this still needs to merge to allow failing guile tests to signal ninja. Any idea if it'll affect guile coverage tests?

[jralls]

@christopherlam I got that half-done and got distracted by working on the macOS tests and by my laptop breaking over the weekend. But you can test it yourself, follow the coverage instructions and compare the reports results with the last regular test results.

[jralls]

@christopherlam OK, I built it and ran tests with GUILE_COVERAGE. Works fine, though the Scheme coverage info shows up under share/guile/3.0/gnucash which surprised me a little. I know it would be a bit more work to just get rid of gnc:test-runner, but it would really be better than leaving the tombstone. I don't think we need the comment, the commit message should be sufficient.

[christopherlam]

Ok I'll get rid of gnc:test-runner however removing srfi64-extras.scm is a tad too difficult!

[christopherlam]

Hmm even this branch doesn't stop CMake incorrectly passing failing scheme tests. I am not familiar enough with the %load-hook mechanism to repair this.

[jralls]

Hmm even this branch doesn't stop CMake incorrectly passing failing scheme tests. I am not familiar enough with the %load-hook mechanism to repair this.

Maybe you could add a test that fails and ctest doesn't notice on Arch. That will give us something to compare between the CI builds that might help track down what's going on.

[christopherlam]

Hmm even this branch doesn't stop CMake incorrectly passing failing scheme tests. I am not familiar enough with the %load-hook mechanism to repair this.

Maybe you could add a test that fails and ctest doesn't notice on Arch. That will give us something to compare between the CI builds that might help track down what's going on.

In the stable build? It's easy to modify any test-equal assertion to make test fail.

[christopherlam]

Hmm even this branch doesn't stop CMake incorrectly passing failing scheme tests. I am not familiar enough with the %load-hook mechanism to repair this.

Maybe you could add a test that fails and ctest doesn't notice on Arch. That will give us something to compare between the CI builds that might help track down what's going on.

https://github.com/christopherlam/gnucash/tree/scheme-fails

[jralls]

In the stable build? It's easy to modify any test-equal assertion to make test fail.

No, in this PR so we can see it in CI. Make it a separate commit so it's easy to back out before we merge.

[christopherlam]

See this latest commit should make ci barf

[jralls]

So it turns out that removing our test runner is counterproductive: With the test runner CI passes only on Arch Linux, without it passes everywhere. Here's the LastTest.log fragment from Arch:

"test-core-utils" start time: May 17 12:37 PDT
Output:
----------------------------------------------------------

;;; WARNING (invalid identifier ${symbol})
[pass] line:9, test: N_ defined
[pass] line:12, test: N_ works properly
[pass] line:17, test: null
[fail] line:21, test: basic
test-core-utils
 -> expected: "pascal"
 -> obtained: "basic"
[pass] line:25, test: basic with unused symbols
[pass] line:29, test: one substitution
[pass] line:33, test: one substitution with hyphen
[pass] line:37, test: two substitutions out of order
[pass] line:41, test: trying to reference invalid symbol
[pass] line:45, test: 
Source:test-core-utils.scm
pass = 9, fail = 1
<end of output>
Test time =   0.06 sec
----------------------------------------------------------
Test Passed.
"test-core-utils" end time: May 17 12:37 PDT
"test-core-utils" time elapsed: 00:00:00
----------------------------------------------------------

[jralls]

On ubuntu 22-04

"test-core-utils" start time: May 17 13:14 PDT
Output:
----------------------------------------------------------
[pass] line:9, test: N_ defined
[pass] line:12, test: N_ works properly
[pass] line:17, test: null
[fail] line:21, test: basic
test-core-utils
 -> expected: "pascal"
 -> obtained: "basic"
[pass] line:25, test: basic with unused symbols
[pass] line:29, test: one substitution
[pass] line:33, test: one substitution with hyphen
[pass] line:37, test: two substitutions out of order
[pass] line:41, test: trying to reference invalid symbol
[pass] line:45, test: 
Source:test-core-utils.scm
pass = 9, fail = 1

;;; WARNING (invalid identifier ${symbol})
<end of output>
Test time =   0.27 sec
----------------------------------------------------------
Test Failed.
"test-core-utils" end time: May 17 13:14 PDT
"test-core-utils" time elapsed: 00:00:00
----------------------------------------------------------

macOS looks the same. It isn't about cmake version, macOS at least is also using the latest 3.29.3.

[jralls]

@christopherlam I found a difference. Do this

--- a/common/cmake_modules/GncAddTest.cmake
+++ b/common/cmake_modules/GncAddTest.cmake
@@ -149,7 +149,10 @@ function(gnc_add_scheme_test _TARGET _SOURCE_FILE)
                   (format #t \"%load-compiled-path = ~s~%\" %load-compiled-path)
                   (error \"Loading guile/site file from outside build tree!\" filename))))
       (load-from-path \"${_TARGET}\")
-      (exit (run-test))"
+      (let ((result (run-test)))
+           (format #t \"Test Results ~s~%\" result)
+           (exit result))
+"
     )
   endif()
   get_guile_env()

And look at the test output either with ctest -V -R test-core-utils. Guile 2.2 on macOS returns Test Results #t. Guile 3.0.9 on Arch Linux returns

Test Results #<test-runner pass-count: 9 fail-count: 1 xpass-count: 0 xfail-count: 0 skip-count: 0 skip-list: () fail-list: () run-list: #t skip-save: () fail-save: () group-stack: () on-test-begin: #<procedure %test-null-callback (runner)> on-test-end: #<procedure 7fee28cc87c0 at srfi64-extras.scm:29:6 (runner)> on-group-begin: #<procedure 7fee2d9b67a0 at srfi/srfi-64/testing.scm:192:40 (runner name count)> on-group-end: #<procedure %test-null-callback (runner)> on-final: #<procedure 7fee28cc86c0 at srfi/srfi-64/testing.scm:285:12 (r)> on-bad-count: #<procedure 7fee2d9b67b8 at srfi/srfi-64/testing.scm:197:38 (runner count expected)> on-bad-end-name: #<procedure 7fee2d9b67c8 at srfi/srfi-64/testing.scm:198:41 (runner begin end)> total-count: 10 count-list: () result-alist: ((source-form test-end "test-core-utils") (source-file . "test-core-utils.scm") (source-line . 53)) aux-value: #f>

And ctest always treats that as true.

[christopherlam]

https://git.savannah.gnu.org/cgit/guile.git/log/module/srfi/srfi-64/testing.scm describes the 2 srfi-64 changes between 2.0.10 (introduce srfi-64) and 3.0.9. Can you figure out what would have changed? I can't understand the %load-hook hack.

[jralls]

Comparing the commit dates with the releases shows the first went into 3.0.5 and the second into 3.0.7. I applied the first one to guile 2.2 and it makes test-core-utils pass.

%load-hook just holds a function to run every time something gets loaded. The following lambda is that something and it just checks and errors out if the test file being loaded isn't in the build directory. It was added in 7cae61d; you can read the commit comment yourself.

[christopherlam]

Thanks @jralls is this ready, sans 0467ac5?

[jralls]

@christopherlam I think so, I was waiting for you to check it.