1 patch for repository http://darcs.net/screened:

patch 568880a10cd72220c5fc71c8b242388a842b5462
Author: Ben Franksen <ben.franksen@online.de>
Date:   Mon Jun 22 12:08:23 CEST 2020
  * extend test for issue2275 and mark (again) as failing

Attachments

• extend-test-for-issue2275-and-mark-_again_-as-failing.dpatch
• patch-preview.txt
• unnamed

Is it worth keeping the passing ones in a non-failing script?

> Is it worth keeping the passing ones in a non-failing script?

This would mean duplicating the test code, so I don't think it is
worthwhile. I factored the actual tests into a shell function so we can
easily test with and without using the index. The latter succeeds, while
the former fails. Also note that normally the index is used by default.
It is only when runing the tests that we disable it because it uses only
low (1 second) resolution file modification timestamps. Fixing this is
difficult because unix-compat does not support modificationTimeHiRes, so
we'd have to roll our own for Windows.

It just seems like a step backwards.

Before we had a test that we would keep working by default, and that
validated some slightly tricky correctness property.

Now we just have a failing test and no particular plan to fix it.

We could also keep the original test as-is and put all the new code
in a new test script.

> It just seems like a step backwards.
> 
> Before we had a test that we would keep working by default, and that 
> validated some slightly tricky correctness property.
> 
> Now we just have a failing test and no particular plan to fix it.

It cannot be that hard to fix that. The culprit is the index code i.e.
Darcs.Util.Index. We have to review that code and find the places where
we accept symbolic links. I guess that won't affect more than a hand
full of code lines. So there is a plan, sort of, but admittedly
not yet very concrete.

I agree that loosing the parts of the test that pass is bad.

> We could also keep the original test as-is and put all the new code 
> in a new test script.

I think a better option is to just disable the part where the tests are
run with --no-ignore-times and add a comment that this currently fails.
This is pretty easy to do. I think I'll send a patch to do that.

Following up on review / discussion.

1 patch for repository http://darcs.net/screened:

patch b1379884b275d9152302a580573b241020489cf4
Author: Ben Franksen <ben.franksen@online.de>
Date:   Tue Jul 21 14:37:41 CEST 2020
  * re-enable test for issue2275
    
  The test fails only if the index is used, that is, if --no-ignore-times is
  in effect. This part is now temporarily disabled until we fix that.

Attachments

• patch-preview.txt
• re_enable-test-for-issue2275.dpatch
• unnamed

Good plan, thanks.