fix(workbench): escape embedded newline before typing job script into console#536
Closed
ianpittwood wants to merge 1 commit into
Closed
fix(workbench): escape embedded newline before typing job script into console#536ianpittwood wants to merge 1 commit into
ianpittwood wants to merge 1 commit into
Conversation
… console _JOB_SCRIPT_CONTENT embeds a real newline between its two R statements. write_test_script only escaped quotes before typing the writeLines() call into the console, so the raw newline reached Playwright's type(), which aliases embedded \n/\r to the Enter key — submitting the command early and splitting it into two invalid R statements. The script file was never created, which the file.exists() check added in #528 now catches loudly as "VIP_JOB_SCRIPT_CHECK: FALSE". Escape the newline into the R string literal (like the quotes already are) so the console receives one line. Also assert in _run_console_command that no raw newline reaches type(), since its contract is a single-line command.
Contributor
|
Lol, I literally just fixed this! Sorry about that! The GHA is running now and should be resolved soon for the newest release! |
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes the Workbench Jobs tests’ console script-writing regression where an embedded newline in _JOB_SCRIPT_CONTENT was being typed into the RStudio console as an Enter keypress (via Playwright type()), splitting the writeLines() command and preventing the script file from being created.
Changes:
- Added
_escape_for_r_string_literal()to escape quotes and newline/CR characters before embedding content in a one-line R string literal. - Hardened
_run_console_command()with a contract check to prevent raw\n/\rfrom reaching Playwright typing. - Added a selftest to validate escaping removes raw newline and carriage-return characters.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| src/vip_tests/workbench/test_jobs.py | Adds newline/CR guard to console typing and introduces/uses an R string-literal escaping helper for the job script write step. |
| selftests/test_job_script_escaping.py | Adds a regression selftest validating the escaping helper removes raw newline/CR characters. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+218
to
+221
| assert "\n" not in r_cmd and "\r" not in r_cmd, ( | ||
| f"r_cmd contains a raw newline, which Playwright's type() would submit as " | ||
| f"a premature Enter keypress: {r_cmd!r}" | ||
| ) |
Comment on lines
+1
to
+8
| """Guard against a raw newline reaching the console as a premature Enter keypress. | ||
|
|
||
| ``_run_console_command`` types R commands via Playwright's ``type()``, which | ||
| aliases embedded "\n"/"\r" characters to the Enter key — a raw newline in the | ||
| command submits it early, splitting it into two invalid R statements (see the | ||
| regression where ``write_test_script``'s ``writeLines()`` call never created | ||
| the test job script). | ||
| """ |
Contributor
|
You should be able to use version 58.7, which won't just skip and should work on version 2026.06 and 2026.07 |
Contributor
|
Contributor
Author
|
Hahaha thanks @samcofer! |
|
Preview Links
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #535
Summary
_JOB_SCRIPT_CONTENTembeds a real newline between its two R statements.write_test_scriptonly escaped quotes before typing thewriteLines()call into the console, so that raw newline reached Playwright'stype(), which aliases embedded\n/\rto the Enter key — submitting the command early and splitting it into two invalid R statements. The script file was never created, which thefile.exists()check added in fix(workbench): harden Workbench Job flow against write/chooser races #528 now catches loudly asVIP_JOB_SCRIPT_CHECK: FALSE._escape_for_r_string_literal(), which escapes quotes and newlines/carriage-returns, used at thewriteLines()call site._run_console_command()now asserts no raw\n/\rreachestype(), enforcing its documented "single-line" contract for any future caller.Test plan
ruff check/ruff format --checkcleanmypycleanselftests/test_job_script_escaping.py) passes🤖 Generated with Claude Code