瀏覽代碼

Fixed a crazy consistency issue in test.py

The root of the problem was the notorious Python quirk with mutable
default parameters. The default defines for the TestSuite class ended
up being mutated as the class determined the permutations to test,
corrupting other test's defines.

However, the only define that was mutated this way was the CACHE_SIZE
config in test_entries.

The crazy thing was how this small innocuous change would cause
"./scripts/test.py -nr test_relocations" and "./scripts/test.py -nr"
to drift out of sync only after a commit spanning the different cache
sizes would be written out with a different number of prog calls. This
offset the power-cycle counter enough to cause one case to make it to
an erase, and the other to not.

Normally, the difference between a successful/unsuccessful erase
wouldn't change the result of a test, but in this case it offset the
revision count used for wear-leveling, causing one run run expand the
superblock and the other to not.

This change to the filesystem would then propogate through the rest of
the test, making it difficult to reproduce test failures.

Fortunately the fix was to just make a copy of the default define
dictionary. This should also prevent accidently mutating of dicts
belonging to our caller.

Oh, also fixed a buffer overflow in test_files.
Christopher Haster 5 年之前
父節點
當前提交
52ef0c1c9e
共有 3 個文件被更改,包括 8 次插入8 次删除
  1. 3 3
      scripts/test_.py
  2. 2 2
      testbd/lfs_testbd.c
  3. 3 3
      tests_/test_files.toml

+ 3 - 3
scripts/test_.py

@@ -304,8 +304,8 @@ class ReentrantTestCase(TestCase):
 
             # exact cycle we should drop into debugger?
             if gdb and failure and failure.cycleno == cycles:
-                return super().test(gdb=gdb,
-                    persist=persist, failure=failure, **args)
+                return super().test(gdb=gdb, persist=persist, cycles=cycles,
+                    failure=failure, **args)
 
             # run tests, but kill the program after prog/erase has
             # been hit n cycles. We exit with a special return code if the
@@ -327,7 +327,7 @@ class TestSuite:
             self.name = self.name[:-len('.toml')]
         self.path = path
         self.classes = classes
-        self.defines = defines
+        self.defines = defines.copy()
         self.filter = filter
 
         with open(path) as f:

+ 2 - 2
testbd/lfs_testbd.c

@@ -199,7 +199,7 @@ int lfs_testbd_prog(const struct lfs_config *cfg, lfs_block_t block,
         bd->power_cycles -= 1;
         if (bd->power_cycles == 0) {
             // sync to make sure we persist the last changes
-            lfs_testbd_rawsync(cfg);
+            assert(lfs_testbd_rawsync(cfg) == 0);
             // simulate power loss
             exit(33);
         }
@@ -241,7 +241,7 @@ int lfs_testbd_erase(const struct lfs_config *cfg, lfs_block_t block) {
         bd->power_cycles -= 1;
         if (bd->power_cycles == 0) {
             // sync to make sure we persist the last changes
-            lfs_testbd_rawsync(cfg);
+            assert(lfs_testbd_rawsync(cfg) == 0);
             // simulate power loss
             exit(33);
         }

+ 3 - 3
tests_/test_files.toml

@@ -60,7 +60,7 @@ code = '''
 [[case]] # rewriting files
 define.SIZE1 = [32, 8192, 131072, 0, 7, 8193]
 define.SIZE2 = [32, 8192, 131072, 0, 7, 8193]
-define.CHUNKSIZE = [31, 16, 1, 1025]
+define.CHUNKSIZE = [31, 16, 1]
 code = '''
     lfs_format(&lfs, &cfg) => 0;
 
@@ -142,7 +142,7 @@ code = '''
 [[case]] # appending files
 define.SIZE1 = [32, 8192, 131072, 0, 7, 8193]
 define.SIZE2 = [32, 8192, 131072, 0, 7, 8193]
-define.CHUNKSIZE = [31, 16, 1, 1025]
+define.CHUNKSIZE = [31, 16, 1]
 code = '''
     lfs_format(&lfs, &cfg) => 0;
 
@@ -219,7 +219,7 @@ code = '''
 [[case]] # truncating files
 define.SIZE1 = [32, 8192, 131072, 0, 7, 8193]
 define.SIZE2 = [32, 8192, 131072, 0, 7, 8193]
-define.CHUNKSIZE = [31, 16, 1, 1025]
+define.CHUNKSIZE = [31, 16, 1]
 code = '''
     lfs_format(&lfs, &cfg) => 0;