OpenBSD test debugging

[imsodin (Simon)]

@vext01

The problem is, I (and other packagers probably too) don’t have the insider knowledge to distinguish a boring test suite bug from a real bug. It could be a racy clean up, or something which would delete user data if ignored.

Understandable, there could be a notice on “meta test failures”. The implicit notice usually is, that real test failures have a meaningful message (however I won’t vouch for that ).

@vext01

I ran your branch on constant testing for about an hour and a half. Here’s what now fails:

TestAutoAcceptNewFolderPremutationsNoPanic
TestIssue5063
TestRequestRemoteRenameChanged

I pushed one more change which might have fixed 5063. The auto accept one is a directory not empty failure (more on that below). And RemoteRenameChanged is “real”, I am trying to reproduce with running in a loop locally, no luck so far.

@vext01

I probably won’t dig deeper regarding the directory not empty stuff

That’s a shame. It looks like this is the source of the failures.

Well, actually you do the brunt of the work, so why not. You just have to be fine with me not giving any promises.

To get more info on what’s going on with the directory not empty failures, I need logs with fs debug facility enabled. For the RequestRemoteRenameChanged with model (for good measure, just always enable model). To reduce test times and log sizes I’d do the test loops similar to this to only run the relevant test and only save the latest output: i=0; while [ $i -lt 1000 ]; do i=$((i+1)); echo "Try $i"; STTRACE=model go test -short -race -tags purego -run TestRequestRemoteRenameChanged 2>&1 > test.out || break; done

@vext01

Didn’t you say in an earlier PR that something was returning an error code that should be impossible? Do you still believe this to be the case?

I don’t remember what exactly I said, but I am pretty certain I used should for a reason xD
Probably it was about RemoveAll returning directory not empty. At some point I thought golangs os implementation was “atomic”, but then I found a bug report that said that it is expected and possible that another goroutine can interfere with the execution of RemoveAll and thus cause this error.

@vext01

Cheers, and sorry again for the persistence (I got another email from a user today asking for syncthing to be updated in the pacakages collection).

No worries about the persistence.

Are you delaying updating Syncthing due to this? If so really just don’t please. There are known bugs fixed, while our test coverage isn’t nearly 100%. And even if it was, they might not cover all possible scenarios where something can in some way go wrong. If you want it more concrete: Out of all the failures discussed here, only RequestRemoteR… might still be a real bug (I doubt it) - all the rest are not. On the other hand Incorrectly out of sync, sequence number oddness. · Issue #5340 · syncthing/syncthing · GitHub is certainly a problem, and it’s fixed in v1.0.1 and existed for a long time.