Re: [PR] test: keep Nightly workflow policy hermetic [maka]

["jackwener (via GitHub)" <gi...@apache.org>]

jackwener commented on code in PR #4282:
URL: https://github.com/apache/maka/pull/4282#discussion_r3890094085


##########
scripts/desktop-nightly-workflow-policy.test.mjs:
##########
@@ -133,8 +133,21 @@ test('the first Desktop Nightly creates its destination with rsync 3.1-compatibl
   const remoteBase = join(workspace, 'remote');
   await mkdir(remoteBase);
   try {
-    const localBootstrap = bootstrap.run.replace('--protect-args ', '');
-    const result = spawnSync('bash', ['-c', localBootstrap], {
+    const rsyncStub = `
+rsync() {
+  local arguments=("$@")
+  local argument
+  for argument in "\${arguments[@]}"; do
+    [[ "$argument" != "--mkpath" ]] || return 64
+  done
+  local argument_count="\${#arguments[@]}"
+  local source_path="\${arguments[$((argument_count - 2))]}"
+  local destination_path="\${arguments[$((argument_count - 1))]}"
+  mkdir -p "$destination_path"
+  cp -R "$source_path". "$destination_path"

Review Comment:
   **P3 — this line makes the stub more permissive than real rsync, which removes the property the test existed to prove.**
   
   Real rsync creates the *final* destination directory when its parent already exists; `--mkpath` was added in 3.2.3 precisely to create **missing parent components**. That distinction is the entire reason the #4280 fix works: `$BASE` exists, so `maka/` can be created without `--mkpath`, and `desktop/` then arrives from the source tree.
   
   `mkdir -p "$destination_path"` creates the destination unconditionally, including cases where several parent levels are missing. So the stub models an rsync that always succeeds, when the constraint being tested is exactly the one case where it would not.
   
   Concretely: if someone later pointed this bootstrap at a deeper path with two missing levels, this test would still pass while real rsync 3.1 would fail with the same error the fix was written to avoid. That is the regression this test exists to catch, and after this change it no longer catches it.
   
   Two ways to close it, smallest first:
   
   1. Make the stub faithful — fail unless the destination's parent exists, for example `[[ -d "$(dirname "$destination_path")" ]] || return 64` before the `mkdir`. One line, and it encodes the actual constraint rather than assuming it away.
   2. Keep real rsync when it is available and fall back to the stub only when it is absent. That preserves portability without giving up the real-behaviour evidence, which on #4280 was the thing that made this test stronger than string matching.
   
   Option 1 alone would restore what matters here.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@maka.apache.org
For additional commands, e-mail: commits-help@maka.apache.org