ci: don't skip smallest test slice in GitLab

The "ci/run-test-slice.sh" script can be used to slice up all of our
tests into N pieces and then run each of them on a separate CI job.
This is used by both GitLab and GitHub CI to speed up Windows tests,
which would otherwise be painfully slow.

The infra itself is fueled by `test-tool path-utils slice-tests`. This
tool receives as input an "offset" and a "stride" that can be combined
to slice up tests. This framing can be misleading though: you are
expected to pass a zero-based index as "offset", and the complete number
of slices to the "stride". The latter makes sense, but it is somewhat
surprising that the offset needs to be zero-based. And this is in fact
biting us: while GitHub passes zero-based indices, GitLab passes
`$CI_NODE_INDEX`, which is a one-based indice.

Ideally, we should have verification that the parameters make sense.
And naturally, one would for example expect that it's an error to call
the binary with an offset larger than the stride. But with the current
framing as "offset" it's not even wrong to do so, as it is of course
well-defined to start at a larger offset than the stride.

This means that we get this wrong on GitLab's CI, as we pass a one based
index there, and this causes us to skip one of the tests. Interestingly,
it's not the lexicographically first test that we skip. Instead, as we
sort tests by size before slicing them, we skip the _smallest_ test.

Reframe the problem to instead talk about "slice number" and "total
number of slices". For all of our use cases this is semantically
equivalent, but it allows us to perform some verifications:

  - The total number of slices must be greater than 1.

  - The selected slice must be between 1 <= nr <= slices_total.

As the indices are now one-based it means that GitLab's CI is fixed.
The GitHub workflow is updated accordingly.

Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de>
Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
index f2e93f54611b62268a441014d1b7e310e29875c4..ec1a6609816e935425699880796d4e513aaf440c 100644 (file)
--- a/.github/workflows/main.yml
+++ b/.github/workflows/main.yml
@@ -150,7 +150,7 @@ jobs:
     - uses: git-for-windows/setup-git-for-windows-sdk@v1
     - name: test
       shell: bash
-      run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10
+      run: . /etc/profile && ci/run-test-slice.sh $((${{matrix.nr}} + 1)) 10
     - name: print test failures
       if: failure() && env.FAILED_TEST_ARTIFACTS != ''
       shell: bash
@@ -237,7 +237,7 @@ jobs:
       shell: bash
       env:
         NO_SVN_TESTS: 1
-      run: . /etc/profile && ci/run-test-slice.sh ${{matrix.nr}} 10
+      run: . /etc/profile && ci/run-test-slice.sh $((${{matrix.nr}} + 1)) 10
     - name: print test failures
       if: failure() && env.FAILED_TEST_ARTIFACTS != ''
       shell: bash

diff --git a/t/helper/test-path-utils.c b/t/helper/test-path-utils.c
index f5f33751da620dc73af1b9c6da0728c3d94415e6..874542ec3462a5c4396da05c764d8b409f737ac9 100644 (file)
--- a/t/helper/test-path-utils.c
+++ b/t/helper/test-path-utils.c
@@ -477,14 +477,20 @@ int cmd__path_utils(int argc, const char **argv)
 
        if (argc > 5 && !strcmp(argv[1], "slice-tests")) {
                int res = 0;
-               long offset, stride, i;
+               long slice, slices_total, i;
                struct string_list list = STRING_LIST_INIT_NODUP;
                struct stat st;
 
-               offset = strtol(argv[2], NULL, 10);
-               stride = strtol(argv[3], NULL, 10);
-               if (stride < 1)
-                       stride = 1;
+               slices_total = strtol(argv[3], NULL, 10);
+               if (slices_total < 1)
+                       die("there must be at least one slice, got '%s'",
+                           argv[3]);
+
+               slice = strtol(argv[2], NULL, 10);
+               if (1 > slice || slice > slices_total)
+                       die("slice must be in the range 1 <= slice <= %ld, got '%s'",
+                           slices_total, argv[2]);
+
                for (i = 4; i < argc; i++)
                        if (stat(argv[i], &st))
                                res = error_errno("Cannot stat '%s'", argv[i]);
@@ -492,7 +498,7 @@ int cmd__path_utils(int argc, const char **argv)
                                string_list_append(&list, argv[i])->util =
                                        (void *)(intptr_t)st.st_size;
                QSORT(list.items, list.nr, cmp_by_st_size);
-               for (i = offset; i < list.nr; i+= stride)
+               for (i = slice - 1; i < list.nr; i+= slices_total)
                        printf("%s\n", list.items[i].string);
 
                return !!res;