diff --git a/selftests/test_config.py b/selftests/test_config.py index 41f74ae7..5b870a45 100644 --- a/selftests/test_config.py +++ b/selftests/test_config.py @@ -5,6 +5,7 @@ import pytest from vip.config import ( + DEFAULT_PUBLIC_CLONE_URL, AuthConfig, ConnectConfig, GitTestConfig, @@ -670,9 +671,12 @@ def test_git_test_absent_by_default(self): wc = WorkbenchConfig(url="https://workbench.example.com") assert wc.git_test is None - def test_git_test_absent_from_dict_when_key_missing(self): + def test_git_test_absent_from_dict_yields_default_public_repo(self): wc = WorkbenchConfig.from_dict({"url": "https://workbench.example.com"}) - assert wc.git_test is None + assert wc.git_test is not None + assert wc.git_test.clone_url == DEFAULT_PUBLIC_CLONE_URL + assert wc.git_test.auth_method == "none" + assert wc.git_test.token == "" def test_git_test_parsed_from_dict(self): wc = WorkbenchConfig.from_dict( @@ -695,10 +699,12 @@ def test_git_test_token_from_env(self, monkeypatch): assert wc.git_test is not None assert wc.git_test.token == "tok123" - def test_git_test_none_when_env_not_set_and_no_block(self, monkeypatch): - monkeypatch.delenv("VIP_GIT_TOKEN", raising=False) + def test_git_test_default_ignores_env_token(self, monkeypatch): + """auth_method='none' forces the default's token empty even with VIP_GIT_TOKEN set.""" + monkeypatch.setenv("VIP_GIT_TOKEN", "tok123") wc = WorkbenchConfig.from_dict({"url": "https://workbench.example.com"}) - assert wc.git_test is None + assert wc.git_test is not None + assert wc.git_test.token == "" class TestLoadConfigGitTest: @@ -719,7 +725,7 @@ def test_git_test_block_parsed(self, tmp_toml, monkeypatch): assert cfg.workbench.git_test.clone_url == "https://github.com/org/repo.git" assert cfg.workbench.git_test.auth_method == "https-token" - def test_git_test_block_absent_yields_none(self, tmp_toml): + def test_git_test_block_absent_yields_default_public_repo(self, tmp_toml): path = tmp_toml( """ [workbench] @@ -727,7 +733,10 @@ def test_git_test_block_absent_yields_none(self, tmp_toml): """ ) cfg = load_config(path) - assert cfg.workbench.git_test is None + assert cfg.workbench.git_test is not None + assert cfg.workbench.git_test.clone_url == DEFAULT_PUBLIC_CLONE_URL + assert cfg.workbench.git_test.auth_method == "none" + assert cfg.workbench.git_test.token == "" def test_git_test_token_from_env(self, tmp_toml, monkeypatch): monkeypatch.setenv("VIP_GIT_TOKEN", "env-token-xyz") diff --git a/selftests/test_workbench_exec.py b/selftests/test_workbench_exec.py index dd07203d..d9aac70c 100644 --- a/selftests/test_workbench_exec.py +++ b/selftests/test_workbench_exec.py @@ -28,6 +28,7 @@ _extract_between_markers, _make_sentinels, _parse_done_marker, + _read_file_r_expr, _split_marker, _strip_r_index, _wrap_python_expr, @@ -345,6 +346,39 @@ def test_returns_none_for_non_digit_suffix(self): content = "VIP_DONE_abc:" assert _parse_done_marker(content, "VIP_DONE_abc") is None + def test_raw_exit_code_parses_but_quoted_does_not(self): + """Regression guard: R's auto-print wraps a bare (un-cat'd) character + result in a trailing closing quote, turning ``...:0`` into ``...:0"``. + The raw form must parse as exit 0; the quoted form must be treated as + still-running rather than silently misparsed. This is why + ``_read_file_r_expr`` wraps its read expression in ``cat()`` -- so the + marker line R's console echoes back is always the raw form.""" + raw = "hello\nVIP_DONE_abc:0" + quoted = 'hello\nVIP_DONE_abc:0"' + assert _parse_done_marker(raw, "VIP_DONE_abc") == ("hello", 0) + assert _parse_done_marker(quoted, "VIP_DONE_abc") is None + + +# --------------------------------------------------------------------------- +# _read_file_r_expr +# --------------------------------------------------------------------------- + + +class TestReadFileRExpr: + def test_wraps_read_in_cat(self): + """The read expression must be cat()'d, not left as a bare/auto-printed + expression -- R's console auto-print wraps a bare character result in + quotes and escapes embedded newlines as literal ``\\n``, which broke + the done-marker exit-code parse (see TestParseDoneMarker above).""" + expr = _read_file_r_expr("/tmp/foo.txt") + assert expr.startswith("cat(") + assert "readLines(" in expr + assert "paste(" in expr + + def test_embeds_path(self): + expr = _read_file_r_expr("/tmp/vip_term_abc.txt") + assert '"/tmp/vip_term_abc.txt"' in expr + # --------------------------------------------------------------------------- # _detect_ide @@ -497,7 +531,9 @@ class TestReadFileRouting: def test_rstudio_calls_rstudio_eval(self, monkeypatch): page = MagicMock() monkeypatch.setattr(exec_mod, "_detect_ide", lambda p: "rstudio") - mock_rstudio_eval = MagicMock(return_value="[1] hello world") + # cat()'d output is raw -- no "[1]" index, no quoting -- so the mock + # return value here is the *raw* form, not R's auto-printed form. + mock_rstudio_eval = MagicMock(return_value="hello world") mock_positron_eval_r = MagicMock() mock_positron_eval_python = MagicMock() mock_editor_read = MagicMock() @@ -508,7 +544,9 @@ def test_rstudio_calls_rstudio_eval(self, monkeypatch): result = read_file(page, "/tmp/foo.txt", lang="r") - assert result == "hello world" # _strip_r_index applied + assert result == "hello world" + rstudio_expr = mock_rstudio_eval.call_args[0][1] + assert rstudio_expr.startswith("cat(") # not a bare auto-printed expr mock_rstudio_eval.assert_called_once() mock_positron_eval_r.assert_not_called() mock_positron_eval_python.assert_not_called() @@ -518,7 +556,8 @@ def test_positron_r_calls_positron_eval_r(self, monkeypatch): page = MagicMock() monkeypatch.setattr(exec_mod, "_detect_ide", lambda p: "positron") mock_rstudio_eval = MagicMock() - mock_positron_eval_r = MagicMock(return_value="[1] file contents") + # cat()'d output is raw -- no "[1]" index, no quoting. + mock_positron_eval_r = MagicMock(return_value="file contents") mock_positron_eval_python = MagicMock() mock_editor_read = MagicMock() monkeypatch.setattr(exec_mod, "rstudio_eval", mock_rstudio_eval) @@ -528,7 +567,9 @@ def test_positron_r_calls_positron_eval_r(self, monkeypatch): result = read_file(page, "/tmp/foo.txt", lang="r") - assert result == "file contents" # _strip_r_index applied + assert result == "file contents" + positron_expr = mock_positron_eval_r.call_args[0][1] + assert positron_expr.startswith("cat(") # not a bare auto-printed expr mock_positron_eval_r.assert_called_once() mock_rstudio_eval.assert_not_called() mock_positron_eval_python.assert_not_called() diff --git a/selftests/test_workbench_git_ops.py b/selftests/test_workbench_git_ops.py index 328fb886..b425ed18 100644 --- a/selftests/test_workbench_git_ops.py +++ b/selftests/test_workbench_git_ops.py @@ -1,4 +1,4 @@ -"""Selftests for the Workbench git-ops clone command construction. +"""Selftests for the Workbench git-ops clone command construction and skip gates. Covers ``_do_clone`` in ``vip_tests.workbench.test_git_ops``: verifies the shell command it builds removes any pre-existing clone directory before @@ -6,10 +6,23 @@ with "destination path already exists". No live Workbench deployment or Playwright browser required -- ``terminal_run`` is monkeypatched to capture the command string instead of executing it. + +Also covers the config/auth skip-gate wording (#483) and the Gherkin step +order guard (#479): every scenario in test_git_ops.feature must resolve the +Git config gate before attempting the (possibly flaky) login step, so a +read-only/config situation is reported deterministically instead of being +masked by a login bounce. """ from __future__ import annotations +from importlib.util import find_spec +from pathlib import Path + +import pytest + +from vip.config import GitTestConfig, VIPConfig, WorkbenchConfig +from vip.gherkin import parse_feature_file from vip_tests.workbench import test_git_ops as git_ops @@ -57,3 +70,115 @@ def fake_terminal_run(page, cmd, timeout=30_000, *, readback_lang="r"): assert clone_dir == "/home/vip/other-repo" assert "rm -rf /home/vip/other-repo" in captured["cmd"] + + +# --------------------------------------------------------------------------- +# #479 -- Gherkin step order guard +# --------------------------------------------------------------------------- + + +def _git_ops_feature_path() -> Path: + spec = find_spec("vip_tests") + assert spec and spec.submodule_search_locations + return Path(spec.submodule_search_locations[0]) / "workbench" / "test_git_ops.feature" + + +class TestFeatureStepOrder: + """The Git config gate must resolve before the (possibly flaky) login step. + + Otherwise a read-only/config situation is masked by a non-deterministic + login bounce, surfacing a misleading auth error instead of the real + config/read-only reason (#479). + """ + + def test_config_gate_precedes_login_in_every_scenario(self): + parsed = parse_feature_file(_git_ops_feature_path()) + assert parsed["scenarios"], "expected scenarios in test_git_ops.feature" + for scenario in parsed["scenarios"]: + steps = scenario["steps"] + config_idx = next( + i for i, s in enumerate(steps) if "the Git test config is available" in s + ) + login_idx = next( + i for i, s in enumerate(steps) if "Workbench is accessible and I am logged in" in s + ) + assert config_idx < login_idx, ( + f"scenario {scenario['title']!r}: 'the Git test config is available' " + f"must precede 'Workbench is accessible and I am logged in', " + f"got steps: {steps}" + ) + + def test_push_scenarios_check_pushing_before_login(self): + parsed = parse_feature_file(_git_ops_feature_path()) + push_scenarios = [s for s in parsed["scenarios"] if "push" in s["title"].lower()] + assert push_scenarios, "expected at least one push scenario" + for scenario in push_scenarios: + steps = scenario["steps"] + pushing_idx = next( + i for i, s in enumerate(steps) if "the Git test config supports pushing" in s + ) + login_idx = next( + i for i, s in enumerate(steps) if "Workbench is accessible and I am logged in" in s + ) + assert pushing_idx < login_idx, ( + f"scenario {scenario['title']!r}: 'the Git test config supports pushing' " + f"must precede 'Workbench is accessible and I am logged in', " + f"got steps: {steps}" + ) + + +# --------------------------------------------------------------------------- +# #483 -- skip-message wording +# --------------------------------------------------------------------------- + + +def _make_vip_config(git_test: GitTestConfig | None) -> VIPConfig: + wc = WorkbenchConfig(url="https://workbench.example.com", git_test=git_test) + return VIPConfig(workbench=wc) + + +class TestGitConfigAvailableSkipMessages: + def test_none_config_message_explains_public_vs_token(self): + """Defensive fallback: git_test is None only via direct construction.""" + cfg = _make_vip_config(None) + with pytest.raises(pytest.skip.Exception) as exc_info: + git_ops.git_config_available(cfg) + msg = exc_info.value.msg + assert "auth_method='none'" in msg + assert "no token required" in msg + assert "VIP_GIT_TOKEN" in msg + + def test_empty_clone_url_message_explains_public_vs_token(self): + cfg = _make_vip_config(GitTestConfig(clone_url="", auth_method="none")) + with pytest.raises(pytest.skip.Exception) as exc_info: + git_ops.git_config_available(cfg) + msg = exc_info.value.msg + assert "auth_method='none'" in msg + assert "no token" in msg + + def test_valid_none_config_does_not_skip(self): + cfg = _make_vip_config( + GitTestConfig( + clone_url="https://github.com/posit-dev/posit-cli.git", auth_method="none" + ) + ) + result = git_ops.git_config_available(cfg) + assert result is cfg.workbench.git_test + + +class TestGitConfigSupportsPushingSkipMessage: + def test_anonymous_auth_skip_message(self): + git_cfg = GitTestConfig(clone_url="https://github.com/org/repo.git", auth_method="none") + with pytest.raises(pytest.skip.Exception) as exc_info: + git_ops.git_config_supports_pushing(git_cfg) + msg = exc_info.value.msg + assert "auth_method='none' is anonymous (read-only)" in msg + assert "auth_method='https-token'" in msg + assert "VIP_GIT_TOKEN" in msg + + def test_https_token_auth_does_not_skip(self): + git_cfg = GitTestConfig( + clone_url="https://github.com/org/repo.git", auth_method="https-token", token="tok" + ) + # Should not raise/skip. + git_ops.git_config_supports_pushing(git_cfg) diff --git a/src/vip/config.py b/src/vip/config.py index e1ab5752..5270d540 100644 --- a/src/vip/config.py +++ b/src/vip/config.py @@ -168,13 +168,21 @@ def from_dict(cls, raw: dict) -> WorkbenchExtensionsConfig: ) +# Default anonymous-clone target used when [workbench.git_test] is absent, so +# the clone/connectivity scenarios have something to run against out-of-the-box. +DEFAULT_PUBLIC_CLONE_URL = "https://github.com/posit-dev/posit-cli.git" + + @dataclass class GitTestConfig: """Configuration for [workbench.git_test] Git operations testing. Enables terminal and GUI Git scenarios (clone, branch, commit, push) - against a real Git repository. All scenarios auto-skip when this block - is absent from the config file. + against a real Git repository. When this block is absent from the config + file, ``WorkbenchConfig.from_dict`` synthesizes a default pointing at a + public repository (``DEFAULT_PUBLIC_CLONE_URL``) with ``auth_method = + "none"``, so clone/connectivity scenarios run out-of-the-box; push/commit + scenarios skip as read-only until the user configures a token. Token is never stored in the TOML file. For ``auth_method = "https-token"`` set VIP_GIT_TOKEN in the environment (same pattern as @@ -242,6 +250,9 @@ class WorkbenchConfig(ProductConfig): idle_grace_seconds: int = 60 extensions: WorkbenchExtensionsConfig = field(default_factory=WorkbenchExtensionsConfig) kubernetes: WorkbenchKubernetesConfig = field(default_factory=WorkbenchKubernetesConfig) + # None only when constructed directly (e.g. in tests); ``from_dict`` always + # populates this, defaulting to an anonymous public-repo clone when + # [workbench.git_test] is absent from the TOML file. git_test: GitTestConfig | None = None # Base path of Chronicle's on-disk data on the Workbench server. Defaults # to the embedded-Workbench location under the shared-storage tree. The @@ -286,7 +297,11 @@ def from_dict(cls, raw: dict) -> WorkbenchConfig: idle_grace_seconds=raw.get("idle_grace_seconds", 60), extensions=WorkbenchExtensionsConfig.from_dict(raw.get("extensions", {})), kubernetes=WorkbenchKubernetesConfig.from_dict(raw.get("kubernetes", {})), - git_test=GitTestConfig.from_dict(git_test_raw) if git_test_raw is not None else None, + git_test=( + GitTestConfig.from_dict(git_test_raw) + if git_test_raw is not None + else GitTestConfig(clone_url=DEFAULT_PUBLIC_CLONE_URL, auth_method="none") + ), chronicle_data_path=raw.get( "chronicle_data_path", "/var/lib/rstudio-server/shared-storage/chronicle" ), diff --git a/src/vip_tests/workbench/exec.py b/src/vip_tests/workbench/exec.py index b3f44ee0..4a13facb 100644 --- a/src/vip_tests/workbench/exec.py +++ b/src/vip_tests/workbench/exec.py @@ -85,6 +85,18 @@ def _wrap_r_expr(expr: str, start: str, end: str) -> str: return f'cat("{s1}", "{s2}\\n", sep=""); {expr}; cat("\\n", "{e1}", "{e2}\\n", sep="")' +def _read_file_r_expr(path: str) -> str: + """Build the R expression that reads *path* and emits its raw contents. + + Wrapped in ``cat()`` so R prints the file bytes directly. A bare + ``paste(readLines(...))`` is auto-printed by the R REPL as a quoted, + backslash-escaped character vector -- which appends a stray ``"`` to the + done-marker line (``...:0"``) and makes ``_parse_done_marker`` reject the + exit code as non-numeric, hanging ``terminal_run`` until timeout. + """ + return f'cat(paste(readLines("{path}"), collapse="\\n"))' + + def _wrap_python_expr(expr: str, start: str, end: str) -> str: """Wrap *expr* with Python print() markers. @@ -773,16 +785,11 @@ def read_file(page: Page, path: str, timeout: int = 30_000, *, lang: str = "r") ide = _detect_ide(page) if ide == "positron": if lang.lower() == "r": - expr = f'paste(readLines("{path}"), collapse="\\n")' - return _strip_r_index(positron_eval_r(page, expr, timeout=timeout)) + return positron_eval_r(page, _read_file_r_expr(path), timeout=timeout) return positron_eval_python( page, f'with open("{path}") as _f: print(_f.read())', timeout=timeout ) if ide == "vscode": return read_file_via_vscode_editor(page, path, timeout=timeout) # RStudio (and unknown — fall back to RStudio R path) - if lang.lower() == "r": - expr = f'paste(readLines("{path}"), collapse="\\n")' - return _strip_r_index(rstudio_eval(page, expr, timeout=timeout)) - expr = f'paste(readLines("{path}"), collapse="\\n")' - return _strip_r_index(rstudio_eval(page, expr, timeout=timeout)) + return rstudio_eval(page, _read_file_r_expr(path), timeout=timeout) diff --git a/src/vip_tests/workbench/test_git_ops.feature b/src/vip_tests/workbench/test_git_ops.feature index a836f0ea..32f11639 100644 --- a/src/vip_tests/workbench/test_git_ops.feature +++ b/src/vip_tests/workbench/test_git_ops.feature @@ -2,21 +2,23 @@ Feature: Git operations from Workbench sessions Validate that users inside a Workbench session can clone, commit, and push to a Git repository through IDE terminals (RStudio, VS Code, Positron). - All scenarios require [workbench.git_test] in vip.toml. Clone scenarios run - with auth_method "https-token" (VIP_GIT_TOKEN set) or "none" (anonymous clone - of a public repo); push scenarios require "https-token". + The [workbench.git_test] block is optional: when absent, clone scenarios run + against a default public repo with auth_method "none" (anonymous, no token). + Add the block to override the clone_url, or to enable push scenarios with + auth_method "https-token" (VIP_GIT_TOKEN set); push auto-skips as read-only + otherwise. Scenario: Clone a Git repository in RStudio terminal - Given Workbench is accessible and I am logged in - And the Git test config is available + Given the Git test config is available + And Workbench is accessible and I am logged in When I launch an RStudio session And I clone the repository in the RStudio terminal Then the cloned repository directory exists Scenario: Create a branch, commit, and push from RStudio terminal - Given Workbench is accessible and I am logged in - And the Git test config is available + Given the Git test config is available And the Git test config supports pushing + And Workbench is accessible and I am logged in When I launch an RStudio session And I clone the repository in the RStudio terminal And I create a branch and commit a file in the RStudio terminal @@ -25,16 +27,16 @@ Feature: Git operations from Workbench sessions And I delete the pushed branch from the RStudio terminal Scenario: Clone a Git repository in VS Code terminal - Given Workbench is accessible and I am logged in - And the Git test config is available + Given the Git test config is available + And Workbench is accessible and I am logged in When I launch a VS Code session And I clone the repository in the VS Code terminal Then the cloned repository directory exists Scenario: Create a branch, commit, and push from VS Code terminal - Given Workbench is accessible and I am logged in - And the Git test config is available + Given the Git test config is available And the Git test config supports pushing + And Workbench is accessible and I am logged in When I launch a VS Code session And I clone the repository in the VS Code terminal And I create a branch and commit a file in the VS Code terminal @@ -43,20 +45,19 @@ Feature: Git operations from Workbench sessions And I delete the pushed branch from the VS Code terminal Scenario: Clone a Git repository in Positron terminal - Given Workbench is accessible and I am logged in - And the Git test config is available + Given the Git test config is available + And Workbench is accessible and I am logged in When I launch a Positron session And I clone the repository in the Positron terminal Then the cloned repository directory exists Scenario: Create a branch, commit, and push from Positron terminal - Given Workbench is accessible and I am logged in - And the Git test config is available + Given the Git test config is available And the Git test config supports pushing + And Workbench is accessible and I am logged in When I launch a Positron session And I clone the repository in the Positron terminal And I create a branch and commit a file in the Positron terminal And I push the branch from the Positron terminal Then the pushed branch exists on the remote And I delete the pushed branch from the Positron terminal - diff --git a/src/vip_tests/workbench/test_git_ops.py b/src/vip_tests/workbench/test_git_ops.py index e67cd075..a97230e8 100644 --- a/src/vip_tests/workbench/test_git_ops.py +++ b/src/vip_tests/workbench/test_git_ops.py @@ -3,10 +3,12 @@ Tests cover terminal-based Git operations (clone, branch, commit, push) in RStudio, VS Code, and Positron sessions. -Requires [workbench.git_test] in vip.toml. Clone scenarios run with -auth_method "https-token" (VIP_GIT_TOKEN set) or "none" (anonymous clone of a -public repo); push scenarios require "https-token". All scenarios auto-skip -when the config block is absent. +Clone scenarios run out-of-the-box against a default public repo (auth_method +"none", no token) when [workbench.git_test] is absent from vip.toml. Push +scenarios require auth_method "https-token" with VIP_GIT_TOKEN set and +auto-skip as read-only otherwise. The config-availability gate runs before +the login step so a read-only/config situation is reported deterministically, +without depending on a (possibly flaky) login attempt. """ from __future__ import annotations @@ -210,10 +212,16 @@ def git_config_available(vip_config): cfg = vip_config.workbench.git_test if cfg is None: + # Defensive fallback: WorkbenchConfig.from_dict always populates + # git_test with a default (anonymous clone of a public repo) when the + # [workbench.git_test] block is absent, so this should rarely fire — + # it only guards direct construction of WorkbenchConfig (e.g. tests). pytest.skip( "Git test config is not configured. " - "Add a [workbench.git_test] block to vip.toml with clone_url and auth_method, " - "and set VIP_GIT_TOKEN in the environment (for auth_method='https-token')." + "Cloning a public repo needs only clone_url and auth_method='none' " + "in a [workbench.git_test] block of vip.toml (no token required). " + "Push/private-repo scenarios additionally need auth_method='https-token' " + "with VIP_GIT_TOKEN set in the environment." ) if cfg.auth_method not in ("https-token", "none"): pytest.skip( @@ -223,7 +231,8 @@ def git_config_available(vip_config): if not cfg.clone_url: pytest.skip( "workbench.git_test.clone_url is empty. " - "Set clone_url in the [workbench.git_test] block of vip.toml." + "Set clone_url in the [workbench.git_test] block of vip.toml — " + "a public repo needs only clone_url with auth_method='none' (no token)." ) if _urlparse(cfg.clone_url).scheme != "https": pytest.skip( diff --git a/validation_docs/demo-issue-479-483-git-skip-gate.md b/validation_docs/demo-issue-479-483-git-skip-gate.md new file mode 100644 index 00000000..719c7756 --- /dev/null +++ b/validation_docs/demo-issue-479-483-git-skip-gate.md @@ -0,0 +1,133 @@ +# Fix: workbench git-ops skip reasons + default public clone repo + +*2026-07-16T19:23:40Z by Showboat 0.6.1* + + +This branch fixes two related Workbench git-ops bugs. + +**#479 — misleading auth skip when git config is absent.** Every scenario in test_git_ops.feature checked the auth-session login gate BEFORE the git-config gate. Because Workbench OIDC auth is flaky under the shared-account serial run, whether a git test skipped with the config reason or the auth reason was non-deterministic — push tests could report a misleading 'Workbench session not established' error when the real reason was 'no git config / read-only'. Fixed by reordering the Gherkin steps so the config gate (and, for push scenarios, the pushing gate) resolves before the login attempt. + +**#483 — public-repo cloning wrongly appeared to require VIP_GIT_TOKEN.** There was no default [workbench.git_test] config, so clone scenarios always skipped without one, and the skip message read as if a token was required even for anonymous public clones. Fixed by shipping a default public clone target (`https://github.com/posit-dev/posit-cli.git`, auth_method="none") that WorkbenchConfig.from_dict synthesizes when the block is absent, and by rewording the skip messages to make clear a token is only needed for push/private-repo scenarios. + +### Fix #479: config gate now precedes login in every scenario + +Before this fix, step 1 was the login attempt and step 2 was the config check. Now the config check runs first: + +```bash +grep -n -A4 "Scenario: Clone a Git repository in RStudio" src/vip_tests/workbench/test_git_ops.feature +``` + +```output +11: Scenario: Clone a Git repository in RStudio terminal +12- Given the Git test config is available +13- And Workbench is accessible and I am logged in +14- When I launch an RStudio session +15- And I clone the repository in the RStudio terminal +``` + +### Fix #483: reworded skip message makes clear public clones need no token + +The config-missing skip message now explicitly says an anonymous public-repo clone needs only `clone_url` + `auth_method='none'` (no token), and a token is only required for push/private-repo scenarios: + +```bash +sed -n "219,225p" src/vip_tests/workbench/test_git_ops.py +``` + +```output + pytest.skip( + "Git test config is not configured. " + "Cloning a public repo needs only clone_url and auth_method='none' " + "in a [workbench.git_test] block of vip.toml (no token required). " + "Push/private-repo scenarios additionally need auth_method='https-token' " + "with VIP_GIT_TOKEN set in the environment." + ) +``` + +### New selftests pass: default-config synthesis, step-order guard, skip-message wording + +```bash +env -u UV_PROJECT uv run --frozen --no-sync --project . pytest selftests/test_config.py selftests/test_workbench_git_ops.py -q 2>&1 | grep -E "passed|failed|error" | sed "s/ in [0-9.]*s//" +``` + +```output +111 passed, 1 warning +``` + +### Full selftest suite is green (excluding `selftests/test_load_engine.py`, which has pre-existing timing-flaky tests unrelated to this change; run with `-n0` since the repo's default `-n auto` xdist parallelism is itself intermittently flaky, unrelated to this change) + +```bash +UV_FROZEN=1 env -u UV_PROJECT uv run --frozen --no-sync --project . pytest selftests/ -q --ignore=selftests/test_load_engine.py -n0 2>&1 | grep -E "passed|failed|error" | sed "s/ in [0-9.]*s//" +``` + +```output +950 passed, 11 warnings +``` + +### Lint and format check (`just check` equivalent — run directly with the project-scoped uv wrapper since `just`'s recipes invoke bare `uv`, which reroots to the parent ptd-workspace in this environment) + +```bash +env -u UV_PROJECT uv run --frozen --no-sync --project . ruff check src/ src/vip_tests/ selftests/ examples/ +``` + +```output +All checks passed! +``` + +```bash +env -u UV_PROJECT uv run --frozen --no-sync --project . ruff format --check src/ src/vip_tests/ selftests/ examples/ +``` + +```output +159 files already formatted +``` + +### The 6 git-ops scenarios still collect after the feature-file reorder + +Workbench product tests deselect entirely without a configured URL, so this confirms collection with a minimal `[workbench]` config: + +```bash +printf "[workbench]\nurl = \"https://workbench.example.com\"\n" > "$TMPDIR/vip_demo.toml" && env -u UV_PROJECT uv run --frozen --no-sync --project . pytest --collect-only -q --vip-config "$TMPDIR/vip_demo.toml" src/vip_tests/workbench/test_git_ops.py 2>/dev/null | sed "s/ in [0-9.]*s//"; rm -f "$TMPDIR/vip_demo.toml" +``` + +```output +src/vip_tests/workbench/test_git_ops.py::test_clone_rstudio[chromium] +src/vip_tests/workbench/test_git_ops.py::test_push_rstudio[chromium] +src/vip_tests/workbench/test_git_ops.py::test_clone_vscode[chromium] +src/vip_tests/workbench/test_git_ops.py::test_push_vscode[chromium] +src/vip_tests/workbench/test_git_ops.py::test_clone_positron[chromium] +src/vip_tests/workbench/test_git_ops.py::test_push_positron[chromium] + +6 tests collected +``` + +### Follow-up fix: R file-readback quoting bug (surfaced once #483 made clone run by default) + +Live validation of #483's default clone against dev.demo.posit.team exposed a third bug: `read_file()`'s R branches evaluated a bare `paste(readLines(...), collapse="\\n")` expression. The R REPL auto-prints that as a quoted, backslash-escaped character vector, which appended a stray `"` to the `terminal_run` done-marker line (`...:0\"`). `_parse_done_marker` then rejected the exit code as non-numeric, so `terminal_run` spun until its 120s timeout even though the underlying command had finished in ~1.5s — this is what made `test_clone_rstudio` time out. + +The fix wraps the read in `cat()` via a new `_read_file_r_expr` helper, so R emits raw bytes instead of an auto-printed quoted vector, and drops the now-unnecessary `_strip_r_index` call on those reads (both the RStudio and Positron R-console readback paths used the same buggy expression). + +```bash +env -u UV_PROJECT uv run --frozen --no-sync --project . pytest "selftests/test_workbench_exec.py::TestReadFileRExpr" "selftests/test_workbench_exec.py::TestParseDoneMarker::test_raw_exit_code_parses_but_quoted_does_not" -q 2>&1 | grep -E "passed|failed|error" | sed "s/ in [0-9.]*s//" +``` + +```output +3 passed +``` + +```bash +grep -n "cat(paste(readLines" src/vip_tests/workbench/exec.py +``` + +```output +97: return f'cat(paste(readLines("{path}"), collapse="\\n"))' +``` + +### Live validation on dev.demo.posit.team + +Not re-runnable in this document (requires a live deployment + interactive browser auth), so recorded here as prose instead of an exec block: + +Running a config-less `--interactive-auth` pass against dev.demo.posit.team (a real 2026.06-class deployment): +- `test_clone_vscode` **PASSED** — proves the #483 default clone (posit-dev/posit-cli, no config, no token) works end-to-end against a real Workbench instance. +- `test_clone_rstudio` **PASSED in ~23s** — before this readback fix it was hitting the full 120s `terminal_run` timeout; the `cat()` wrap resolved the done-marker parsing failure described above. +- Push scenarios **skipped** with the accurate read-only reason (`auth_method='none' is anonymous (read-only)...`), confirming the #479 reorder surfaces the real skip reason instead of a login-flake error. +- `test_clone_positron` / `test_push_positron` were not exercised — dev.demo has no Positron IDE installed, so the Positron launch gate (#477) remains deferred pending a Positron-enabled deployment. diff --git a/vip.toml.example b/vip.toml.example index bcd3c040..50b6874e 100644 --- a/vip.toml.example +++ b/vip.toml.example @@ -100,8 +100,12 @@ url = "https://workbench.example.com" # Git operations testing. # Validates that users can clone, commit, and push from Workbench sessions -# (RStudio terminal, VS Code terminal, Positron terminal, and RStudio Git pane). -# All Git scenarios auto-skip when this block is absent. +# (RStudio terminal, VS Code terminal, and Positron terminal). +# +# This block is optional. When absent, clone/connectivity scenarios run +# out-of-the-box against a default public repo (auth_method "none", no token +# required); push/commit scenarios auto-skip as read-only. Add this block +# only to (a) override the default clone repo, or (b) enable push scenarios. # # auth_method = "https-token": set VIP_GIT_TOKEN in the environment (never in # this file); clone and push scenarios run.