Fossil Forum

Failing test cases for wiki & stash
Login

Failing test cases for wiki & stash

Failing test cases for wiki & stash

(1) By anonymous on 2021-09-28 10:38:11 [source]

Wiki test

The wiki tests currently fail in wiki-10. This is due to change 88e5336007.

Solution 1

Adding a !fTechnote makes the test pass again, but still references a non-existant column in that query (where wrid is assigned).

Index: src/wiki.c
==================================================================
--- src/wiki.c
+++ src/wiki.c
@@ -2439,11 +2439,11 @@
       );
     }
     while( db_step(&q)==SQLITE_ROW ){
       const char *zName = db_column_text(&q, 0);
       const int wrid = db_column_int(&q, 2);
-      if(!showAll && !wrid){
+      if(!showAll && !wrid && !fTechnote){
         continue;
       }
       if( showIds ){
         const char *zUuid = db_column_text(&q, 1);
         fossil_print("%s ",zUuid);

Solution 2

Another option would be to add an additional column to the query in the fTechnote!=1 case which is always 1.

Index: src/wiki.c
==================================================================
--- src/wiki.c
+++ src/wiki.c
@@ -2428,11 +2428,11 @@
     verify_all_options();
     if (fTechnote==0){
       db_prepare(&q, listAllWikiPages/*works-like:""*/);
     }else{
       db_prepare(&q,
-        "SELECT datetime(e.mtime), substr(t.tagname,7)"
+        "SELECT datetime(e.mtime), substr(t.tagname,7), 1"
          " FROM event e, tag t"
         " WHERE e.type='e'"
           " AND e.tagid IS NOT NULL"
           " AND t.tagid=e.tagid"
         " ORDER BY e.mtime DESC /*sort*/"

Stash test

The other failing test is in stash-1-show. This is due to change ed06585f41ee2ace. The reason is that the SELECTs in stash_apply and stash_pop match the hash column to blob.uuid, but hash is NULL in the case of added files. A possible solution is this:

Index: src/stash.c
==================================================================
--- src/stash.c
+++ src/stash.c
@@ -308,12 +308,14 @@
 static void stash_apply(int stashid, int nConflict){
   int vid;
   Stmt q;
   db_prepare(&q,
      "SELECT blob.rid, isRemoved, isExec, isLink, origname, newname, delta"
-     "  FROM stashfile, blob WHERE stashid=%d AND blob.uuid=stashfile.hash",
-     stashid
+     "  FROM stashfile, blob WHERE stashid=%d AND blob.uuid=stashfile.hash"
+     " UNION ALL SELECT 0, isRemoved, isExec, isLink, origname, newname, delta"
+     "  FROM stashfile WHERE stashid=%d AND stashfile.hash IS NULL",
+     stashid, stashid
   );
   vid = db_lget_int("checkout",0);
   db_multi_exec("CREATE TEMP TABLE sfile(pathname TEXT PRIMARY KEY %s)",
                 filename_collation());
   while( db_step(&q)==SQLITE_ROW ){
@@ -412,12 +414,14 @@
   int bWebpage = (pCfg->diffFlags & (DIFF_WEBPAGE|DIFF_JSON|DIFF_TCL))!=0;
   blob_zero(&empty);
   diff_begin(pCfg);
   db_prepare(&q,
      "SELECT blob.rid, isRemoved, isExec, isLink, origname, newname, delta"
-     "  FROM stashfile, blob WHERE stashid=%d AND blob.uuid=stashfile.hash",
-     stashid
+     "  FROM stashfile, blob WHERE stashid=%d AND blob.uuid=stashfile.hash"
+     " UNION ALL SELECT 0, isRemoved, isExec, isLink, origname, newname, delta"
+     "  FROM stashfile WHERE stashid=%d AND stashfile.hash IS NULL",
+     stashid, stashid
   );
   while( db_step(&q)==SQLITE_ROW ){
     int rid = db_column_int(&q, 0);
     int isRemoved = db_column_int(&q, 1);
     int isLink = db_column_int(&q, 3);

(2) By Stephan Beal (stephan) on 2021-09-28 12:07:26 in reply to 1 [link] [source]

The wiki tests currently fail in wiki-10. This is due to change 88e5336007.

Thank you! Here's an alternative fix:

Index: src/wiki.c
==================================================================
--- src/wiki.c
+++ src/wiki.c
@@ -2428,11 +2428,11 @@
     verify_all_options();
     if (fTechnote==0){
       db_prepare(&q, listAllWikiPages/*works-like:""*/);
     }else{
       db_prepare(&q,
-        "SELECT datetime(e.mtime), substr(t.tagname,7)"
+        "SELECT datetime(e.mtime), substr(t.tagname,7), e.objid"
          " FROM event e, tag t"
         " WHERE e.type='e'"
           " AND e.tagid IS NOT NULL"
           " AND t.tagid=e.tagid"
         " ORDER BY e.mtime DESC /*sort*/"

Because that query sorts newest first, e.objid (blob.rid of the technote) will give the equivalent result as that column gets for the wiki page query case.

i'll have that checked in momentarily.

Thank you for the report!

(3) By anonymous on 2021-10-04 12:19:18 in reply to 2 [link] [source]

Thanks!

What about the stash test?

(4) By anonymous on 2021-10-11 08:16:58 in reply to 1 [link] [source]

Stash test are still failing with the newest version 1dc5e1ce6d. This still makes them pass again.

Index: src/stash.c
==================================================================
--- src/stash.c
+++ src/stash.c
@@ -308,12 +308,14 @@
 static void stash_apply(int stashid, int nConflict){
   int vid;
   Stmt q;
   db_prepare(&q,
      "SELECT blob.rid, isRemoved, isExec, isLink, origname, newname, delta"
-     "  FROM stashfile, blob WHERE stashid=%d AND blob.uuid=stashfile.hash",
-     stashid
+     "  FROM stashfile, blob WHERE stashid=%d AND blob.uuid=stashfile.hash"
+     " UNION ALL SELECT 0, isRemoved, isExec, isLink, origname, newname, delta"
+     "  FROM stashfile WHERE stashid=%d AND stashfile.hash IS NULL",
+     stashid, stashid
   );
   vid = db_lget_int("checkout",0);
   db_multi_exec("CREATE TEMP TABLE sfile(pathname TEXT PRIMARY KEY %s)",
                 filename_collation());
   while( db_step(&q)==SQLITE_ROW ){
@@ -412,12 +414,14 @@
   int bWebpage = (pCfg->diffFlags & (DIFF_WEBPAGE|DIFF_JSON|DIFF_TCL))!=0;
   blob_zero(&empty);
   diff_begin(pCfg);
   db_prepare(&q,
      "SELECT blob.rid, isRemoved, isExec, isLink, origname, newname, delta"
-     "  FROM stashfile, blob WHERE stashid=%d AND blob.uuid=stashfile.hash",
-     stashid
+     "  FROM stashfile, blob WHERE stashid=%d AND blob.uuid=stashfile.hash"
+     " UNION ALL SELECT 0, isRemoved, isExec, isLink, origname, newname, delta"
+     "  FROM stashfile WHERE stashid=%d AND stashfile.hash IS NULL",
+     stashid, stashid
   );
   while( db_step(&q)==SQLITE_ROW ){
     int rid = db_column_int(&q, 0);
     int isRemoved = db_column_int(&q, 1);
     int isLink = db_column_int(&q, 3);

(5) By anonymous on 2021-10-18 14:10:37 in reply to 4 [link] [source]

This fix has been tested on Alpine Linux and OpenBSD.

(6) By Stephan Beal (stephan) on 2021-10-19 04:12:16 in reply to 5 [link] [source]

This fix has been tested on Alpine Linux and OpenBSD.

Being completely unfamiliar with the stash internals, i'm extremely hesitant to go modify them based on an anonymous passer-by's word (no offense intended).

Can you explain (to someone who isn't well-versed in the stash internals) and/or justify why the stash code needs to be changed instead of the failing tests?

(7) By anonymous on 2021-10-19 12:02:50 in reply to 6 [link] [source]

None taken.

What does the failing test check?

There are eleven stash tests failing in total. The first one is the stash-1-show test. Here is a side-by-side diff between the output from fossil on the left and the expected output on the right:

DELETE f1                                DELETE f1
Index: f1                                Index: f1
======================================   ======================================
--- f1                                   --- f1
+++ f1                                   +++ f1
@@ -1,1 +0,0 @@                          @@ -1,1 +0,0 @@
-f1                                      -f1
                                         
CHANGED f2                               CHANGED f2
--- f2                                   --- f2
+++ f2                                   +++ f2
@@ -1,1 +1,1 @@                          @@ -1,1 +1,1 @@
-f2                                      -f2
+f2.1                                    +f2.1
                                         
CHANGED f3n                              CHANGED f3n
--- f3n                                  --- f3n
+++ f3n                                  +++ f3n
                                         
                                       | ADDED f0
                                       > Index: f0
                                       > ======================================
                                       > --- f0
                                       > +++ f0
                                       > @@ -0,0 +1,1 @@
                                       > +f0

The command tested here is fossil stash show which (from the helptext):

Show the contents of a stash as a diff against its baseline. (...)

The tests expect the addition of f0 to be shown, which I think is the correct (and expected) behavior.

What was the change that made the test fail?

Bisection revealed that the test started failing in commit ed06585f41:

Enhance the stash so that it stores hashes and no long depends on RID value. Do this is a way that is backwards compatible and transparent to the user. After running any "stash" command using this version of Fossil or later, the schema will automatically update and the stash should survive a subsequent RID renumbering event in the repository without damage.

The specific part of the checkin that broke the test are lines 415-419 (stash_diff) where (Q1)

  db_prepare(&q,
     "SELECT rid, isRemoved, isExec, isLink, origname, newname, delta"
     "  FROM stashfile WHERE stashid=%d",
     stashid
  );
was changed to this (Q2):
  db_prepare(&q,
     "SELECT blob.rid, isRemoved, isExec, isLink, origname, newname, delta"
     "  FROM stashfile, blob WHERE stashid=%d AND blob.uuid=stashfile.hash",
     stashid
  );
Note that there is an identical change in lines 311-314 inside the stash_apply function. More about that later.

What is going wrong?

If a newly added file is stashed (happens in lines 194 and following), the rid is set to zero and this part of the query (line 198) which sets the hash entry:

(SELECT uuid FROM blob WHERE rid=:rid)
is NULL. This means that newly added files are never SELECTed by Q2 because
     "SELECT blob.rid, isRemoved, isExec, isLink, origname, newname, delta"
     "  FROM stashfile, blob WHERE stashid=%d AND blob.uuid=stashfile.hash",
                               this is NULL for added files ^^^^^^^^^^^^^^

How does this patch help?

This patch helps by extending the the query to include files which have a hash of NULL i.e. are new. rid is set to zero which is correct because they are not in the blob table. The ALL in UNION ALL SELECT is not strictly necessary, but seems to keep the output in the same order as the test expect.

So fixing the function stash_diff with this ``` @@ -412,12 +414,14 @@ int bWebpage = (pCfg->diffFlags & (DIFF_WEBPAGE|DIFF_JSON|DIFF_TCL))!=0; blob_zero(&empty); diff_begin(pCfg); db_prepare(&q, "SELECT blob.rid, isRemoved, isExec, isLink, origname, newname, delta"

  • " FROM stashfile, blob WHERE stashid=%d AND blob.uuid=stashfile.hash",
  • stashid
  • " FROM stashfile, blob WHERE stashid=%d AND blob.uuid=stashfile.hash"
  • " UNION ALL SELECT 0, isRemoved, isExec, isLink, origname, newname, delta"
  • " FROM stashfile WHERE stashid=%d AND stashfile.hash IS NULL",
  • stashid, stashid ); while( db_step(&q)==SQLITE_ROW ){ int rid = db_column_int(&q, 0); int isRemoved = db_column_int(&q, 1); int isLink = db_column_int(&q, 3); ``` fixes five of those eleven errors.

The other errors come from stash_apply suffering from exactly the same problem: It selects the files to apply the changes to the checkout, but doesn't SELECT newly added files for the same reason. The same change makes them show up. Line 331 shows that the code knows how to deal with files having rid 0. So this patch ``` @@ -308,12 +308,14 @@ static void stash_apply(int stashid, int nConflict){ int vid; Stmt q; db_prepare(&q, "SELECT blob.rid, isRemoved, isExec, isLink, origname, newname, delta"

  • " FROM stashfile, blob WHERE stashid=%d AND blob.uuid=stashfile.hash",
  • stashid
  • " FROM stashfile, blob WHERE stashid=%d AND blob.uuid=stashfile.hash"
  • " UNION ALL SELECT 0, isRemoved, isExec, isLink, origname, newname, delta"
  • " FROM stashfile WHERE stashid=%d AND stashfile.hash IS NULL",
  • stashid, stashid ); vid = db_lget_int("checkout",0); db_multi_exec("CREATE TEMP TABLE sfile(pathname TEXT PRIMARY KEY %s)", filename_collation()); while( db_step(&q)==SQLITE_ROW ){ ``` fixes the remaining six failures.

Before/after comparison

Let's take a fossil binary before (fold) and after this patch is applied (fnew) and see how they behave:

$ fold init t.fossil
$ mkdir t && cd t && fold open ../t.fossil
$ echo "Failing stash tests" > README.md
$ fold add README.md
$ fold stash

# Enter a description of what is being stashed.  Lines beginning
# with "#" are ignored.  Stash comments are plain text except
# newlines are not preserved.
#
# Since no default text editor is set using EDITOR or VISUAL
# environment variables or the "fossil set editor" command,
# and because no comment was specified using the "-m" or "-M"
# command-line options, you will need to enter the comment below.
# Type "." on a line by itself when you are done:
Stash with added README.md
.
UNMANAGE README.md
$ rm README.md
$ fold stash show
$ fnew stash show
ADDED README.md
Index: README.md
==================================================================
--- README.md
+++ README.md
@@ -0,0 +1,1 @@
+Failing stash tests

$ ls
$ fold stash apply
$ ls
$ fnew stash apply
ADDED  README.md
 "fossil undo" is available to undo changes to the working checkout.
$ ls
README.md

(8) By Stephan Beal (stephan) on 2021-10-19 12:13:16 in reply to 7 [link] [source]

The specific part ....

Wow, that is an astounding amount of impressive detail, thank you. i will comb through that all in the next day or so and address the code appropriately.

Thank you for the detailed analysis!

(9) By Stephan Beal (stephan) on 2021-10-21 08:42:50 in reply to 8 [link] [source]

The specific part ....

Follow-up: your stash fix is now in trunk. Thank you again for the report, your persistence in getting this applied, and taking the time to explain the need for/utility of this fix in detail.