Merge pull request #20 from jeromekelleher/more-tests-c

Fixup itoa testing
sgkit-dev · Jul 11, 2024 · 4c503c4 · 4c503c4
2 parents a0b5342 + 10a6f46
commit 4c503c4
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 5 deletions.
diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
@@ -66,3 +66,29 @@ jobs:
           # Make sure we don't have ``vcztools`` in the CWD
           cd tests
           python -m vcztools --help
+
+  c_test:
+    name: C tests
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - name: Install dependencies
+        run: |
+          sudo apt install -y ninja-build libcunit1-dev valgrind meson gcovr
+      - name: Build
+        working-directory: ./lib
+        run: |
+          meson setup -Db_coverage=true build
+      - name: Tests
+        working-directory: ./lib
+        run: |
+          ninja -C build test
+      - name: Show coverage
+        working-directory: ./lib
+        run: |
+          ninja -C build coverage-text
+          cat build/meson-logs/coverage.txt
+      - name: Valgrind 
+        working-directory: ./lib
+        run: |
+          valgrind --leak-check=full --error-exitcode=1 ./build/tests
diff --git a/lib/tests.c b/lib/tests.c
@@ -245,7 +245,6 @@ test_variant_encoder_fields_all_missing(void)
     vcz_variant_encoder_free(&writer);
 }
 
-// TODO also do this with larger values to hit all cases.
 static void
 test_itoa_small(void)
 {
@@ -262,6 +261,45 @@ test_itoa_small(void)
     }
 }
 
+static void
+test_itoa_pow10(void)
+{
+    char dest1[64], dest2[64];
+    int len1, len2;
+    int32_t j, k, power, value;
+
+    for (power = 0; power < 10; power++) {
+        j = (int32_t) pow(10, power);
+        for (k = -1; k < 2; k++) {
+            value = j + k;
+            len1 = sprintf(dest1, "%d", value);
+            len2 = vcz_itoa(dest2, value);
+            /* printf("%s %s\n", dest1, dest2); */
+            CU_ASSERT_STRING_EQUAL_FATAL(dest1, dest2);
+            CU_ASSERT_EQUAL(len1, len2);
+        }
+    }
+}
+
+static void
+test_itoa_boundary(void)
+{
+    char dest1[64], dest2[64];
+    int len1, len2;
+    size_t j;
+    int32_t value;
+    int32_t cases[] = { INT32_MIN, INT32_MAX };
+
+    for (j = 0; j < sizeof(cases) / sizeof(*cases); j++) {
+        value = cases[j];
+        len1 = sprintf(dest1, "%d", value);
+        len2 = vcz_itoa(dest2, value);
+        /* printf("%s %s\n", dest1, dest2); */
+        CU_ASSERT_STRING_EQUAL(dest1, dest2);
+        CU_ASSERT_EQUAL(len1, len2);
+    }
+}
+
 static void
 test_ftoa(void)
 {
@@ -407,6 +445,8 @@ main(int argc, char **argv)
         { "test_variant_encoder_minimal", test_variant_encoder_minimal },
         { "test_variant_fields_all_missing", test_variant_encoder_fields_all_missing },
         { "test_itoa_small", test_itoa_small },
+        { "test_itoa_pow10", test_itoa_pow10 },
+        { "test_itoa_boundary", test_itoa_boundary },
         { "test_ftoa", test_ftoa },
         { NULL, NULL },
     };

diff --git a/lib/vcf_encoder.c b/lib/vcf_encoder.c
@@ -8,8 +8,9 @@
 #include <math.h>
 
 int
-vcz_itoa(char *buf, int32_t value)
+vcz_itoa(char *buf, int32_t v)
 {
+    int64_t value = v;
     int p = 0;
     int j, k;
 
@@ -40,6 +41,10 @@ vcz_itoa(char *buf, int32_t value)
             k = 7;
         } else if (value < 1000000000) {
             k = 8;
+        } else if (value < 10000000000) {
+            k = 9;
+        } else {
+            assert(false);
         }
 
         // iterate backwards in buf
@@ -76,9 +81,9 @@ vcz_ftoa(char *buf, float value)
         return p + 3;
     }
 
-     /* integer part */
+    /* integer part */
     i = (int64_t) round(((double) value) * 1000);
-    p += vcz_itoa(buf + p, (int32_t) (i / 1000));
+    p += vcz_itoa(buf + p, (int32_t)(i / 1000));
 
     /* fractional part */
     d3 = i % 10;

diff --git a/tests/test_vcf_roundtrip.py b/tests/test_vcf_roundtrip.py
@@ -17,7 +17,7 @@ def vcz_path_cache(vcf_path):
         cache_path.mkdir()
     cached_vcz_path = (cache_path / vcf_path.name).with_suffix(".vcz")
     if not cached_vcz_path.exists():
-        vcf2zarr.convert([vcf_path], cached_vcz_path, worker_processes=0)
+        vcf2zarr.convert([vcf_path], cached_vcz_path, worker_processes=0, local_alleles=False)
     return cached_vcz_path