I've committed updates to the Fossil tests to a new branch: test-fixes-2.24.

Two check-ins are marked with 👀 and I would like to ask for expert review before merging:

• Possibly due to changed tracking of file renames, some merge warnings about a missing common ancestor are replaced with merge conflict warnings.

And there's a test I haven't been able to fix, yet:

• amend-tag-2.2

With these changes, I get:

***** Final results: 1 errors out of 38868 tests
***** Considered failures: amend-tag-2.2
***** Ignored results: 5 ignored errors out of 38868 tests
***** Ignored failures: merge5-sqlite3-issue stash-1-diff stash-WY-1-CODE stash-3-2 stash-3-2-show-1

Wow - that's much better.

I was, coincidentally, looking at amend-tag-2.2, as it was the first failed test I saw when testing the other day.

Turns out that the fossil tag ls order changed between 2.18 and 2.19.

I added a fix that sorts the expected results specifically for the amend-tag-2.2 test.

I had not seen that and I suppose the question still stands.

Looking at the code, where "COLLATE uintnocase" is used in browse.c and tag.c, I see three ways forward:

1. Accept that sorting in browsing and tags has changed and fix amend-tag-2.2 test case.
2. Add e.g. a "COLLATE uintcase" and use it in tag.c and use the original amend-tag-2.2 test.
3. Change to sorting case sensitively for both browse and tag code (59dfca5ed5 added the browse code for numbers, but also changed case).

Personally, I have no real preference either way. Might lean to 1. as the uintnocase has gone through a few releases already.

FWIW, I did a quick test for the second approach, and that passed the long standing amend-tag.2.2 test.

Either way, I suppose amend-tag tests should include tests that exercises the integer comparison.

Two check-ins are marked with 👀 and I would like to ask for expert review before merging:

Without claiming to be an expert on the test scripts...

Those all look really good to me.

Thank you for taking the time to look into this.

Thank you all for the feedback, and for fixing amend-tag-2.2!

Florian,

I'm curious if unversioned tests ran clean for you.

Asking as I get errors for unversioned-46, unversioned-52 and unversioned-55. AFAICT they would have failed for a long time as the affected transfer summaries have changed in [3500dbfd84].

But if you are not seeing it, I'll check further before committing updates.

BTW, if you do test unversioned.test, do you also get prompted for a password? [I do on macOS and as I recall both linux and OpenBSD]

Thanks!

I'll fix that then.

I forgot if I needed something special, but as I recall needing the tcllib package on both OpenBSD and Fedora (not sure just what I did on macOS, except I at lest used brew to install tcl-tk).

I'm looking closer at the testing, and have a few ideas in mind for improvements (in no particular order):

1. Listing "skipped tests" in the summary (if any?), when pre-requisites are not met (like the sha1 package)
2. Avoiding test-framework-xxx failures when running with -keep (it messes with some tests)
3. Using --save-http-password when cloning in the unversioned test (otherwise I need to type/paste a password)
4. The symlinks test seems to need updating (it won't run if following another test, for instance).
5. Exit tester.tcl with code 1 if there are failed tests
6. Sort tests so they always run in the same order (for output comparison)
7. Add a script that can rewrite some lines in the output for comparison between runs (/path/to/fossil becomes just "fossil", hashes becomes HASH, and similar).

Most of those I am testing in one incarnation or other at the moment.

Question: Should I use different branches for these (as they are not inherently linked) or a general "test-improvements" branch as a mini-project (where I would initially add changes from small to large)?

Thanks!

One branch is okay, maybe in units suitable for fossil merge --cherrypick, so the Fossil architects can still decide to pick only individual parts, if they want. (But to me any of your listed points sound reasonable.)

FWIW, the changes are up in the testing-improvements branch. I want to test the script that rewrites the test output on Windows, but have not gotten around to completing a full test setup there.

Between that branch and test-fixes-2.24, I have a few failures still:

• Some amend tests fail if I make my terminal narrow.
• For JSON: json-login-c-m json-login-c-n json-login-c-c

The JSON tests seem to check if anonymous has "m", "n" and "c" capabilities. I haven't been able to track back if that was ever the case, but currently it is not.

Does anyone knows the history of the json tests to confirm if they can be updated to test for just "hz"?

On a separate note, the fix for merge1 tests seem to prevent merges as fossil trips on the conflict markers. Indenting the markers by two spaces seems sufficient to both let the tests succeed and no longer hinders merges.

On Windows, I'm using IronTcl linked from the official Tcl "Software" link:

• https://www.tcl.tk/software/tcltk/
• https://www.irontcl.com/

I get two or three test-framework-* failures, but I need to re-run the test suite, probably on the weekend, and I'll also try it on a narrow terminal window. (I was using Yori's hilite utility to color errors, so maybe the narrow terminal window problem was covered by the additional output layer.)

Not sure about the JSON tests, but I vaguely recall that some default user capabilities have been adjusted, and that change probably didn't go down to the JSON tests? But I currently don't have the details, sorry.

I'll try the tip to indent the merge markers for merge1.test.

Thanks for the tip on Tcl. I had tried the one from Thomas Perschak, but the IronTcl page made me realise I'd been trying the wrong environment variable (TCL_LIBRARY, which I thought best matched by error messages) where I should se PATH.

For the narrow terminal and amend tests (33 columns triggers a fair few), they no longer show if I add "close stdin" after "test_setup".

I suppose fossil then assumes it's not on a terminal and defaults the width of the text (in a way that matches the test expectations).

That's a bit scary to me because I have done some work to have better UTF-8 support for the comment formatter -- but the comment formatter has its own set of tests that seem to pass, so maybe this is unrelated. I'll try to reproduce this on Windows on the weekend.

As I get the code, the text will be wrapped according to the terminal width - if it can be determined. If the width cannot be determined, no wrapping will be performed.

I tested by adding a fossil test-terminal-size at the end of amend.test and testing it with -verbose.

If I do not also add "close stdin", my macOS, OpenBSD and Linux systems report the terminal size. However, run in PowerShell on Windows 2022, I get "0 0".

As I get it, amend.test on Windows would always match agains un-wrapped text.

Using "close stdin" works on my macOS, but unfortunately not on OpenBSD and Linux. Here I get errors that the database is being opened using file descriptor 0 (which was freed up when closing stdin).

Here is a regular run (against trunk, so amend-tag-2.2 still fails):

    $ tclsh8.6 ../fossil/test/tester.tcl ../fossil/fossil amend
    [...]
    ***** End of amend: 7 errors so far ******
    ***** Final results: 7 errors out of 124 tests
    ***** Considered failures: amend-author-1.3 amend-tag-2.2 amend-comment-2.1 amend-comment-2.4 amend-comment-4.1 amend-comment-4.4 amend-comment-5.4
    ***** Ignored results: 0 ignored errors out of 124 tests

By redirecting stdin in the shell, though, the results look better:

    $ tclsh8.6 ../fossil/test/tester.tcl ../fossil/fossil amend </dev/null
    [...]
    ***** End of amend: 1 errors so far ******
    ***** Final results: 1 errors out of 124 tests
    ***** Considered failures: amend-tag-2.2
    ***** Ignored results: 0 ignored errors out of 124 tests

I think I'll stick to this rather than adding extra code in the scripts (e.g. close stdin and then open a known regular file like the script to hi-jack fd 0).

Duh, the test-framework-* failures were due to running the tests from an open check-out.

The amend tests seem to work fine with narrow terminal window sizes on Windows.

I think I'll stick to this rather than adding extra code in the scripts (e.g. close stdin and then open a known regular file like the script to hi-jack fd 0).

So people who see amend test failures need to manually re-run the script with the redirection? I also think this is reasonable, but maybe they need a hint to do this in the docs or printed along the test results?

With the test-fixes-2.24 and testing-improvements branches merged, I currently get 0 errors!

In my test beds (macOS, OpenBSD FreeBSD and Fedora Linux), I still have some errors.

I fixed the anonymous capability json tests as mentioned earlier.

I also fixed merge_renames-13-3 on macOS (tested against the others).

For the narrow terminals, I fixed the amend tests by closing stdin and then temporarily opening the script in readonly mode. On some systems or TCL implementations, file descriptor 0 used when fossil opens repos - which fossil gaurds against to avoid working with stdin.

BUT, that broke further tests (yes, I should have tested more than just amend :-( ). Moved that fix to test/tester.tcl.

On OpenBSD, FreeBSD and Fedora (all arm64/aarch64), I also see failures for fossil-diff-setup and json-diff-diff. They fail as checkins are sorted in the wrong order.

This is due to json_page_finfo() using char sort = -1; to indicate "DESC" sort order. However, on these systems chars are unsigned, so the order becomes "ASC".

Besides that, I get 0 errors on these systems if merging test-fixes-2.24 and testing-improvements.

If merging test-fixes-2.24, I think it's mostly sound, but two things:

1. Andy's question on sorting of tags is still open.
2. The fixes for amend may need confirmation (or merge just f0444376).

On OpenBSD, FreeBSD and Fedora (all arm64/aarch64) [...] However, on these systems chars are unsigned

On FreeBSD/amd64, char seems to be signed. But per standard char (without qualifier) is implementation-dependent, so I would say every occurrence of it with a negative value should be explicitly marked as signed.

Indeed.

I've tracked down a a couple more where functions should return signed char.

I think fixing all the warnings I get with -Wconversion may be too much (at least for me, worrying if I'd break anything on some platform or other, be it legacy or not).

The ones I am sure should be fixed are in the signed-chars branch.

Looking at similar fixes, stephan's approach seems to be to change char to int for such cases.

I don't have a technical nor a personal opinion to about this, and maybe it's not important at all, just an interesting observation.

Seems good to follow that path for consistency.

On a separate note, I had th1-expr-3 fail on me on OmniOS Community Edition v11 r151046ac (i86pc).

I vaguely recall having had that fly by on another system, too (not OmniOS), that I did not get to check.

Looks like a compiler optimisation as the expected -2147483648 becomes -8 with a standard build, but is correct if I configure with --no-opt.

The issue is in Th_SetResultInt(), where the while loop is not entered with an optimised build:

  *(--z) = '\0';
  *(--z) = (char)(48+((unsigned)iVal%10));
  while( (iVal = ((unsigned)iVal/10))>0 ){
    *(--z) = (char)(48+((unsigned)iVal%10));
    assert(z>zBuf);
  }
  if( isNegative ){
    *(--z) = '-';
  }

Should such be fixed in code, by unrolling the iVal assignment?

Or tracked down and reported for the compiler chain?

This affected several systems using GCC (versions ranging from 10 to 13) on various Linux, NetBSD and OmniOS. While I think this is a compiler bug, it's also sufficiently wide spread that I attempted to fix it.

Andy's question on sorting of tags is still open.

Pragmatic answer: the patch to change the tag sort order was accepted, so the test needs to be fixed, too.

The fixes for amend may need confirmation (or merge just f0444376).

You're referring to these changes?

They look simple and unobtrusive--but the fact that I don't get why the problem doesn't appear on my machine proves that I don't understand the matter well enough to comment on it :( Maybe this is an IronTcl vs. Thomas Perschak's Tcl issue?

You're referring to these changes?

I am indeed.

I have an example that may help check if the test on widows does not wrap lines in relevant output.

It is roughly based on amend-comment-5.4.

First, set up a new test file just for this as test/xxx.test

    test_setup

    set test_file "test.txt"
    set comment "This is a fairly long comment used for testing wrapped lines.\
    If wrapped somewhere, there will be too many spaces for string match."

    write_file $test_file "this is a test"
    fossil add test.txt
    fossil commit -m "$comment"

    fossil timeline -n 1
    test xxx-wrapped {[string match "*$comment*" $RESULT]}

    close stdin
    set fd [open $test_file r]

    fossil timeline -n 1
    test xxx-unwrapped {[string match "*$comment*" $RESULT]}

    test_cleanup

Then run the test:

    % tclsh8.6 test/tester.tcl ./fossil xxx -verbose
    ***** xxx ******
    /Users/preben/src/fossil/trunk/fossil add test.txt
    RESULT (0): ADDED  test.txt
    /Users/preben/src/fossil/trunk/fossil commit -m {This is a fairly long comment used for testing wrapped lines. If wrapped somewhere, there will be too many spaces for string match.}
    RESULT (0): New_Version: b94b78c43e42d6b6b6d2bd973293af1dbd1888f8a9c5b1503614078bea2566bd
    /Users/preben/src/fossil/trunk/fossil timeline -n 1
    RESULT (0): === 2023-11-27 ===
    19:41:01 [b94b78c43e] *CURRENT* This is a fairly long comment used for testing wrapped
             lines. If wrapped somewhere, there will be too many spaces for string match.
             (user: preben tags: trunk)
    --- entry limit (1) reached ---
    test xxx-wrapped FAILED!
    /Users/preben/src/fossil/trunk/fossil timeline -n 1
    RESULT (0): === 2023-11-27 ===
    19:41:01 [b94b78c43e] *CURRENT* This is a fairly long comment used for testing wrapped lines. If wrapped somewhere, there will be too many spaces for string match. (user: preben tags: trunk)
    --- entry limit (1) reached ---
    test xxx-unwrapped OK
    test test-framework-xxx OK
    ***** End of xxx: 1 errors so far ******
    ***** Final results: 1 errors out of 3 tests
    ***** Considered failures: xxx-wrapped
    ***** Ignored results: 0 ignored errors out of 3 tests

A few things to note:

• The comment is a string with a single space between words.
• The comment is wrapped in output before I close stdin
• Because it is wrapped, the [string match ...] test fails
• The comment is not wrapped after closing stdin
• As a result, the same test succeeds

If both tests are OK for you (and the output is never wrapped), then you would not see these tests fail for different terminal widths.

Mine looks different, i.e. no word wrapping, and IronTcl does report an error, but not to either of the xxx-wrapped or xxx-unwrapped tests:

    C:\...>tclsh86t fossil-co\test\tester.tcl .\fossil.exe xxx -verbose
    ***** xxx ******
    C:/.../fossil.exe add test.txt
    RESULT (0): ADDED  test.txt
    C:/.../fossil.exe commit -m {This is a fairly long comment used for testing wrapped lines. If wrapped somewhere, there will be too many spaces for string match.}
    RESULT (0): New_Version: 1375910a0a0e351b42e6a2a68c1824ef3973ee49b05f181d8871854c2ddb67bd
    C:/.../fossil.exe timeline -n 1
    RESULT (0): === 2023-11-28 ===
    06:44:45 [1375910a0a] *CURRENT* This is a fairly long comment used for testing wrapped lines. If wrapped somewhere, there will be too many spaces for string match. (user: Florian tags: trunk)
    --- entry limit (1) reached ---
    test xxx-wrapped OK
    C:/.../fossil.exe timeline -n 1
    RESULT (0): === 2023-11-28 ===
    06:44:45 [1375910a0a] *CURRENT* This is a fairly long comment used for testing wrapped lines. If wrapped somewhere, there will be too many spaces for string match. (user: Florian tags: trunk)
    --- entry limit (1) reached ---
    test xxx-unwrapped OK
    test test-framework-xxx FAILED!
    !!!!! xxx: Could not delete "C:/Users/Florian/AppData/Local/Temp/repo_992/1701153884_1/xxx.test", error: error deleting "C:/Users/Florian/AppData/Local/Temp/repo_992/1701153884_1/xxx.test\test.txt": permission denied
    -code       = 1
    -level      = 0
    -errorstack = INNER {returnImm {Could not delete "C:/Users/Florian/AppData/Local/Temp/repo_992/1701153884_1/xxx.test", error: error deleting "C:/Users/Florian/AppData/Local/Temp/repo_992/1701153884_1/xxx.test\test.txt": permission denied} {}} CALL {robust_delete C:/Users/Florian/AppData/Local/Temp/repo_992/1701153884_1/xxx.test YES_DO_IT} CALL test_cleanup
    -errorcode  = NONE
    -errorinfo  = Could not delete "C:/Users/Florian/AppData/Local/Temp/repo_992/1701153884_1/xxx.test", error: error deleting "C:/Users/Florian/AppData/Local/Temp/repo_992/1701153884_1/xxx.test\test.txt": permission denied
        while executing
    "error "Could not delete \"$path\", error: $error""
        (procedure "robust_delete" line 15)
        invoked from within
    "robust_delete $repoPath YES_DO_IT"
        (procedure "test_cleanup" line 16)
        invoked from within
    "test_cleanup"
        (file "C:/.../fossil-co/test/xxx.test" line 20)
        invoked from within
    "source $testdir/$testfile.test"
    -errorline  = 1"
    ***** End of xxx: 1 errors so far ******
    ***** Final results: 1 errors out of 3 tests
    ***** Considered failures: test-framework-xxx
    ***** Ignored results: 0 ignored errors out of 3 tests

Mine looks different, i.e. no word wrapping

In that case, the terminal width checks fail both before and after closing stdin.

In other words, the commands producing the $RESULT text would not wrap for you - and the terminal width therefore does not matter for you.

and IronTcl does report an error

I think I know what's going on there: The test file is still open when test_cleanup runs, preventing the deletion.

I used that as it was easy to verify, but probably should have opened the script instead as is done in the commit.

I would expect running the test with the -keep flag too would not trigger the error (the temporary directory is not deleted).

For the code change, this should not matter as the script file is not inside any of the temporary directories that may get cleaned up by test_setup.

Wow, an IronTcl issue! Thanks for sorting out!

Wow, an IronTcl issue!

Are you referring to the (lack of) wrapping? Or the error deleting the file?

For either case, I think it may be the environment rather than Tcl.

For the wrapping/terminal part, that's more of a hunch, to be honest.

For the file deletion, that's Windows file system semantics I didn't consider when writing the test. In Windows an application can open a file and set a "delete-on-close" flag (if it has deletion rights). Only when the last file descriptor referencing the file is closed will the file actually be deleted. In the meantime, other processes cannot open the file (even to delete it).

Unix filesystem semantics allow deleting a file (by removing a reference to an inode) while the inode is referenced by a file descriptor (if any). The inode is no longer in use when there are no links to it and no file descriptors have it open.

Are you referring to the (lack of) wrapping? ... that's more of a hunch, to be honest.

Yes, to wrapping. I agree it's not a bad bug, but still something wasting our time ...

For the file deletion, that's Windows file system semantics ... Only when the last file descriptor referencing the file is closed will the file actually be deleted. In the meantime, other processes cannot open the file (even to delete it).

With traditional Windows NT semantics, it's possible to open new handles to the same file (from the same or other threads/processes) if the file was opened with FILE_SHARE_DELETE, even if the file has been scheduled for deletion after closing the last handle (either by opening with FILE_FLAG_DELETE_ON_CLOSE or by explicitly calling NtSetInformationFile(FileDispositionInformation,DeleteFile=1)). It's even possible to unschedule a file from deletion by calling NtSetInformationFile(FileDispositionInformationEx,FILE_DISPOSITION_DO_NOT_DELETE)!

It's possible to switch to POSIX semantics with NtSetInformationFile(FileDispositionInformationEx,FILE_DISPOSITION_DELETE|FILE_DISPOSITION_POSIX_SEMANTICS) to disallow opening new handles as soon as the one handle for which the call was made is closed. Already opened handles (by the same or other threads/processes) can be used normally until closed. Unscheduling from deletion doesn't work in this case. And it's possible to create a new file with the same name before all other handles are closed, i.e. the POSIX way.

In some release of Windows 10, the default behavior for the Win32 DeleteFile() function was changed to POSIX semantics. This can be tested with Windows Explorer, for example. The del command from the CMD.EXE shell still works with traditional Windows NT semantics, likely for backwards compatibility with batch scripts.

I thought I'd followed up to thank you for the extra details.

For the wrapping, inspecting test logs revealed that fossil could not determine the terminal width at all on OmniOS. I added an extra include in src/terminal.c in the omnios-terminal-width branch.

I think that's safe to do, but testing is welcome.

The JSON tests seem to check if anonymous has "m", "n" and "c" capabilities. I haven't been able to track back if that was ever the case, but currently it is not.

It is very possible that those were set for anonymous at some point. The JSON tests are mostly about 10 years old, so finding that out for sure would require digging around in ancient history.

Does anyone knows the history of the json tests to confirm if they can be updated to test for just "hz"?

That particular history is remembered only by fossil itself and is no longer really relevant, but updating them for the current anonymous defaults is absolutely the right thing to do.

FWIW, I've been running the test suite through a few systems by now.

If I merge the test-fixes-2.24 branch with testing-improvements as well as the other branches with changes prompted by the tests (auto-def-compilation-flags, omnios-terminal-width, signed-chars and th-int-min-errors) plus trunk as of yesterday, it's all clean.

FWIW, I improved the rewrite script in the testing-improvements branch. It rewrites hashes, paths and more in the output.

If running tests in -verbose mode, and rewrite with a new -extra flag, the resulting output is similar across OSes for me:

    % shasum * | sort
    a62aae03789720f7e9ff7ceb5004fa0ee3b29fff  DragonFly-x86_64-6.4.0.test
    c4e55bb01482ad73f30f58843ecfde7813bb5643  Archlinux-x86_64-NA.test
    c4e55bb01482ad73f30f58843ecfde7813bb5643  OpenIndiana-i386-illumos-67fa3f2c31.test
    c4e55bb01482ad73f30f58843ecfde7813bb5643  Rocky-aarch64-9.3.test
    e2e97a30a9300944f6e3cc9758efb6f5965594be  OpenBSD-aarch64-7.4.test
    e2e97a30a9300944f6e3cc9758efb6f5965594be  OpenBSD-amd64-7.4.test
    e45151953cc22ee98ecd25d614128a1eeada50ee  Alpine-aarch64-3.18.4.test
    e45151953cc22ee98ecd25d614128a1eeada50ee  Debian-aarch64-12.2.test
    e45151953cc22ee98ecd25d614128a1eeada50ee  Fedora-aarch64-38.test
    e45151953cc22ee98ecd25d614128a1eeada50ee  Fedora-aarch64-39.test
    e45151953cc22ee98ecd25d614128a1eeada50ee  Fedora-x86_64-39.test
    e45151953cc22ee98ecd25d614128a1eeada50ee  FreeBSD-arm64-14.0.test
    e45151953cc22ee98ecd25d614128a1eeada50ee  MacOSX-arm64-13.6.3.test
    e45151953cc22ee98ecd25d614128a1eeada50ee  NetBSD-evbarm-10.0_RC1.test
    e45151953cc22ee98ecd25d614128a1eeada50ee  OmniOS-i386-r151046ac.test
    e45151953cc22ee98ecd25d614128a1eeada50ee  TinyCore-x86_64-14.0.test
    e45151953cc22ee98ecd25d614128a1eeada50ee  Ubuntu-aarch64-22.04.test
    e45151953cc22ee98ecd25d614128a1eeada50ee  Ubuntu-aarch64-23.10.test
    e45151953cc22ee98ecd25d614128a1eeada50ee  Void-aarch64-rolling.test
    e5d7cb808830311316bd748f9648445827be811d  Windows-x86_64-2022.test

Most, like Alpine linux, run all tests successfully.

The difference with Windows 2022 is simply an ignored test:

    % diff -u Alpine-aarch64-3.18.4.test Windows-x86_64-2022.test
    --- Alpine-aarch64-3.18.4.test	2023-12-12 15:01:41.579422104 +0100
    +++ Windows-x86_64-2022.test	2023-12-12 15:01:41.650941395 +0100
    @@ -154682,3 +154682,4 @@
     ***** Ignored failure: stash-3-2
     ***** Ignored failure: stash-3-2-show-1
     ***** Skipped test: merge5
    +***** Skipped test: symlinks

In another case, I don't have the SQL package for TCL on Arch linux:

    % diff -u Alpine-aarch64-3.18.4.test Archlinux-x86_64-NA.test
    --- Alpine-aarch64-3.18.4.test	2023-12-12 15:01:41.579422104 +0100
    +++ Archlinux-x86_64-NA.test	2023-12-12 15:01:41.631654700 +0100
    @@ -117355,10 +117355,6 @@
     three words now

     test th1-tcl-1 OK
    -fossil test-th-render --open-config /CHECKOUT/test/th1-tcl2.txt
    -RESULT (0): 5
    -
    -test th1-tcl-2 OK
     fossil test-th-render --open-config /CHECKOUT/test/th1-tcl3.txt
     RESULT (0): <hr><p class="thmainError">ERROR: invalid command name &quot;bad_command&quot;</p>
     test th1-tcl-3 OK

The other discrepancies are all due to the SQL TCL package, in various ways.

Is Windows-x86_64-2022.test a windows native build or a cygwin build (I assume the former)?

That's an MSVC build. Thanks for asking.

Next time I spin up a Windows VM, I'll see if I can try Cygwin too. I'm not familiar enough with MinGW to know what's a good environment to use there.

The other tests are a mix of multiple versions of GCC and some Clang.

I'm running a test now under cygwin/windows 10. I'm not using the mingGW cross compiler but native cygwin/unix emulation.

Started with trunk (as of an hour ago) and merged all the branches you specified.

It's in the test-delta-revert-90* tests. Been running for a bit. I'll edit this post with the results here when it's done.

Update:

I ran it using: time tclsh ./src/../test/tester.tcl fossil removing --quiet so I could see that it wasn't hanging.

After 100 minutes I saw:

/home/rouilj/develop/fossil/fossil whatis --type w tcltest-x-random-short
/home/rouilj/develop/fossil/fossil artifact 8d89fcdf12132050fb9f12489edd11c9369a40b1619002a48fcd720cd6dbc18f
test wiki-57 OK
test test-framework-wiki OK
***** End of wiki: 10 errors so far ******
Could not delete "/tmp/home_9009", error: error deleting "/tmp/home_9009": directory not empty
    while executing
"error "Could not delete \"$path\", error: $error""
    (procedure "robust_delete" line 15)
    invoked from within
"robust_delete $::tempHomePath"
    (procedure "delete_temporary_home" line 8)
    invoked from within
"delete_temporary_home"
    (file "./src/../test/tester.tcl" line 1128)

I assume this means that all tests passed. It looks like the cleanup at the end of testing is failing however.

I think the issue is here in test/tester.tcl:

proc delete_temporary_home {} {
  if {$::KEEP} {return}; # All cleanup disabled?
  if {$::is_windows} {
    robust_delete [file join $::tempHomePath _fossil]
  } else {
    robust_delete [file join $::tempHomePath .fossil]
  }
  robust_delete $::tempHomePath
}

The directory /tmp/home_9009 has an _fossil file. If I use the cygwin built fossil binary (rather than my own) to run fossil open a_repo.fossil, it creates an _FOSSIL_ file in the working directory.

So it appears that a cygwin fossil is a unique mix of windows/unix standards.

Note that $::tcl_platform(platform) returns unix so $::is_windows should be false.

If I look in src/db.c I see:

#if defined(_WIN32) || defined(__CYGWIN__)

around the block that select _fossil. I am not sure why this is the case, but I assume something similar happens causing _FOSSIL_ to be used in the working directory. (Maybe to support the 8.3 file name requirement of some windows filesystems.)

To solve the error, remove the if conditional and have delete_temporary_home call both robust_delete lines. A missing file shouldn't be an error.

You could also remove the entire if clause and replace robust_delete $::tempHomePath with robust_delete $::tempHomePath YES_DO_IT to recursively delete the directory.

I'm running a test now under cygwin/windows 10. I'm not using the mingGW cross compiler but native cygwin/unix emulation.

Thanks for testing.

I wanted to see if I could address your points, so am trying the same.

/home/rouilj/develop/fossil/fossil whatis --type w tcltest-x-random-short /home/rouilj/develop/fossil/fossil artifact 8d89fcdf12132050fb9f12489edd11c9369a40b1619002a48fcd720cd6dbc18f test wiki-57 OK test test-framework-wiki OK ***** End of wiki: 10 errors so far ******

This indicates that there was 10 failures up until now among all the tests.

Unfortunately, the delete_temporary_home problem prevented the summary from being shown.

The directory /tmp/home_9009 has an fossil file. If I use the cygwin built fossil binary (rather than my own) to run fossil open a_repo.fossil, it creates an _FOSSIL file in the working directory.

The _fossil fils is the global configuration file for the tests (.fossil on unix). This is different from the _FOSSIL_ file in a checkout (.fslckout on unix).

Note that $::tcl_platform(platform) returns unix so $::is_windows should be false.

That's indeed an issue here. I added a check for cygwin in test/tester.tcl in the testing-improvements branch. The check is based on $::tcl_platform(os).

BTW, to test if delete_temporary_home works, you can run a single test instead of waiting for all tests to complete.

I am running through the tests now, with the following setup under cygwin (Windows 2019, cygwin packages and a manually installed tcllib):

    $ fossil up trunk
    $ for b in test-fixes-2.24 testing-improvements auto-def-compilation-flags omnios-terminal-width signed-chars th-int-min-errors; do fossil merge $b; done
    $ ./configure --json --with-th1-docs --with-th1-hooks --with-tcl=1 --with-tcl-private-stubs=1
    $ make
    $ tclsh8.6 ./test/tester.tcl ./fossil.exe set-manifest th1-docs
    $ time tclsh8.6 ./test/tester.tcl ./fossil.exe -verbose >tests.log

The set-manifest and th1-docs tests is a quick test if tcl support works and that tcllib is available.

The full test suite is still running. I'll update when it completes.

The results are in:

***** End of wiki: 17 errors so far ******
***** Final results: 17 errors out of 40490 tests
***** Considered failures: th1-tcl-9 th1-globalState-15 unversioned-1 unversioned-28 unversioned-29 unversioned-30 unversioned-31 unversioned-32 unversioned-33 unversioned-37 unversioned-48 unversioned-49 unversioned-51 unversioned-52 test-framework-unversioned test-framework-utf test-framework-wiki
***** Ignored results: 7 ignored errors out of 40490 tests
***** Ignored failures: json-cap-POSTenv-name json-env-RC-1103-code merge5-sqlite3-issue stash-1-diff stash-WY-1-CODE stash-3-2 stash-3-2-show-1
***** Skipped tests: merge5

Haven't checked what fails, yet.

I ran another test overnight with edits to tester.tcl so I didn't get the error. Here is the output:

***** End of wiki: 10 errors so far ******
***** Final results: 10 errors out of 37899 tests
***** Considered failures: test-framework-diff test-framework-merge_renames test-framework-merge_renames_2 test-framework-mv-rm test-framework-revert test-framework-settings-repo test-framework-stash test-framework-symlinks test-framework-th1-repo th1-globalState-15
***** Ignored results: 1 ignored errors out of 37899 tests
***** Ignored failures: merge5-sqlite3-issue
***** Skipped tests: json merge5 set-manifest th1-docs th1-tcl unversioned

As you said 10 errors plus a little more.

Also I configured with:

configure --json --with-tcl-private-stubs --with-th1-hooks --with-openssl=auto --with-zlib=auto

Also I configured with:

configure --json --with-tcl-private-stubs --with-th1-hooks --with-openssl=auto --with-zlib=auto

To get TCL support, I think --with-tcl is required as well. To get all tests, I think you also need to (probably manually) install tcllib.

Anyway, I pushed some changes into the testing-improvements that fixes the errors I saw.

I tested on individual tests, the full suite is running (but takes long).

Including the various branches mentioned above, it ran clean:

***** End of wiki: 0 errors so far ******
***** Final results: 0 errors out of 40990 tests
***** Ignored results: 7 ignored errors out of 40990 tests
***** Ignored failures: json-cap-POSTenv-name json-env-RC-1103-code merge5-sqlite3-issue stash-1-diff stash-WY-1-CODE stash-3-2 stash-3-2-show-1
***** Skipped tests: merge5

Hi Preben:

Yeah the full suite takes quite a while.

I just did a fossil update followed by a fossil merge testing-improvements. Hopefully, this brings my copy up to date. Rebuilding now and I'll run a new test. The last run I did only failed on th1-globalState-15 which it looks like you fixed in your last updates.

To get TCL support, I think --with-tcl is required as well. To get all tests, I think you also need to (probably manually) install tcllib.

I thought using --with-tcl-private-stubs was the way to enable --with-tcl but using stubs to dynamically link against the tcl library.

My latest run reported:

***** Ignored results: 5 ignored errors out of 39423 tests
***** Ignored failures: merge5-sqlite3-issue stash-1-diff stash-WY-1-CODE stash-3-2 stash-3-2-show-1
***** Skipped tests: json merge5 set-manifest th1-docs th1-tcl unversioned

Are these expected if tcllib is not installed (or if tcl is not properly configured)?

I thought the test suite was supposed to be able to run under jimTcl. Does it include tcllib or will running with jimTcl also skip parts of the test?

I thought using --with-tcl-private-stubs was the way to enable --with-tcl but using stubs to dynamically link against the tcl library.

As I get it, the --with-tcl flag says to compile fossil with Tcl support (either with a path or using "1" to use a dynamic library when run later).

The configure run will then attempt to find the right way to interact with the library, where --with-tcl-private-stubs and --with-tcl-stubs influences jsut how this is done.

***** Ignored results: 5 ignored errors out of 39423 tests
***** Ignored failures: merge5-sqlite3-issue stash-1-diff stash-WY-1-CODE stash-3-2 stash-3-2-show-1
***** Skipped tests: json merge5 set-manifest th1-docs th1-tcl unversioned

Are these expected if tcllib is not installed (or if tcl is not properly configured)?

The merge5 test is currently aslways skipped and th1-docs need both --th1-docs and --with-tcl=X.

The other skipped tests would fail if packages found in tcllib are not present (and may fail for other reasons, too).

I thought the test suite was supposed to be able to run under jimTcl. Does it include tcllib or will running with jimTcl also skip parts of the test?

I have only used jimTcl with configure, when tclsh was not available on the system (it is included under autosetup/ for this purpose). No idea if it can be installed or used in a way where it can be the base for actual TCL integration in fossil.

On systems where I test, I install tcl with the local package mananger (except Windows when not using Cygwin) and tcllib as package or manually as needed.

Not sure if it can be made to work for testing. It might well work as the command to invoke the tester.tcl script, but I am not sure about the packaging aspect following that (and I sincerely doubt that the invoking script will influence whether Tcl support is enabled in fossil).

Florian, are you happy to commit test-fixes-2.24 at this point?

Absolutely! Thanks for your additional work on this!

Merged and closed.

I'll be fixing a fall out from the diff-deleted-files branch separately (that branch was merged later anyway, and I was not yet in the habit of running frequent tests when creating it).

For the test suite, I'd also like to integrate the testing-improvements branch.

If merged into the current trunk (where test-fixes-2.24 is integrated), the improvements are:

• There are no errors, if still 7 ignored failures (excluding an sqlite version error on OpenBSD and thread error in th1-docs on DragonFly BSD).
• Running make test now runs all tests, regardless of whether you build inside or outside the fossil checkout.
• Exit code is 1 on errors; useful with e.g. make test.
• Skipped tests are now included in the summary, making it easier to see how complete the coverage was.
• Several issues are fixed for testing under Cygwin.
• The unversioned tests no longer stalls on a password prompt.
• The symlinks test no longer requires a global setting.
• The th1-docs no longer requires the test to run inside the fossil checkout.

Additionally, for working on testing and comparing tests on different OSes, I also added:

• Tests and output is sorted consistently on different platforms.
• I added a rewrite-test-output.tcl that cleans up test output to make it possible to diff output of test output (including in -verbose testing where it may even highlight changes in output that is not explicitly tested for).

The rewrite script made it particularly easy to compare tests runs:

1. Run all tests in -verbose mode and save the output as tests.out
2. Clean up the ooutput with the rewrite script and save as tests.clean
3. Compute a SHA checksum for the test.clean file
4. Compare the checksums and use diff when not identical

The cleaned up outputs are generally identical across BSDs, Linux, macOS, Windows and Cygwin. Minor differences like skipping symlinks tests on Windows ar easy to confirm using diff.

Preben, I find all your work on the test suite (and a plethora of other things) highly commendable and look forward to seeing it landed on trunk. Yhanks!

Agreed. +1