Stop sharding the gameplay tests twice on CI

README.md

@@ -83,6 +83,13 @@ took with `harness.takeScreenshot(...)` in a `gameplay-test-screenshots/`
 folder next to the results — which refer to them by a relative path, so a
 downloaded results file still points at the right images.
 
+A handful of tests end with a screenshot of what the game looks like once
+the thing they check has happened (the tank's target after the shell
+exploded on it, the inventory holding the seed that was harvested, the car
+past the finish line with the lap counter at 1...). They are a few kilobytes
+each and make a run readable at a glance, so they are worth adding to a test
+whose result is something you would want to look at.
+
 The tests of every game can also be run on a branch, without waiting for it to
 land on `main`, by triggering a CircleCI pipeline with the
 `run-all-gameplay-tests` parameter set to `true`. The number of parallel

examples/starting-3D-platformer/starting-3D-platformer.json

@@ -2764,7 +2764,11 @@
         "harness.assert(",
         "  coinIds().length < coinsBefore,",
         "  'The number of coins left in the level went down.'",
-        ");"
+        ");",
+        "",
+        "// A picture of what the game looks like once this has happened, kept with",
+        "// the test results.",
+        "await harness.takeScreenshot('the player standing where the coin was');"
       ]
     }
   ],

examples/starting-3d-car-racing/starting-3d-car-racing.json

@@ -2212,7 +2212,11 @@
         "  finalProgress.checkpoint === 1,",
         "  'The next checkpoint to reach is the first one of the new lap (it is ' +",
         "    finalProgress.checkpoint + ').'",
-        ");"
+        ");",
+        "",
+        "// A picture of what the game looks like once this has happened, kept with",
+        "// the test results.",
+        "await harness.takeScreenshot('the car just past the finish line');"
       ]
     }
   ],

examples/starting-3d-draggable-tiles/starting-3d-draggable-tiles.json

@@ -1308,7 +1308,11 @@
         "  'The piece is dropped on the cell it was dragged to (it is at ' +",
         "    Math.round(dropped.x) + ',' + Math.round(dropped.y) +",
         "    ', expected ' + targetX + ',' + targetY + ').'",
-        ");"
+        ");",
+        "",
+        "// A picture of what the game looks like once this has happened, kept with",
+        "// the test results.",
+        "await harness.takeScreenshot('the board with the piece dropped on its new cell');"
       ]
     },
     {

examples/starting-3d-rts-unit-selection/starting-3d-rts-unit-selection.json

@@ -1771,7 +1771,11 @@
         "  unitsThatMoved === positionsBeforeOrder.size,",
         "  'Every unit of the selection walks to where the group was ordered (' +",
         "    unitsThatMoved + ' of ' + positionsBeforeOrder.size + ' moved).'",
-        ");"
+        ");",
+        "",
+        "// A picture of what the game looks like once this has happened, kept with",
+        "// the test results.",
+        "await harness.takeScreenshot('the whole selection walking to where it was sent');"
       ]
     }
   ],

examples/starting-3d-tank/starting-3d-tank.json

@@ -2472,7 +2472,11 @@
         "harness.assert(",
         "  blownAway,",
         "  'The explosion blows the target away (it moved ' + Math.round(knockback) + 'px).'",
-        ");"
+        ");",
+        "",
+        "// A picture of what the game looks like once this has happened, kept with",
+        "// the test results.",
+        "await harness.takeScreenshot('the target after the shell exploded on it');"
       ]
     }
   ],

examples/starting-3d-tile-placement/starting-3d-tile-placement.json

@@ -1647,7 +1647,11 @@
         "  placedTiles() === 1,",
         "  'Clicking a cell that already holds a tile does not place another one (' +",
         "    placedTiles() + ' tiles on the board).'",
-        ");"
+        ");",
+        "",
+        "// A picture of what the game looks like once this has happened, kept with",
+        "// the test results.",
+        "await harness.takeScreenshot('the board with the tile placed on it');"
       ]
     }
   ],

examples/starting-first-person-farming/starting-first-person-farming.json

@@ -6511,7 +6511,11 @@
         "  getPlayer().rotationY > 2,",
         "  'The view really had to be lowered onto the patch (it ended at ' +",
         "    getPlayer().rotationY.toFixed(1) + ' degrees).'",
-        ");"
+        ");",
+        "",
+        "// A picture of what the game looks like once this has happened, kept with",
+        "// the test results.",
+        "await harness.takeScreenshot('the inventory holding the seed that was harvested');"
       ]
     }
   ],

examples/starting-first-person-shooter/starting-first-person-shooter.json

@@ -2831,7 +2831,11 @@
         "harness.assert(",
         "  knockedOver,",
         "  'Being shot knocks the target over (it moved ' + pushed.toFixed(1) + ' units).'",
-        ");"
+        ");",
+        "",
+        "// A picture of what the game looks like once this has happened, kept with",
+        "// the test results.",
+        "await harness.takeScreenshot('the target that was shot at');"
       ]
     }
   ],

examples/starting-platformer/starting-platformer.json

@@ -1292,7 +1292,11 @@
         "harness.assert(",
         "  remaining.length === coinsBefore - targetIds.length,",
         "  'Only the coins the player ran into were collected.'",
-        ");"
+        ");",
+        "",
+        "// A picture of what the game looks like once this has happened, kept with",
+        "// the test results.",
+        "await harness.takeScreenshot('the level once every coin has been collected');"
       ]
     }
   ],

examples/starting-top-down-rpg/starting-top-down-rpg.json

@@ -1679,7 +1679,11 @@
         "harness.assert(",
         "  !harness.getObjects('NPC').some((one) => one.id === npc.id),",
         "  'The NPC that was talked to is the one that left.'",
-        ");"
+        ");",
+        "",
+        "// A picture of what the game looks like once this has happened, kept with",
+        "// the test results.",
+        "await harness.takeScreenshot('the scene after the NPC was answered');"
       ]
     }
   ],

scripts/lib/ChangedProjectFiles.js

@@ -35,7 +35,7 @@ const runGitCommand = (command) => {
  * the base branch in the meantime are not reported as changes.
  * @param {Object} options
  * @param {string} options.baseRef For example `origin/main`.
- * @returns {{ changedFiles: string[], requiresFullRun: boolean } | null} Null
+ * @returns {{ changedFiles: string[], filesRequiringAFullRun: string[] } | null} Null
  * when the changed files could not be determined (the caller should then
  * test everything rather than nothing).
  */
@@ -65,10 +65,10 @@ const findChangedFiles = ({ baseRef }) => {
   }
 
   const changedFiles = diffOutput.split('\n').filter(Boolean);
-  const requiresFullRun = changedFiles.some((changedFile) =>
+  const filesRequiringAFullRun = changedFiles.filter((changedFile) =>
     FILES_REQUIRING_A_FULL_RUN.includes(changedFile)
   );
-  return { changedFiles, requiresFullRun };
+  return { changedFiles, filesRequiringAFullRun };
 };
 
 /**

scripts/run-gameplay-tests.js

@@ -19,8 +19,10 @@
  *                              `circleci tests split`).
  *   --list                     Print the project files that would be tested,
  *                              one per line, and exit. Nothing is run.
- *   --shard-index=0            Test only a slice of the projects. Defaults to
- *   --shard-total=1            CIRCLE_NODE_INDEX / CIRCLE_NODE_TOTAL.
+ *   --shard-index=0            Test only one slice of the projects, for
+ *   --shard-total=1            splitting a run by hand. NOT used by the CI,
+ *                              which splits with `circleci tests split` (and
+ *                              balances by timing) - see the note below.
  *   --gdevelop-branch=master   Branch of GDevelop to take the build from.
  *   --gdevelop-version=5.6.277 Skip reading the version from the branch.
  *   --work-dir=...             Where GDevelop is downloaded and extracted
@@ -70,10 +72,15 @@ const junitPath = path.resolve(
     path.join(repositoryPath, 'gameplay-tests-results/results.xml')
 );
 const timeoutMs = Number(args['timeout-ms']) || 15 * 60 * 1000;
-const shardTotal =
-  Number(args['shard-total'] || process.env.CIRCLE_NODE_TOTAL) || 1;
-const shardIndex =
-  Number(args['shard-index'] || process.env.CIRCLE_NODE_INDEX) || 0;
+// Sharding only happens when it is asked for explicitly. It used to default
+// to CIRCLE_NODE_INDEX/CIRCLE_NODE_TOTAL, which quietly cut the run down
+// twice on CI: `--list` returned only this container's quarter of the games,
+// `circleci tests split` then split that quarter again, and the run sharded
+// what was left once more - so each of 4 containers tested a single game
+// instead of a quarter of them. The CI splits with `circleci tests split`,
+// which balances by recorded timings; this script just runs what it is given.
+const shardTotal = Number(args['shard-total']) || 1;
+const shardIndex = Number(args['shard-index']) || 0;
 
 /**
  * Print an informational message. With `--list`, stdout is reserved for the
@@ -123,9 +130,12 @@ const getRestrictedProjectFiles = () => {
     );
     return null;
   }
-  if (changes.requiresFullRun) {
+  if (changes.filesRequiringAFullRun.length > 0) {
+    // A change to the runner or to the CI can break any game, so testing only
+    // the games whose project file changed would say nothing about it.
     log(
-      'ℹ️ The gameplay tests runner or the CI configuration changed: testing every game.'
+      `ℹ️ Testing every game: ${changes.filesRequiringAFullRun.join(', ')} ` +
+        'changed, and that affects how every game is tested.'
     );
     return null;
   }