diff --git a/README.md b/README.md index a789c6c..9373a6b 100644 --- a/README.md +++ b/README.md @@ -11,7 +11,8 @@ can link workspaces to Jira issues to log time against them. - `create`: wrapper to create Jira issue from the command line (can take GitHub PR as input) - `start`: let you select and assigned task, create a workspace associated to it, and track the time you spend on it. - `log-time`: log your current progress (typically to do it on a daily-basis) - - `complete`: log the time spent, and move the issue to Done. + - `complete`: log the time spent, and move the issue to Done. If less than 60 seconds + have elapsed, skip logging and continue with the completion prompts. - `comment`: add a comment to the ticket. - `status`: show and let you update the status. - **`et`** (no subcommand) / **`et info`**: show the Jira issue linked to the workspace (if any). diff --git a/scripts/lib/remove-isolated-gnome-shell-env.sh b/scripts/lib/remove-isolated-gnome-shell-env.sh index 57af47f..6fa07a3 100755 --- a/scripts/lib/remove-isolated-gnome-shell-env.sh +++ b/scripts/lib/remove-isolated-gnome-shell-env.sh @@ -11,18 +11,30 @@ if [ "$(dirname "${test_root}")" != "${temporary_parent}" ] \ exit 2 fi -for mount_path in "${test_root}/runtime/doc" "${test_root}/runtime/gvfs"; do - if mountpoint -q "${mount_path}" 2>/dev/null; then - if command -v fusermount3 >/dev/null 2>&1; then - fusermount3 -uz "${mount_path}" 2>/dev/null || true - elif command -v fusermount >/dev/null 2>&1; then - fusermount -uz "${mount_path}" 2>/dev/null || true - fi +# Portal FUSE mounts can disappear while rm is traversing runtime/doc after +# the private session exits. Retry cleanup briefly, checking mounts each time. +for attempt in 1 2 3; do + for mount_path in "${test_root}/runtime/doc" "${test_root}/runtime/gvfs"; do if mountpoint -q "${mount_path}" 2>/dev/null; then - echo "error: refusing to remove test root while ${mount_path} is mounted" >&2 - exit 1 + if command -v fusermount3 >/dev/null 2>&1; then + fusermount3 -uz "${mount_path}" 2>/dev/null || true + elif command -v fusermount >/dev/null 2>&1; then + fusermount -uz "${mount_path}" 2>/dev/null || true + fi + if mountpoint -q "${mount_path}" 2>/dev/null; then + echo "error: refusing to remove test root while ${mount_path} is mounted" >&2 + exit 1 + fi fi + done + + if rm -rf --one-file-system "${test_root}"; then + exit 0 + fi + if [ "${attempt}" -lt 3 ]; then + sleep 1 fi done -rm -rf --one-file-system "${test_root}" +echo "error: could not remove disposable test root: ${test_root}" >&2 +exit 1 diff --git a/src/et/cli.py b/src/et/cli.py index a35d13c..92aa0cd 100644 --- a/src/et/cli.py +++ b/src/et/cli.py @@ -1032,10 +1032,13 @@ def jira_complete( jira log-time`), tells you how much was logged, then asks whether to delete the workspace (reclaiming its GNOME workspace slot) and whether to move the linked Jira issue to "Done". Both actions are skipped unless - confirmed. + confirmed. Time below 60 seconds is skipped without creating a worklog. """ def on_logged(result: LogTimeResult) -> None: + if result.seconds_logged == 0: + typer.echo("Skipped time logging: less than 60 seconds elapsed.") + return duration = format_duration(result.seconds_logged) typer.echo( f"Logged {duration} to {_jira_ref_link(result.issue_key)} " diff --git a/src/et/jira_time.py b/src/et/jira_time.py index e57dabe..0afab98 100644 --- a/src/et/jira_time.py +++ b/src/et/jira_time.py @@ -23,7 +23,7 @@ from et import et_extension, workspaces from et.config import EtConfig, JiraConfig, load_config -from et.et_extension import EtExtensionError +from et.et_extension import EtExtensionError, WorkspaceCounterNotFoundError from et.jira import JiraError, create_worklog, fetch_issue from et.jira_ref import jira_key_from_ref @@ -134,7 +134,11 @@ def resolve_active_issue( def log_time_for_current_workspace( - *, description: str | None = None, reset: bool = True, issue_key: str | None = None + *, + description: str | None = None, + reset: bool = True, + issue_key: str | None = None, + skip_short: bool = False, ) -> LogTimeResult: """Log the active workspace's extension counter elapsed time to its Jira issue. @@ -150,6 +154,11 @@ def log_time_for_current_workspace( installed/enabled), its elapsed time is under `MIN_LOGGABLE_SECONDS`, or the Jira API call fails. + With `skip_short=True`, time below the minimum returns zero seconds + logged without creating a worklog or resetting the counter. A missing + workspace counter is treated as zero elapsed time in this mode, allowing + task completion even when there is no time to log. + On success, resets the counter to 0 (unless `reset=False`) so the same elapsed time isn't accidentally logged again later. The reset only happens after the Jira worklog call has actually succeeded. If Jira @@ -162,11 +171,22 @@ def log_time_for_current_workspace( try: counter = et_extension.get_workspace_counter(index) + except WorkspaceCounterNotFoundError as exc: + if not skip_short: + raise JiraLogTimeError(str(exc)) from exc + counter = None except EtExtensionError as exc: raise JiraLogTimeError(str(exc)) from exc - seconds = counter.elapsed_seconds + seconds = counter.elapsed_seconds if counter is not None else 0 if seconds < MIN_LOGGABLE_SECONDS: + if skip_short: + return LogTimeResult( + workspace_index=index, + issue_key=resolved_key, + seconds_logged=0, + counter_reset=False, + ) raise JiraLogTimeError( f"workspace {index + 1}'s counter has only {seconds}s elapsed " f"(minimum {MIN_LOGGABLE_SECONDS}s to log)" diff --git a/src/et/task.py b/src/et/task.py index 24d9877..f3630cd 100644 --- a/src/et/task.py +++ b/src/et/task.py @@ -738,7 +738,9 @@ def complete_task_for_current_workspace( Delegates the logging step to `et.jira_time.log_time_for_current_workspace` (which already resets the counter on success) and, once it succeeds, calls `on_logged(log_result)` - so the caller can report the logged time before any prompts. Logs + so the caller can report the logged time before any prompts. Time below + the logging minimum is skipped, allowing completion with no elapsed time. + The counter is left untouched unless workspace deletion is confirmed. Logs against `issue_key` if given (e.g. via `-j/--jira`), instead of the issue linked to the active workspace — workspace deletion still applies to the active workspace regardless, since it's not tied to which issue @@ -766,7 +768,7 @@ def complete_task_for_current_workspace( `TaskError` if freeing the workspace or transitioning the issue fails. """ log_result = log_time_for_current_workspace( - description=comment, reset=True, issue_key=issue_key + description=comment, reset=True, issue_key=issue_key, skip_short=True ) on_logged(log_result) diff --git a/tests/test_cli.py b/tests/test_cli.py index a870c47..030ed8a 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -1648,3 +1648,44 @@ def test_edit_branch_description_returns_stripped_input(): with patch("et.cli.typer.prompt", return_value=" custom slug "): assert _edit_branch_description("default-slug") == "custom slug" + + +@pytest.mark.parametrize("seconds", [None, 0, 25, 59, 60]) +@pytest.mark.parametrize("confirm", [False, True]) +def test_jira_complete_with_short_counter(seconds, confirm): + config = EtConfig( + jira=JiraConfig( + base_url="https://example.atlassian.net", email="me@example.com", pat="token", + jql="assignee = currentUser()", + ), + workspaces=[WorkspaceConfigEntry(name="Task", ref="jira:ISD-321")], + ) + with ( + patch("et.jira_time.load_config", return_value=config), + patch("et.task.load_config", return_value=config), + patch("et.jira_time.workspaces.get_active_workspace_index", return_value=0), + patch("et.jira_time.et_extension.get_workspace_counter", + return_value=WorkspaceCounter(elapsed_seconds=seconds or 0, running=True), + side_effect=WorkspaceCounterNotFoundError("not prepared") + if seconds is None else None), + patch("et.jira_time.create_worklog") as worklog, + patch("et.jira_time.et_extension.reset_workspace_counter") as reset, + patch("et.task._free_workspace_slot") as delete, + patch("et.task._transition_to_status") as transition, + ): + answer = "y\n" if confirm else "n\n" + result = runner.invoke(app, ["jira", "complete"], input=answer * 2) + + assert result.exit_code == 0, result.output + assert "Delete workspace 1?" in result.output + assert "to 'Done'?" in result.output + assert delete.called is confirm + assert transition.called is confirm + if seconds is None or seconds < 60: + assert "Skipped time logging: less than 60 seconds elapsed." in result.output + assert "Logged " not in result.output + worklog.assert_not_called() + reset.assert_not_called() + else: + worklog.assert_called_once_with(config.jira, "ISD-321", 60, comment=None) + reset.assert_called_once_with(0) diff --git a/tests/test_gnome_extension_tooling.py b/tests/test_gnome_extension_tooling.py index 6b67f8d..abd996b 100644 --- a/tests/test_gnome_extension_tooling.py +++ b/tests/test_gnome_extension_tooling.py @@ -9,6 +9,8 @@ import zipfile from pathlib import Path +import pytest + REPO_ROOT = Path(__file__).parents[1] EXTENSION_DIR = REPO_ROOT / "gnome-extension" / "et@seb4stien.github.com" VALIDATOR = REPO_ROOT / "scripts" / "check-gnome-shell-extension.py" @@ -121,3 +123,42 @@ def test_enable_extension_setting_handles_typed_empty_array(tmp_path: Path) -> N assert calls.read_text().strip() == ( "set org.gnome.shell enabled-extensions ['et@example.com']" ) + + +@pytest.mark.parametrize("persistent", [False, True]) +def test_cleanup_retries_removal_failure(tmp_path: Path, persistent: bool) -> None: + test_root = tmp_path / "et-gnome-shell-test-cleanup" + (test_root / "runtime" / "doc").mkdir(parents=True) + fake_bin = tmp_path / "bin" + fake_bin.mkdir() + calls = tmp_path / "calls" + real_rm = shutil.which("rm") + assert real_rm is not None + fake_rm = fake_bin / "rm" + fake_rm.write_text( + "#!/usr/bin/env bash\n" + 'echo called >> "$CALLS_FILE"\n' + 'if [ "$PERSISTENT" = true ] || [ "$(wc -l < "$CALLS_FILE")" -eq 1 ]; then\n' + ' echo "simulated portal teardown race" >&2\n' + " exit 1\n" + "fi\n" + 'exec "$REAL_RM" "$@"\n' + ) + fake_rm.chmod(0o755) + result = subprocess.run( + [str(REPO_ROOT / "scripts/lib/remove-isolated-gnome-shell-env.sh"), str(test_root)], + env={ + **os.environ, + "TMPDIR": str(tmp_path), + "PATH": f"{fake_bin}:{os.environ['PATH']}", + "CALLS_FILE": str(calls), + "REAL_RM": real_rm, + "PERSISTENT": str(persistent).lower(), + }, + capture_output=True, + text=True, + check=False, + ) + assert result.returncode == (1 if persistent else 0), result.stderr + assert test_root.exists() is persistent + assert len(calls.read_text().splitlines()) == (3 if persistent else 2) diff --git a/tests/test_task.py b/tests/test_task.py index fc1e96b..161aa69 100644 --- a/tests/test_task.py +++ b/tests/test_task.py @@ -847,7 +847,9 @@ def test_complete_task_deletes_workspace_after_logging( comment="done", confirm_delete=lambda _result: True ) - mock_log_time.assert_called_once_with(description="done", reset=True, issue_key=None) + mock_log_time.assert_called_once_with( + description="done", reset=True, issue_key=None, skip_short=True + ) assert result.log_result.issue_key == "ISD-2" assert result.workspace_freed is True # The completed workspace is reclaimed exactly like `et ws delete` — force @@ -867,7 +869,9 @@ def test_complete_task_forwards_issue_key_override(mock_log_time, mock_load_conf result = complete_task_for_current_workspace(comment="done", issue_key="ISD-999") - mock_log_time.assert_called_once_with(description="done", reset=True, issue_key="ISD-999") + mock_log_time.assert_called_once_with( + description="done", reset=True, issue_key="ISD-999", skip_short=True + ) assert result.log_result.issue_key == "ISD-999"