feat: Visual Regression Testing

[gabriellsh]

Proposed changes (including videos or screenshots)

TODO:

• Verify CD pipeline
• Fix docker build cache
• Make this available as a "package" (tools)
• Fix requirements for "ready to merge"

Added visual-regression tests for fuselage package.

Added a new tool package for easy visual-regression tests setup.

This includes a docker setup necessary to ensure that the screenshots taken in different OSes match.

To run it:
yarn && yarn build
yarn workspace @rocket.chat/<PACKAGE> visual-regression

WARNING: Docker engine is required and has to be active when running the tests

To update the snapshots, ensure all packages have their dependencies installed and all packages are built, then run:
yarn workspace @rocket.chat/<PACKAGE> visual-regression-update

To run for every package:
yarn turbo run visual-regression

Enable for other packages:
Add @rocket.chat/visual-regression as a devDependency. "@rocket.chat/visual-regression": "workspace:~",
Add the following scripts inside package.json:

scripts: {
    "visual-regression": "visual-regression",
    "visual-regression-update": "visual-regression --update-snapshots",
}

Requirements: Package should have storybook already setup and the build-storybook script must exist for running on CI, since visual-regression depends on it.

Issue(s)

ARCH-1329

Further comments

Unfortunately it's currently impossible to implement these tests without using a docker container. Differences between OSes cause a lot of different issues, specially regarding font rendering/kerning. A lot of time was invested during this implementation trying to find a solution, but fonts never match (most of the times it's a 1px difference in the text positioning).

~Three new scripts where added, one to run the tests locally, one to update the snapshots locally, and one to run the tests on the CI. The CI script visual-regression-ci requires "build-storybook" to have been ran before the tests can be ran. It was made this way in order to optimize the CI and avoid building the storybook packages multiple times during the pipeline. ~

[changeset-bot]

🦋 Changeset detected

Latest commit: b080479

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@rocket.chat/fuselage	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

In general, the safest fix is to avoid building a single shell command string that embeds dynamic paths or arguments. Instead, use the argv form of child_process APIs (execFile/execFileSync, or spawn/spawnSync) where the command and its arguments are passed as an array and not interpreted by the shell. If a shell is truly needed, use exec/execSync with shell: true plus robust escaping, but that is more error-prone.

For this file, we can keep using npx and concurrently, but invoke them via the argument-vector form. We currently run:

• In CI:
  npx -y concurrently -k -s first -n "SB,TEST" "npx -y http-server storybook-static -s -p 6006" "npx -y wait-on tcp:127.0.0.1:6006 && ${runPlaywrightScript} ${updateSnapshots}"

• Locally:
  npx -y concurrently -k -s first -n "SB,TEST" "yarn storybook -p 6006 --ci --host 0.0.0.0" "npx -y wait-on tcp:127.0.0.1:6006 && ${runPlaywrightScript} ${updateSnapshots}"

The only unsafe part is ${runPlaywrightScript} (and potentially ${updateSnapshots}) inside the shell string passed to execSync. We can instead:

1. Import spawnSync (or execFileSync) from child_process.
2. Replace the string-form execSync(...) calls with spawnSync('npx', [...args...], options).
3. Keep concurrently and its subcommands as arguments to npx, but avoid interpolating runPlaywrightScript into a shell string.
   A minimal and safe change is to keep the existing concurrently command structure but pass it as arguments to npx in array form; we still need the shell inside concurrently for the chained wait-on && script command, but runPlaywrightScript will be added as an argument in a controlled way.

However, concurrently itself expects its commands as strings, and internally will invoke a shell. To keep behavior identical with a minimal change strictly confined to this file, the simplest robust fix is to safely quote runPlaywrightScript and updateSnapshots before interpolation. Yet this still leaves shell interpretation risks if quoting is incomplete.

A better and still minimal change is:

• Switch from execSync string to spawnSync with shell: true and pass a single command string but ensure that runPlaywrightScript is wrapped in single quotes and any single quotes in its value are escaped.
• Likewise, ensure updateSnapshots is either a fixed known flag (--update-snapshots or empty) or safely appended.

In this code, updateSnapshots is either a literal --update-snapshots or an empty string, so it is safe. Taint only flows through runPlaywrightScript. Therefore the best fix is:

• Introduce a small helper to safely shell-quote a path.
• Use that helper when interpolating runPlaywrightScript into the command string.

This keeps external behavior the same while eliminating misinterpretation due to spaces or metacharacters, and removes the CodeQL warning since the formerly tainted absolute path is now properly escaped.

Concretely in tools/visual-regression/bin/index.js:

• Add a shellQuote helper function near the top.
• Replace ${runPlaywrightScript} with ${shellQuote(runPlaywrightScript)} in both execSync command strings.
• Optionally, normalize updateSnapshots usage so that any spacing issues are handled (e.g., only add a leading space when non-empty), though currently it is safe.

In general, the problem is that execSync is being used with a single template string that mixes literal shell syntax with dynamic values, including an absolute path (runPlaywrightScript). To fix this without changing behavior, we should avoid constructing a single shell command string and instead invoke npx directly with an argument array via spawnSync/execFileSync (which do not go through a shell by default). That way, runPlaywrightScript and other arguments are passed as raw arguments and cannot change the structure of the command even if they contain spaces or shell metacharacters.

The best approach here is:

• Replace the two execSync calls with spawnSync from child_process.
• Call npx directly as the command and pass the concurrently invocation and its two subcommands as proper arguments.
• Keep cwd, env, and stdio options the same to preserve behavior.
• Check the exit status from spawnSync and throw if the child process fails, so the script’s failure semantics remain similar to execSync (which throws on non‑zero exit).

Concretely in tools/visual-regression/bin/index.js:

• Change the import to bring in spawnSync instead of execSync.
• Define the two subcommand strings (the Storybook command and the wait-on + Playwright command) as plain strings, but only ever pass them as arguments to npx concurrently, not as part of a shell string.
• Build an array like ['-y', 'concurrently', '-k', '-s', 'first', '-n', 'SB,TEST', storybookCmd, playwrightCmd] and pass that to spawnSync('npx', concurrentArgs, { ... }).
• Apply this both for the CI and non-CI branches, differing only in the Storybook command string and the env.PACKAGE_DIR value.