diff --git a/cli/common/clicore/focus.go b/cli/common/clicore/focus.go index 4285ed4c30..10fbb9c58e 100644 --- a/cli/common/clicore/focus.go +++ b/cli/common/clicore/focus.go @@ -28,10 +28,6 @@ func RunFocusWindow(ctx context.Context, projectRoot string, stdout io.Writer, s return runFocusWindow(ctx, projectRoot, stdout, stderr, defaultFocusWindowDeps()) } -func FindRunningUnityProcess(ctx context.Context, projectRoot string) (*UnityProcess, error) { - return unityprocess.FindRunningUnityProcess(ctx, projectRoot) -} - func FocusUnityProcess(ctx context.Context, pid int) error { return unityprocess.FocusUnityProcess(ctx, pid) } diff --git a/cli/common/clicore/tool_readiness.go b/cli/common/clicore/tool_readiness.go index f31a5dcc6b..a35a0926a6 100644 --- a/cli/common/clicore/tool_readiness.go +++ b/cli/common/clicore/tool_readiness.go @@ -28,6 +28,8 @@ const executeDynamicCodeReadinessProbe = `return "Unity CLI Loop dynamic code pr type toolReadinessDeps struct { findRunningUnityProcess func(context.Context, string) (*unityprocess.UnityProcess, error) probeToolReadinessSequence func(context.Context, string) error + // now is the clock the Unity exit watch spaces its lookups with; nil uses time.Now. + now func() time.Time } func WaitForToolReadiness(ctx context.Context, projectRoot string) error { @@ -40,7 +42,7 @@ func WaitForToolReadinessWithTimeout(ctx context.Context, projectRoot string, ti func defaultToolReadinessDeps() toolReadinessDeps { return toolReadinessDeps{ - findRunningUnityProcess: unityprocess.FindRunningUnityProcess, + findRunningUnityProcess: unityprocess.FindRunningUnityProcessDefinitively, probeToolReadinessSequence: ProbeToolReadinessSequence, } } @@ -52,6 +54,7 @@ func waitForToolReadinessWithDeps(ctx context.Context, projectRoot string, timeo defer cancel() var lastErr error + exitWatch := NewUnityExitWatch(projectRoot, resolveProjectEndpointAddress(projectRoot), deps.findRunningUnityProcess, deps.now) ticker := time.NewTicker(ToolReadinessPoll) defer ticker.Stop() for { @@ -67,6 +70,11 @@ func waitForToolReadinessWithDeps(ctx context.Context, projectRoot string, timeo if clierrors.IsPermanentConnectError(err) { return err } + // Why abort: an Editor that exited cannot become ready, and launch would otherwise wait + // its whole ten minutes for it. + if exitErr := exitWatch.Observe(timeoutContext, err); exitErr != nil { + return exitErr + } lastErr = err } diff --git a/cli/common/clicore/tool_readiness_test.go b/cli/common/clicore/tool_readiness_test.go index ee4f1a10fe..9724034bf7 100644 --- a/cli/common/clicore/tool_readiness_test.go +++ b/cli/common/clicore/tool_readiness_test.go @@ -7,6 +7,7 @@ import ( "net" "os" "path/filepath" + "reflect" "strings" "syscall" "testing" @@ -264,3 +265,39 @@ func assertConnectionAttemptToProject(t *testing.T, err error, projectRoot strin t.Fatalf("connection endpoint = %q, want %q", attemptErr.Endpoint, expected) } } + +// Verifies a readiness wait whose Editor exited ends as soon as the exit is confirmed, instead of +// polling a server that cannot come back until the wait times out. +func TestWaitForToolReadinessStopsOnceUnityHasExited(t *testing.T) { + clock := time.Unix(1_700_000_000, 0) + deps := toolReadinessDeps{ + probeToolReadinessSequence: func(context.Context, string) error { + return &unityipc.ConnectionAttemptError{Cause: syscall.ECONNREFUSED} + }, + findRunningUnityProcess: func(context.Context, string) (*UnityProcess, error) { + return nil, nil + }, + // Each reading moves a full confirm interval, so the second poll can already confirm. + now: func() time.Time { + clock = clock.Add(unityExitConfirmInterval) + return clock + }, + } + + err := waitForToolReadinessWithDeps(context.Background(), t.TempDir(), time.Hour, deps) + + var notRunning clierrors.UnityNotRunningError + if !errors.As(err, ¬Running) { + t.Fatalf("expected UnityNotRunningError, got %v", err) + } +} + +// Verifies the readiness wait's default lookup is the one that reports an Editor of unknown project +// instead of "none", since its exit watch concludes from "none" that Unity is not running. +func TestDefaultToolReadinessLookupDoesNotTakeAnUnidentifiedEditorAsNone(t *testing.T) { + lookup := defaultToolReadinessDeps().findRunningUnityProcess + + if reflect.ValueOf(lookup).Pointer() != reflect.ValueOf(unityprocess.FindRunningUnityProcessDefinitively).Pointer() { + t.Fatal("the default lookup must be unityprocess.FindRunningUnityProcessDefinitively") + } +} diff --git a/cli/common/clicore/unity_exit.go b/cli/common/clicore/unity_exit.go new file mode 100644 index 0000000000..6813b35c6c --- /dev/null +++ b/cli/common/clicore/unity_exit.go @@ -0,0 +1,109 @@ +package clicore + +import ( + "context" + "os" + "time" + + clierrors "github.com/hatayama/unity-cli-loop/common/errors" + "github.com/hatayama/unity-cli-loop/common/unityprocess" +) + +const ( + // Why spaced lookups: an Editor that is tearing down can still be listed for a moment after its + // server closed, and one empty listing is thin evidence for ending a ten-minute wait. Two lookups this + // far apart that both find nothing are not a transient. + unityExitConfirmInterval = 2 * time.Second + unityExitConfirmations = 2 +) + +// NewUnityNotRunningError describes a project whose Unity Editor a successful process lookup did not +// find, noting whether the Editor left its lockfile behind and, if so, where its log is. exitedDuringCommand +// says the command had reached Unity before it went away. +func NewUnityNotRunningError( + projectRoot string, + endpoint string, + cause error, + exitedDuringCommand bool, +) clierrors.UnityNotRunningError { + leftBehind := lockfileLeftBehind(projectRoot) + editorLogPath := "" + // Why only after an abnormal exit: the log is pointed at only then, and finding it reads two logs. + if leftBehind { + editorLogPath = unityprocess.EditorLogPathFor(projectRoot) + } + return clierrors.UnityNotRunningError{ + ProjectRoot: projectRoot, + Endpoint: endpoint, + Cause: cause, + LockfileLeftBehind: leftBehind, + EditorLogPath: editorLogPath, + ExitedDuringCommand: exitedDuringCommand, + } +} + +// Why only a lockfile that is certainly there counts: a stat that fails for another reason does not show +// how Unity shut down, and the abnormal-exit guidance must not rest on it. +func lockfileLeftBehind(projectRoot string) bool { + _, err := os.Stat(unityprocess.LockfilePath(projectRoot)) + return err == nil +} + +// UnityExitWatch tells a wait loop that keeps losing its connection to Unity when the Editor process +// for the project has exited, so the loop can stop instead of polling until its own timeout. +type UnityExitWatch struct { + projectRoot string + endpoint string + findProcess func(context.Context, string) (*unityprocess.UnityProcess, error) + now func() time.Time + lastLookupAt time.Time + goneLookups int +} + +// NewUnityExitWatch creates a watch for the project at projectRoot. findProcess is the process lookup and +// now the clock; a nil now uses time.Now. +func NewUnityExitWatch( + projectRoot string, + endpoint string, + findProcess func(context.Context, string) (*unityprocess.UnityProcess, error), + now func() time.Time, +) *UnityExitWatch { + if now == nil { + now = time.Now + } + return &UnityExitWatch{ + projectRoot: projectRoot, + endpoint: endpoint, + findProcess: findProcess, + now: now, + } +} + +// Observe takes the error of one poll, nil for a poll that succeeded. It returns a +// clierrors.UnityNotRunningError once polls that found the server gone were followed by two lookups, at +// least unityExitConfirmInterval apart, that both found no Editor process; otherwise it returns nil. +func (watch *UnityExitWatch) Observe(ctx context.Context, pollErr error) error { + // Any other outcome, a success included, shows a server that may be alive, so the lookups that + // found nothing so far no longer belong to one exit. + if !clierrors.IsUnityServerGoneError(pollErr) { + watch.goneLookups = 0 + return nil + } + now := watch.now() + if !watch.lastLookupAt.IsZero() && now.Sub(watch.lastLookupAt) < unityExitConfirmInterval { + return nil + } + watch.lastLookupAt = now + + process, err := watch.findProcess(ctx, watch.projectRoot) + // A lookup that failed observed nothing, so it cannot count toward an exit. + if err != nil || process != nil { + watch.goneLookups = 0 + return nil + } + watch.goneLookups++ + if watch.goneLookups < unityExitConfirmations { + return nil + } + return NewUnityNotRunningError(watch.projectRoot, watch.endpoint, pollErr, true) +} diff --git a/cli/common/clicore/unity_exit_test.go b/cli/common/clicore/unity_exit_test.go new file mode 100644 index 0000000000..296aa9a60c --- /dev/null +++ b/cli/common/clicore/unity_exit_test.go @@ -0,0 +1,232 @@ +package clicore + +import ( + "context" + "errors" + "io" + "os" + "path/filepath" + "syscall" + "testing" + "time" + + clierrors "github.com/hatayama/unity-cli-loop/common/errors" + "github.com/hatayama/unity-cli-loop/common/unityipc" + "github.com/hatayama/unity-cli-loop/common/unityprocess" +) + +// unityExitWatchFixture drives a UnityExitWatch with a hand-moved clock and a scripted process lookup. +type unityExitWatchFixture struct { + now time.Time + running bool + lookErr error + lookups int + watch *UnityExitWatch +} + +func newUnityExitWatchFixture(t *testing.T) *unityExitWatchFixture { + t.Helper() + fixture := &unityExitWatchFixture{now: time.Unix(1_700_000_000, 0), running: true} + fixture.watch = NewUnityExitWatch( + t.TempDir(), + "/tmp/uloop/sample.sock", + func(context.Context, string) (*unityprocess.UnityProcess, error) { + fixture.lookups++ + if fixture.lookErr != nil { + return nil, fixture.lookErr + } + if fixture.running { + return &unityprocess.UnityProcess{Pid: 4321}, nil + } + return nil, nil + }, + func() time.Time { return fixture.now }, + ) + return fixture +} + +func (fixture *unityExitWatchFixture) observe(pollErr error) error { + return fixture.watch.Observe(context.Background(), pollErr) +} + +func (fixture *unityExitWatchFixture) advance(duration time.Duration) { + fixture.now = fixture.now.Add(duration) +} + +func serverGonePollError() error { + return &unityipc.ConnectionAttemptError{Cause: syscall.ECONNREFUSED} +} + +// Verifies the watch reports Unity as gone only after two lookups, spaced at least the confirm +// interval apart, both find no process, and that the report says Unity went away during the command. +func TestUnityExitWatchReportsAfterTwoSpacedLookupsFindNoProcess(t *testing.T) { + fixture := newUnityExitWatchFixture(t) + fixture.running = false + + if err := fixture.observe(serverGonePollError()); err != nil { + t.Fatalf("one lookup must not be enough, got %v", err) + } + fixture.advance(unityExitConfirmInterval - time.Millisecond) + if err := fixture.observe(serverGonePollError()); err != nil { + t.Fatalf("a poll inside the confirm interval must not confirm, got %v", err) + } + fixture.advance(time.Millisecond) + err := fixture.observe(serverGonePollError()) + + var notRunning clierrors.UnityNotRunningError + if !errors.As(err, ¬Running) { + t.Fatalf("expected UnityNotRunningError after the confirming lookup, got %v", err) + } + if !notRunning.ExitedDuringCommand { + t.Fatal("a wait loop's report must say Unity went away during the command") + } + if fixture.lookups != 2 { + t.Fatalf("lookups = %d, want 2 (the poll inside the interval must not look up)", fixture.lookups) + } +} + +// Verifies a running Editor is looked up at most once per confirm interval, so a wait that polls every +// few milliseconds through a long domain reload does not list processes on every poll. +func TestUnityExitWatchThrottlesLookupsWhileUnityRuns(t *testing.T) { + fixture := newUnityExitWatchFixture(t) + + for poll := 0; poll < 10; poll++ { + if err := fixture.observe(serverGonePollError()); err != nil { + t.Fatalf("a running Editor must never be reported as gone, got %v", err) + } + fixture.advance(unityExitConfirmInterval / 4) + } + + if fixture.lookups != 3 { + t.Fatalf("lookups = %d, want 3 over 2.5 confirm intervals", fixture.lookups) + } +} + +// Verifies each kind of evidence that the Editor may still be there restarts the confirmation: a process +// found, a lookup that failed, a poll that succeeded, and a poll error that a live server also produces. +func TestUnityExitWatchRestartsTheConfirmationOnEvidenceOfALiveEditor(t *testing.T) { + cases := []struct { + name string + interrupt func(fixture *unityExitWatchFixture) error + wantLookup bool + }{ + { + name: "process found", + interrupt: func(fixture *unityExitWatchFixture) error { + fixture.running = true + defer func() { fixture.running = false }() + return fixture.observe(serverGonePollError()) + }, + }, + { + name: "lookup failed", + interrupt: func(fixture *unityExitWatchFixture) error { + fixture.lookErr = errors.New("sysctl kern.proc.all: operation not permitted") + defer func() { fixture.lookErr = nil }() + return fixture.observe(serverGonePollError()) + }, + }, + { + name: "poll succeeded", + interrupt: func(fixture *unityExitWatchFixture) error { + return fixture.observe(nil) + }, + }, + { + name: "dial timed out", + interrupt: func(fixture *unityExitWatchFixture) error { + return fixture.observe(&unityipc.ConnectionAttemptError{Cause: os.ErrDeadlineExceeded}) + }, + }, + } + + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + fixture := newUnityExitWatchFixture(t) + fixture.running = false + if err := fixture.observe(serverGonePollError()); err != nil { + t.Fatalf("one lookup must not be enough, got %v", err) + } + fixture.advance(unityExitConfirmInterval) + if err := testCase.interrupt(fixture); err != nil { + t.Fatalf("the interruption must not report Unity as gone, got %v", err) + } + fixture.advance(unityExitConfirmInterval) + if err := fixture.observe(serverGonePollError()); err != nil { + t.Fatalf("the lookup after the interruption must start a new confirmation, got %v", err) + } + fixture.advance(unityExitConfirmInterval) + if err := fixture.observe(serverGonePollError()); err == nil { + t.Fatal("two spaced lookups after the interruption must report Unity as gone") + } + }) + } +} + +// Verifies a dropped connection counts as the server going away, the same as a dial nobody answered. +func TestUnityExitWatchTreatsADroppedConnectionAsTheServerGoingAway(t *testing.T) { + fixture := newUnityExitWatchFixture(t) + fixture.running = false + + _ = fixture.observe(io.EOF) + fixture.advance(unityExitConfirmInterval) + if err := fixture.observe(io.EOF); err == nil { + t.Fatal("two spaced lookups after dropped connections must report Unity as gone") + } +} + +// writeProjectEditorLogForTest points Unity's default log location at a temporary home and writes a log +// there that records projectRoot as its project, returning the log's path. +func writeProjectEditorLogForTest(t *testing.T, projectRoot string) string { + t.Helper() + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("LOCALAPPDATA", home) + editorLogPath := unityprocess.DefaultEditorLogPath() + if err := os.MkdirAll(filepath.Dir(editorLogPath), 0o755); err != nil { + t.Fatalf("creating the log directory failed: %v", err) + } + content := "COMMAND LINE ARGUMENTS:\nSuccessfully changed project path to: " + projectRoot + "\n" + if err := os.WriteFile(editorLogPath, []byte(content), 0o644); err != nil { + t.Fatalf("writing the Editor log failed: %v", err) + } + return editorLogPath +} + +// Verifies the not-running error records a lockfile left behind, which only a shutdown that did not +// finish leaves, along with the log that Editor wrote. +func TestNewUnityNotRunningErrorRecordsALockfileLeftBehind(t *testing.T) { + projectRoot := t.TempDir() + lockfilePath := unityprocess.LockfilePath(projectRoot) + if err := os.MkdirAll(filepath.Dir(lockfilePath), 0o755); err != nil { + t.Fatalf("creating Temp failed: %v", err) + } + if err := os.WriteFile(lockfilePath, []byte{}, 0o644); err != nil { + t.Fatalf("writing the lockfile failed: %v", err) + } + + editorLogPath := writeProjectEditorLogForTest(t, projectRoot) + + err := NewUnityNotRunningError(projectRoot, "/tmp/uloop/sample.sock", io.EOF, true) + + if !err.LockfileLeftBehind { + t.Fatal("expected the lockfile left behind to be recorded") + } + if err.EditorLogPath != editorLogPath { + t.Fatalf("EditorLogPath = %q, want the project's Editor log %q", err.EditorLogPath, editorLogPath) + } + if err.ProjectRoot != projectRoot || err.Endpoint != "/tmp/uloop/sample.sock" || err.Cause != io.EOF || + !err.ExitedDuringCommand { + t.Fatalf("the arguments were not carried over: %#v", err) + } +} + +// Verifies a project without a lockfile is reported as a normal shutdown. +func TestNewUnityNotRunningErrorWithoutALockfile(t *testing.T) { + err := NewUnityNotRunningError(t.TempDir(), "", nil, false) + + if err.LockfileLeftBehind { + t.Fatal("a project without a lockfile must not be reported as an abnormal exit") + } +} diff --git a/cli/common/errors/error_envelope.go b/cli/common/errors/error_envelope.go index c4460d6bbf..25904bf206 100644 --- a/cli/common/errors/error_envelope.go +++ b/cli/common/errors/error_envelope.go @@ -14,6 +14,7 @@ const ( ErrorCodeUnknownCommand = "UNKNOWN_COMMAND" errorCodeProjectNotFound = "PROJECT_NOT_FOUND" ErrorCodeUnityNotReachable = "UNITY_NOT_REACHABLE" + ErrorCodeUnityNotRunning = "UNITY_NOT_RUNNING" ErrorCodeUnityStartupTimeout = "UNITY_STARTUP_TIMEOUT" ErrorCodeUnityProcessExitTimeout = "UNITY_PROCESS_EXIT_TIMEOUT" errorCodeUnityDisconnectedAfterDispatch = "UNITY_DISCONNECTED_AFTER_DISPATCH" @@ -57,6 +58,7 @@ const ( ErrorPhaseExecution = "execution" commandRunTests = "run-tests" + commandLaunch = "launch" // Why this wording: --skip-compile only skips the CLI-side compile, so agents read a // disconnect here as an IPC fault instead of the domain reload that discarded their patches. diff --git a/cli/common/errors/transport_errors.go b/cli/common/errors/transport_errors.go index a0abd491f9..732e6edede 100644 --- a/cli/common/errors/transport_errors.go +++ b/cli/common/errors/transport_errors.go @@ -60,6 +60,21 @@ func IsPermanentConnectError(err error) bool { return errors.Is(connectionErr, os.ErrPermission) } +// IsUnityServerGoneError reports whether a failed request shows that Unity's server went away: the +// connection dropped mid-request, or nobody was listening. +func IsUnityServerGoneError(err error) bool { + if IsTransportDisconnectError(err) { + return true + } + var connectionErr *unityipc.ConnectionAttemptError + if !errors.As(err, &connectionErr) { + return false + } + // Why not a connect timeout or a denied connect: a Windows named pipe times out while all of its + // instances are busy, and a sandbox can deny the connect, both while the server is alive. + return !IsFinalResponseTimeoutError(err) && !IsPermanentConnectError(err) +} + // Reports whether the error is a connection deadline expiry. The Timeout() probe // runs through the unwrap chain because go-winio's named pipe deadline error is not // os.ErrDeadlineExceeded and os.IsTimeout does not unwrap fmt.Errorf("%w") wrapping. diff --git a/cli/common/errors/transport_errors_test.go b/cli/common/errors/transport_errors_test.go index bc6c8627a5..0fac82acd5 100644 --- a/cli/common/errors/transport_errors_test.go +++ b/cli/common/errors/transport_errors_test.go @@ -184,3 +184,39 @@ func TestIsFinalResponseTimeoutErrorUnwrapsTimeoutCauses(t *testing.T) { t.Fatal("wrapped Timeout()-reporting error was not classified as timeout") } } + +// Verifies a dropped connection and a dial nobody answered count as the server being gone, while a +// dial timeout and a denied dial do not, because the server can be alive behind both. +func TestIsUnityServerGoneErrorSeparatesAGoneServerFromALiveOne(t *testing.T) { + cases := []struct { + name string + err error + want bool + }{ + {name: "dropped connection", err: fmt.Errorf("read: %w", io.EOF), want: true}, + { + name: "nobody listening", + err: &unityipc.ConnectionAttemptError{Cause: syscall.ECONNREFUSED}, + want: true, + }, + { + name: "dial timeout", + err: &unityipc.ConnectionAttemptError{Cause: os.ErrDeadlineExceeded}, + want: false, + }, + { + name: "denied dial", + err: &unityipc.ConnectionAttemptError{Cause: syscall.EPERM}, + want: false, + }, + {name: "unrelated error", err: fmt.Errorf("decode failed"), want: false}, + } + + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + if got := IsUnityServerGoneError(testCase.err); got != testCase.want { + t.Fatalf("IsUnityServerGoneError(%v) = %v, want %v", testCase.err, got, testCase.want) + } + }) + } +} diff --git a/cli/common/errors/unity_not_running.go b/cli/common/errors/unity_not_running.go new file mode 100644 index 0000000000..c26c5db559 --- /dev/null +++ b/cli/common/errors/unity_not_running.go @@ -0,0 +1,105 @@ +package clierrors + +// UnityNotRunningError reports that a process lookup that succeeded found no Unity Editor for the +// project, after the server could not be reached. Only a lookup that answered may produce it: a lookup +// that failed observed nothing and must leave the connection error as it is. +// +// Why it neither unwraps to Cause nor repeats it in Error(): the callers that wait out a domain reload +// recognise a dropped connection through errors.Is and through message text such as "EOF". Matching +// either would send them into a wait for a server that cannot come back, which is the failure this error +// exists to end. The cause is kept for Details only. +type UnityNotRunningError struct { + ProjectRoot string + Endpoint string + Cause error + // LockfileLeftBehind is true when Temp/UnityLockfile, which Unity removes when it shuts down normally, + // is still there. + LockfileLeftBehind bool + // EditorLogPath is the log this Editor wrote at Unity's default location; empty when no log there is + // this Editor's. + EditorLogPath string + // ExitedDuringCommand is true when this command had reached Unity before it went away, so the command + // may have partly run. + ExitedDuringCommand bool +} + +func (err UnityNotRunningError) Error() string { + if err.ExitedDuringCommand { + return "Unity exited while the command was running" + } + return "no Unity Editor is running for this project" +} + +func (err UnityNotRunningError) ToCLIError(context ErrorContext) CLIError { + launching := err.ExitedDuringCommand && context.Command == commandLaunch + phase := ErrorPhaseConnection + safeToRetry := true + // Why launch stays safe: running it again starts a new Editor and cannot double-apply anything. + if err.ExitedDuringCommand && !launching { + phase = ErrorPhaseResponseWaiting + safeToRetry = isSafeRetryCommand(context.Command) + } + return CLIError{ + ErrorCode: ErrorCodeUnityNotRunning, + Phase: phase, + Message: err.message(launching), + Retryable: false, + SafeToRetry: safeToRetry, + ProjectRoot: firstNonEmpty(context.ProjectRoot, err.ProjectRoot), + Command: context.Command, + NextActions: err.nextActions(launching, safeToRetry), + Details: err.details(), + } +} + +// Why "may have crashed" and not "crashed": a lockfile left behind proves only that the normal shutdown +// did not finish, which a forced quit or an interrupted cleanup also leaves. +func (err UnityNotRunningError) message(launching bool) string { + message := "No Unity Editor is running for this project." + if launching { + message = "Unity exited before it finished starting; no Unity Editor is running for this project now." + } else if err.ExitedDuringCommand { + message = "Unity exited while this command was running; no Unity Editor is running for this project now." + } + if err.LockfileLeftBehind { + message += " It left Temp/UnityLockfile behind, so it did not shut down normally and may have crashed." + } + return message +} + +func (err UnityNotRunningError) nextActions(launching bool, safeToRetry bool) []string { + actions := err.EditorLogNextActions() + if launching { + return append(actions, "Run `uloop launch` again.") + } + if !safeToRetry { + actions = append(actions, "The command may have partly run before Unity exited; check the project and scene state before running it again.") + } + return append(actions, "Run `uloop launch` to start Unity, then run the command again.") +} + +// EditorLogNextActions returns the step that sends the caller to this Editor's log before launching Unity +// again, or no step when the Editor shut down normally or no log of it was found. +// Why before the launch step: every Editor start renames Editor.log to Editor-prev.log and replaces the +// previous Editor-prev.log, so launching first can lose the record of why the Editor went away. +func (err UnityNotRunningError) EditorLogNextActions() []string { + if !err.LockfileLeftBehind || err.EditorLogPath == "" { + return []string{} + } + return []string{"Before launching, read the end of this Editor's log for why it exited: " + err.EditorLogPath + + " (the next Editor start moves or replaces it)."} +} + +func (err UnityNotRunningError) details() map[string]any { + details := map[string]any{} + if err.Endpoint != "" { + details["Endpoint"] = err.Endpoint + } + if err.Cause != nil { + details["Cause"] = err.Cause.Error() + } + if err.LockfileLeftBehind && err.EditorLogPath != "" { + details["EditorLog"] = err.EditorLogPath + } + return details +} diff --git a/cli/common/errors/unity_not_running_test.go b/cli/common/errors/unity_not_running_test.go new file mode 100644 index 0000000000..9cb7526e57 --- /dev/null +++ b/cli/common/errors/unity_not_running_test.go @@ -0,0 +1,192 @@ +package clierrors + +import ( + "bytes" + "encoding/json" + "errors" + "io" + "strings" + "testing" + + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +const unityNotRunningTestEditorLog = "/home/user/Library/Logs/Unity/Editor.log" + +// Verifies a command that never reached Unity reports a closed Editor as a condition retrying cannot +// clear, with launching Unity as the first step and no crash guidance. +func TestUnityNotRunningErrorBeforeTheCommandReachedUnity(t *testing.T) { + err := UnityNotRunningError{ + ProjectRoot: "/projects/sample", + Endpoint: "/tmp/uloop/sample.sock", + Cause: errors.New("dial unix /tmp/uloop/sample.sock: connect: no such file or directory"), + } + + cliErr := ClassifyError(err, ErrorContext{Command: "get-logs"}) + + if cliErr.ErrorCode != ErrorCodeUnityNotRunning || cliErr.Phase != ErrorPhaseConnection { + t.Fatalf("unexpected classification: %#v", cliErr) + } + if cliErr.Retryable { + t.Fatal("retrying cannot start Unity, so the error must not be Retryable") + } + if !cliErr.SafeToRetry { + t.Fatal("nothing reached Unity, so running the command again must be SafeToRetry") + } + if cliErr.Message != "No Unity Editor is running for this project." { + t.Fatalf("unexpected message: %q", cliErr.Message) + } + if len(cliErr.NextActions) != 1 || !strings.Contains(cliErr.NextActions[0], "`uloop launch`") { + t.Fatalf("expected launch as the only next action, got %#v", cliErr.NextActions) + } + if cliErr.ProjectRoot != "/projects/sample" { + t.Fatalf("expected the error's project root, got %q", cliErr.ProjectRoot) + } + if cliErr.Details["Cause"] != err.Cause.Error() || cliErr.Details["Endpoint"] != err.Endpoint { + t.Fatalf("expected the cause and endpoint in Details, got %#v", cliErr.Details) + } + if _, ok := cliErr.Details["EditorLog"]; ok { + t.Fatalf("a clean exit must not point at the Editor log, got %#v", cliErr.Details) + } +} + +// Verifies a lockfile left behind is reported as an abnormal exit without asserting a crash, and that +// the Editor's own log is named before launch because the next Editor start moves or replaces it. +func TestUnityNotRunningErrorPointsAtTheEditorLogWhenTheLockfileWasLeftBehind(t *testing.T) { + err := UnityNotRunningError{ + ProjectRoot: "/projects/sample", + LockfileLeftBehind: true, + EditorLogPath: unityNotRunningTestEditorLog, + } + + cliErr := err.ToCLIError(ErrorContext{Command: "get-logs"}) + + if !strings.Contains(cliErr.Message, "did not shut down normally") || + !strings.Contains(cliErr.Message, "may have crashed") { + t.Fatalf("expected an abnormal-exit message, got %q", cliErr.Message) + } + if len(cliErr.NextActions) != 2 { + t.Fatalf("expected the log step and the launch step, got %#v", cliErr.NextActions) + } + if !strings.Contains(cliErr.NextActions[0], unityNotRunningTestEditorLog) || + !strings.Contains(cliErr.NextActions[0], "next Editor start") { + t.Fatalf("expected the Editor log step first, got %q", cliErr.NextActions[0]) + } + if !strings.Contains(cliErr.NextActions[1], "`uloop launch`") { + t.Fatalf("expected launch after the log step, got %q", cliErr.NextActions[1]) + } + if cliErr.Details["EditorLog"] != unityNotRunningTestEditorLog { + t.Fatalf("expected the Editor log path in Details, got %#v", cliErr.Details) + } +} + +// Verifies no Editor log step appears when no log at the default location is this Editor's, since +// pointing at another Editor's log would send the caller to the wrong reason for the exit. +func TestUnityNotRunningErrorOmitsTheEditorLogStepWithoutTheEditorsLog(t *testing.T) { + err := UnityNotRunningError{LockfileLeftBehind: true} + + cliErr := err.ToCLIError(ErrorContext{Command: "get-logs"}) + + if len(cliErr.NextActions) != 1 || !strings.Contains(cliErr.NextActions[0], "`uloop launch`") { + t.Fatalf("expected the launch step only, got %#v", cliErr.NextActions) + } + if _, ok := cliErr.Details["EditorLog"]; ok { + t.Fatalf("no log path may appear in Details, got %#v", cliErr.Details) + } +} + +// Verifies an Editor that went away while a mutating command was running warns that the command may +// have partly run, and marks the command unsafe to repeat. +func TestUnityNotRunningErrorAfterUnityExitedDuringAMutatingCommand(t *testing.T) { + err := UnityNotRunningError{ExitedDuringCommand: true} + + cliErr := err.ToCLIError(ErrorContext{Command: "compile"}) + + if cliErr.Phase != ErrorPhaseResponseWaiting { + t.Fatalf("expected the response-waiting phase, got %q", cliErr.Phase) + } + if cliErr.SafeToRetry { + t.Fatal("compile may have partly run, so it must not be SafeToRetry") + } + if !strings.HasPrefix(cliErr.Message, "Unity exited while this command was running") { + t.Fatalf("expected the exit-during-command message, got %q", cliErr.Message) + } + if len(cliErr.NextActions) != 2 || !strings.Contains(cliErr.NextActions[0], "partly run") { + t.Fatalf("expected the partial-run warning before launch, got %#v", cliErr.NextActions) + } +} + +// Verifies a read-only command that was running when Unity went away stays safe to repeat and gets no +// partial-run warning. +func TestUnityNotRunningErrorAfterUnityExitedDuringAReadOnlyCommand(t *testing.T) { + err := UnityNotRunningError{ExitedDuringCommand: true} + + cliErr := err.ToCLIError(ErrorContext{Command: "get-logs"}) + + if !cliErr.SafeToRetry { + t.Fatal("get-logs cannot double-apply, so it must stay SafeToRetry") + } + if len(cliErr.NextActions) != 1 { + t.Fatalf("expected only the launch step, got %#v", cliErr.NextActions) + } +} + +// Verifies the error never reads as a dropped connection, even when its cause is one: callers that see a +// disconnect start waiting for a domain reload that cannot come. +func TestUnityNotRunningErrorDoesNotLookLikeADisconnect(t *testing.T) { + err := UnityNotRunningError{ExitedDuringCommand: true, Cause: io.EOF} + + if IsTransportDisconnectError(err) { + t.Fatalf("UnityNotRunningError must not be classified as a disconnect: %v", err) + } + if IsUnityServerGoneError(err) { + t.Fatalf("UnityNotRunningError must not be classified as a server that may come back: %v", err) + } +} + +// Verifies the tool failure writer prints the not-running envelope for a dispatched request instead of +// the disconnect envelope that tells the caller to wait for a reload. +func TestWriteToolFailureReportsUnityNotRunningForADispatchedRequest(t *testing.T) { + var output bytes.Buffer + err := UnityNotRunningError{ExitedDuringCommand: true, Cause: io.EOF} + + WriteToolFailure( + &output, + err, + unityipc.UnitySendOutcome{RequestDispatched: true, RequestAccepted: true}, + ErrorContext{Command: "run-tests"}, + ) + + var envelope CLIErrorEnvelope + if decodeErr := json.Unmarshal(output.Bytes(), &envelope); decodeErr != nil { + t.Fatalf("decoding the envelope failed: %v\n%s", decodeErr, output.String()) + } + if envelope.Error.ErrorCode != ErrorCodeUnityNotRunning { + t.Fatalf("expected %s, got %#v", ErrorCodeUnityNotRunning, envelope.Error) + } +} + +// Verifies an Editor that exited while launch waited for it is reported as a startup that did not +// finish: launch stays safe to run again, and no step tells the caller to run launch and then launch. +func TestUnityNotRunningErrorAfterUnityExitedDuringLaunch(t *testing.T) { + err := UnityNotRunningError{ExitedDuringCommand: true, LockfileLeftBehind: true, EditorLogPath: unityNotRunningTestEditorLog} + + cliErr := err.ToCLIError(ErrorContext{Command: "launch"}) + + if !cliErr.SafeToRetry { + t.Fatal("running launch again cannot double-apply anything, so it must stay SafeToRetry") + } + if !strings.HasPrefix(cliErr.Message, "Unity exited before it finished starting") || + !strings.Contains(cliErr.Message, "may have crashed") { + t.Fatalf("expected the startup-exit message with the abnormal-exit note, got %q", cliErr.Message) + } + if len(cliErr.NextActions) != 2 { + t.Fatalf("expected the Editor log step and the launch step, got %#v", cliErr.NextActions) + } + if !strings.Contains(cliErr.NextActions[0], unityNotRunningTestEditorLog) { + t.Fatalf("expected the Editor log step first, got %q", cliErr.NextActions[0]) + } + if cliErr.NextActions[1] != "Run `uloop launch` again." { + t.Fatalf("expected launch to be run again, got %q", cliErr.NextActions[1]) + } +} diff --git a/cli/common/unityprocess/editor_files.go b/cli/common/unityprocess/editor_files.go new file mode 100644 index 0000000000..9f8851db36 --- /dev/null +++ b/cli/common/unityprocess/editor_files.go @@ -0,0 +1,119 @@ +package unityprocess + +import ( + "errors" + "io" + "os" + "path/filepath" + "runtime" + "strings" +) + +const ( + // TempDirectoryName is the project directory Unity keeps its per-session files in. + TempDirectoryName = "Temp" + // LockfileName is the file Unity holds while it has the project open and removes on a normal + // shutdown. + LockfileName = "UnityLockfile" +) + +// LockfilePath returns where Unity keeps the lockfile for the project at projectRoot. +func LockfilePath(projectRoot string) string { + return filepath.Join(projectRoot, TempDirectoryName, LockfileName) +} + +// DefaultEditorLogPath returns where Unity writes its Editor log when it was not started with -logFile, +// or an empty string when the platform's base directory cannot be resolved. +func DefaultEditorLogPath() string { + homeDir, err := os.UserHomeDir() + if err != nil { + homeDir = "" + } + return editorLogPathFor(runtime.GOOS, homeDir, os.Getenv("LOCALAPPDATA")) +} + +const ( + previousEditorLogName = "Editor-prev.log" + // editorLogProjectPathMarker starts the line an Editor writes near the top of its log once it has + // opened a project. + editorLogProjectPathMarker = "Successfully changed project path to: " + // editorLogHeadBytes bounds how much of a log is read to find its project. The marker line follows the + // command line arguments within the first few kilobytes, and a log can grow to many megabytes. + editorLogHeadBytes = 64 * 1024 +) + +// EditorLogPathFor returns the log the Editor of the project at projectRoot wrote at the default location, +// or an empty string when no log there is that Editor's. +// Why either log can miss: every Editor start renames Editor.log to Editor-prev.log, so the log moves once +// any other Editor starts and is gone after a second start, and -logFile writes it elsewhere. +func EditorLogPathFor(projectRoot string) string { + return projectEditorLogPath(DefaultEditorLogPath(), projectRoot) +} + +func projectEditorLogPath(editorLogPath string, projectRoot string) string { + if editorLogPath == "" { + return "" + } + previousLogPath := filepath.Join(filepath.Dir(editorLogPath), previousEditorLogName) + for _, candidate := range []string{editorLogPath, previousLogPath} { + if editorLogBelongsTo(candidate, projectRoot) { + return candidate + } + } + return "" +} + +func editorLogBelongsTo(logPath string, projectRoot string) bool { + file, err := os.Open(logPath) + if err != nil { + return false + } + defer func() { _ = file.Close() }() + + head := make([]byte, editorLogHeadBytes) + readBytes, err := io.ReadFull(file, head) + if err != nil && !errors.Is(err, io.ErrUnexpectedEOF) && !errors.Is(err, io.EOF) { + return false + } + loggedProjectPath := loggedEditorProjectPath(string(head[:readBytes])) + return loggedProjectPath != "" && isProjectPath(loggedProjectPath, projectRoot) +} + +// loggedEditorProjectPath returns the project path a log head records, or an empty string when it records +// none. +func loggedEditorProjectPath(head string) string { + markerIndex := strings.Index(head, editorLogProjectPathMarker) + if markerIndex < 0 { + return "" + } + line := head[markerIndex+len(editorLogProjectPathMarker):] + if lineEnd := strings.IndexByte(line, '\n'); lineEnd >= 0 { + line = line[:lineEnd] + } + // Why TrimSpace: a log written on Windows ends the line with "\r\n". + return strings.TrimSpace(line) +} + +// editorLogPathFor takes the platform and its base directories as parameters so every platform's +// layout can be checked on any host. +func editorLogPathFor(goos string, homeDir string, localAppData string) string { + switch goos { + case "darwin": + return joinUnderBase(homeDir, "Library", "Logs", "Unity", "Editor.log") + case "windows": + return joinUnderBase(localAppData, "Unity", "Editor", "Editor.log") + case "linux": + return joinUnderBase(homeDir, ".config", "unity3d", "Editor.log") + default: + return "" + } +} + +// Why refuse an empty base: filepath.Join would turn it into a path relative to the working directory, +// which points at a file that does not exist. +func joinUnderBase(base string, elements ...string) string { + if base == "" { + return "" + } + return filepath.Join(append([]string{base}, elements...)...) +} diff --git a/cli/common/unityprocess/editor_files_test.go b/cli/common/unityprocess/editor_files_test.go new file mode 100644 index 0000000000..b8a53685dd --- /dev/null +++ b/cli/common/unityprocess/editor_files_test.go @@ -0,0 +1,119 @@ +package unityprocess + +import ( + "os" + "path/filepath" + "strings" + "testing" +) + +// Verifies the lockfile is looked up where Unity keeps it, under the project's Temp directory. +func TestLockfilePathIsUnderTheProjectTempDirectory(t *testing.T) { + projectRoot := filepath.Join("projects", "sample") + + if got, want := LockfilePath(projectRoot), filepath.Join(projectRoot, "Temp", "UnityLockfile"); got != want { + t.Fatalf("LockfilePath() = %q, want %q", got, want) + } +} + +// Verifies the default Editor log location for each platform Unity documents, and that a missing base +// directory yields no path instead of one rooted in the working directory. +func TestEditorLogPathForEachPlatform(t *testing.T) { + const homeDir = "/home/" + const localAppData = `C:\Users\\AppData\Local` + cases := []struct { + name string + goos string + homeDir string + localAppData string + want string + }{ + {name: "macOS", goos: "darwin", homeDir: homeDir, want: filepath.Join(homeDir, "Library", "Logs", "Unity", "Editor.log")}, + {name: "Windows", goos: "windows", localAppData: localAppData, want: filepath.Join(localAppData, "Unity", "Editor", "Editor.log")}, + {name: "Linux", goos: "linux", homeDir: homeDir, want: filepath.Join(homeDir, ".config", "unity3d", "Editor.log")}, + {name: "macOS without a home directory", goos: "darwin", want: ""}, + {name: "Windows without LOCALAPPDATA", goos: "windows", homeDir: homeDir, want: ""}, + {name: "Linux without a home directory", goos: "linux", want: ""}, + {name: "other platform", goos: "plan9", homeDir: homeDir, want: ""}, + } + + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + got := editorLogPathFor(testCase.goos, testCase.homeDir, testCase.localAppData) + if got != testCase.want { + t.Fatalf("editorLogPathFor() = %q, want %q", got, testCase.want) + } + }) + } +} + +// writeEditorLogForTest writes an Editor log whose head records projectPath as Unity does at startup, +// with the given line ending. +func writeEditorLogForTest(t *testing.T, logPath string, projectPath string, lineEnding string) { + t.Helper() + lines := []string{ + "COMMAND LINE ARGUMENTS:", + "Unity", + "-projectPath", + projectPath, + "Successfully changed project path to: " + projectPath, + "Loading GUID <-> Path mappings...", + } + if err := os.WriteFile(logPath, []byte(strings.Join(lines, lineEnding)+lineEnding), 0o644); err != nil { + t.Fatalf("writing %s failed: %v", logPath, err) + } +} + +// Verifies the Editor log that names the project is picked whether it is still Editor.log or a later +// Editor start already renamed it to Editor-prev.log, and that logs of other projects, which every later +// start leaves at both names, are never picked. +func TestProjectEditorLogPath(t *testing.T) { + projectRoot := t.TempDir() + otherProject := t.TempDir() + cases := []struct { + name string + currentLog string + previousLog string + lineEnding string + wantPrevious bool + wantNone bool + }{ + {name: "Editor.log is the project's", currentLog: projectRoot, previousLog: otherProject, lineEnding: "\n"}, + {name: "renamed to Editor-prev.log", currentLog: otherProject, previousLog: projectRoot, lineEnding: "\n", wantPrevious: true}, + {name: "Windows line endings", currentLog: projectRoot, lineEnding: "\r\n"}, + {name: "both are other projects'", currentLog: otherProject, previousLog: otherProject, lineEnding: "\n", wantNone: true}, + {name: "no log at all", lineEnding: "\n", wantNone: true}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + logDirectory := t.TempDir() + currentLogPath := filepath.Join(logDirectory, "Editor.log") + previousLogPath := filepath.Join(logDirectory, "Editor-prev.log") + if testCase.currentLog != "" { + writeEditorLogForTest(t, currentLogPath, testCase.currentLog, testCase.lineEnding) + } + if testCase.previousLog != "" { + writeEditorLogForTest(t, previousLogPath, testCase.previousLog, testCase.lineEnding) + } + want := currentLogPath + if testCase.wantPrevious { + want = previousLogPath + } + if testCase.wantNone { + want = "" + } + + if got := projectEditorLogPath(currentLogPath, projectRoot); got != want { + t.Fatalf("projectEditorLogPath() = %q, want %q", got, want) + } + }) + } +} + +// Verifies no log is picked when the default log location could not be resolved, instead of looking in +// the working directory. +func TestProjectEditorLogPathWithoutADefaultLocation(t *testing.T) { + if got := projectEditorLogPath("", t.TempDir()); got != "" { + t.Fatalf("projectEditorLogPath() = %q, want no log", got) + } +} diff --git a/cli/common/unityprocess/process.go b/cli/common/unityprocess/process.go index 6f8f734e6c..45a509598d 100644 --- a/cli/common/unityprocess/process.go +++ b/cli/common/unityprocess/process.go @@ -7,6 +7,7 @@ import ( "encoding/binary" "errors" "fmt" + "os" "os/exec" "path" "path/filepath" @@ -22,39 +23,104 @@ const windowsPowerShellCommand = "powershell" var ( macUnityExecutablePattern = regexp.MustCompile(`(?i)Unity\.app/Contents/MacOS/Unity`) windowsUnityExecutablePattern = regexp.MustCompile(`(?i)Unity\.exe`) - projectPathFlagPattern = regexp.MustCompile(`(?i)-projectpath(?:=|\s+)(.+)$`) - nextUnityFlagPattern = regexp.MustCompile(`\s-[A-Za-z][A-Za-z0-9-]*(?:=|\s|$)`) + // Why -createProject too: the Editor that creates a project opens it in the same session, and that + // session's command line may name the project only through -createProject. + projectPathFlagPattern = regexp.MustCompile(`(?i)-(?:projectpath|createproject)(?:=|\s+)(.+)$`) + nextUnityFlagPattern = regexp.MustCompile(`\s-[A-Za-z][A-Za-z0-9-]*(?:=|\s|$)`) ) type UnityProcess struct { - Pid int + Pid int + // projectPath is empty for an Editor whose command line does not name its project. projectPath string } +// ErrUnidentifiedUnityEditor reports that no Editor is listed for the project while an Editor whose +// command line does not name its project is running, so that Editor may be the project's. +var ErrUnidentifiedUnityEditor = errors.New( + "a Unity Editor whose command line does not name its project is running, so it may be this project's Editor") + type RestoreFocusFunc func(context.Context) error +// FindRunningUnityProcess returns the running Editor listed for projectRoot, or nil when none is. func FindRunningUnityProcess(ctx context.Context, projectRoot string) (*UnityProcess, error) { processes, err := listUnityProcesses(ctx) if err != nil { return nil, err } + process, _, err := selectProjectProcess(processes, projectRoot) + return process, err +} - normalizedTarget, err := normalizeComparablePath(projectRoot) +// FindRunningUnityProcessDefinitively is FindRunningUnityProcess for callers that conclude from a nil +// result that no Editor is running for the project: it returns nil only when no running Editor can be +// the project's, and ErrUnidentifiedUnityEditor when an Editor whose project is unknown may be. +func FindRunningUnityProcessDefinitively(ctx context.Context, projectRoot string) (*UnityProcess, error) { + processes, err := listUnityProcesses(ctx) if err != nil { return nil, err } + return selectProjectProcessDefinitively(processes, projectRoot) +} + +func selectProjectProcessDefinitively(processes []UnityProcess, projectRoot string) (*UnityProcess, error) { + process, unidentifiedRunning, err := selectProjectProcess(processes, projectRoot) + if err == nil && process == nil && unidentifiedRunning { + return nil, ErrUnidentifiedUnityEditor + } + return process, err +} + +// selectProjectProcess picks projectRoot's Editor from the listed Editors, and reports whether an Editor +// whose project is unknown was listed. +func selectProjectProcess(processes []UnityProcess, projectRoot string) (*UnityProcess, bool, error) { + if _, err := normalizeComparablePath(projectRoot); err != nil { + return nil, false, err + } + unidentifiedRunning := false for _, candidate := range processes { - normalizedCandidate, err := normalizeComparablePath(candidate.projectPath) - if err != nil { + // Why not compare an empty path: filepath.Abs resolves it to the working directory, so it + // would match a project the command runs from. + if candidate.projectPath == "" { + unidentifiedRunning = true continue } - if normalizedCandidate == normalizedTarget { + if isProjectPath(candidate.projectPath, projectRoot) { process := candidate - return &process, nil + return &process, unidentifiedRunning, nil } } - return nil, nil + return nil, unidentifiedRunning, nil +} + +// isProjectPath reports whether path, as an Editor's command line or log records it, names the project at +// projectRoot. +func isProjectPath(path string, projectRoot string) bool { + normalizedPath, err := normalizeComparablePath(path) + if err != nil { + return false + } + normalizedRoot, err := normalizeComparablePath(projectRoot) + if err != nil { + return false + } + return normalizedPath == normalizedRoot || isSameDirectory(path, projectRoot) +} + +// isSameDirectory reports whether both paths name one existing directory. Why on top of the string +// comparison: a command's project root has its symlinks evaluated, while an Editor's command line keeps +// the path it was started with, and a case-insensitive file system accepts either spelling. +func isSameDirectory(first string, second string) bool { + firstInfo, err := os.Stat(first) + if err != nil { + return false + } + secondInfo, err := os.Stat(second) + if err != nil { + return false + } + return os.SameFile(firstInfo, secondInfo) } func listUnityProcesses(ctx context.Context) ([]UnityProcess, error) { @@ -135,11 +201,7 @@ func matchMacUnityProcess(pid int, command string) (UnityProcess, bool) { if !isUnityEditorCommand(command, macUnityExecutablePattern) { return UnityProcess{}, false } - projectPath := extractProjectPath(command) - if projectPath == "" { - return UnityProcess{}, false - } - return UnityProcess{Pid: pid, projectPath: projectPath}, true + return UnityProcess{Pid: pid, projectPath: extractProjectPath(command)}, true } // parseMacProcArgs2 decodes the kern.procargs2 sysctl buffer layout: a leading int32 @@ -206,11 +268,7 @@ func matchLinuxUnityProcess(pid int, args []string) (UnityProcess, bool) { if hasNonEditorLinuxArg(args) { return UnityProcess{}, false } - projectPath := linuxProjectPathFromArgs(args) - if projectPath == "" { - return UnityProcess{}, false - } - return UnityProcess{Pid: pid, projectPath: projectPath}, true + return UnityProcess{Pid: pid, projectPath: linuxProjectPathFromArgs(args)}, true } // isLinuxUnityEditorExecutable reports whether argv[0]'s basename is a Unity Editor @@ -237,20 +295,21 @@ func hasNonEditorLinuxArg(args []string) bool { return false } -// linuxProjectPathFromArgs returns the value of Unity's -projectPath flag, accepting +// linuxProjectPathFromArgs returns the value of Unity's -projectPath or -createProject flag, accepting // both the "-projectPath " and "-projectPath=" forms case-insensitively. func linuxProjectPathFromArgs(args []string) string { - const projectPathFlag = "-projectpath" for index, arg := range args { lowerArg := strings.ToLower(arg) - if lowerArg == projectPathFlag { - if index+1 >= len(args) { - return "" + for _, flag := range []string{"-projectpath", "-createproject"} { + if lowerArg == flag { + if index+1 >= len(args) { + return "" + } + return args[index+1] + } + if strings.HasPrefix(lowerArg, flag+"=") { + return arg[len(flag+"="):] } - return args[index+1] - } - if strings.HasPrefix(lowerArg, projectPathFlag+"=") { - return arg[len(projectPathFlag+"="):] } } return "" @@ -283,12 +342,7 @@ func parseWindowsUnityProcesses(output string) []UnityProcess { if !isUnityEditorCommand(command, windowsUnityExecutablePattern) { continue } - projectPath := extractProjectPath(command) - if projectPath == "" { - continue - } - - processes = append(processes, UnityProcess{Pid: pid, projectPath: projectPath}) + processes = append(processes, UnityProcess{Pid: pid, projectPath: extractProjectPath(command)}) } return processes } diff --git a/cli/common/unityprocess/process_branches_test.go b/cli/common/unityprocess/process_branches_test.go index 7184191c01..687fc97fb7 100644 --- a/cli/common/unityprocess/process_branches_test.go +++ b/cli/common/unityprocess/process_branches_test.go @@ -5,6 +5,7 @@ import ( "encoding/binary" "errors" "fmt" + "os" "os/exec" "path/filepath" "runtime" @@ -12,14 +13,106 @@ import ( "testing" ) -// Verifies a Unity editor command without any -projectPath flag is not reported as a project's Editor. -func TestMatchMacUnityProcessRejectsCommandWithoutProjectPath(t *testing.T) { +// Verifies a Unity editor command without a project flag is kept as an Editor whose project is unknown, +// so the lookup can tell it apart from no Editor at all. +func TestMatchMacUnityProcessKeepsAnEditorWithoutProjectPathUnidentified(t *testing.T) { process, matched := matchMacUnityProcess( 123, `/Applications/Unity/Hub/Editor/6000.0.0f1/Unity.app/Contents/MacOS/Unity -useHub -hubIPC`) - if matched { - t.Fatalf("expected no match without a project path, got %#v", process) + if !matched || process.Pid != 123 || process.projectPath != "" { + t.Fatalf("expected an Editor with no project path, got matched = %v, %#v", matched, process) + } +} + +// Verifies the project's Editor is picked by its project path, and that an Editor whose project is +// unknown is reported beside the result and never matched, not even for a project at the working +// directory, which an empty path would resolve to. +func TestSelectProjectProcess(t *testing.T) { + workingDirectory, err := os.Getwd() + if err != nil { + t.Fatalf("reading the working directory failed: %v", err) + } + projectRoot := t.TempDir() + unidentified := UnityProcess{Pid: 1} + project := UnityProcess{Pid: 2, projectPath: projectRoot} + other := UnityProcess{Pid: 3, projectPath: filepath.Join(projectRoot, "Other")} + cases := []struct { + name string + processes []UnityProcess + projectRoot string + wantPid int + wantUnidentified bool + }{ + {name: "project Editor beside an unidentified one", processes: []UnityProcess{unidentified, project}, projectRoot: projectRoot, wantPid: 2, wantUnidentified: true}, + {name: "only another project's Editor", processes: []UnityProcess{other}, projectRoot: projectRoot}, + {name: "only an unidentified Editor", processes: []UnityProcess{other, unidentified}, projectRoot: projectRoot, wantUnidentified: true}, + {name: "unidentified Editor and a project at the working directory", processes: []UnityProcess{unidentified}, projectRoot: workingDirectory, wantUnidentified: true}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + process, unidentifiedRunning, err := selectProjectProcess(testCase.processes, testCase.projectRoot) + if err != nil { + t.Fatalf("selectProjectProcess failed: %v", err) + } + gotPid := 0 + if process != nil { + gotPid = process.Pid + } + if gotPid != testCase.wantPid || unidentifiedRunning != testCase.wantUnidentified { + t.Fatalf("pid = %d, unidentified = %v, want pid = %d, unidentified = %v", + gotPid, unidentifiedRunning, testCase.wantPid, testCase.wantUnidentified) + } + }) + } +} + +// Verifies the lookup for callers that conclude Unity is not running answers "none" only when no listed +// Editor can be the project's: an Editor of unknown project turns "none" into ErrUnidentifiedUnityEditor, +// while the project's own Editor is still returned beside one. +func TestSelectProjectProcessDefinitively(t *testing.T) { + projectRoot := t.TempDir() + unidentified := UnityProcess{Pid: 1} + project := UnityProcess{Pid: 2, projectPath: projectRoot} + other := UnityProcess{Pid: 3, projectPath: filepath.Join(projectRoot, "Other")} + cases := []struct { + name string + processes []UnityProcess + wantPid int + wantErr error + }{ + {name: "unidentified Editor only", processes: []UnityProcess{other, unidentified}, wantErr: ErrUnidentifiedUnityEditor}, + {name: "project Editor beside an unidentified one", processes: []UnityProcess{unidentified, project}, wantPid: 2}, + {name: "no Editor can be the project's", processes: []UnityProcess{other}}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + process, err := selectProjectProcessDefinitively(testCase.processes, projectRoot) + + gotPid := 0 + if process != nil { + gotPid = process.Pid + } + if gotPid != testCase.wantPid || !errors.Is(err, testCase.wantErr) || (testCase.wantErr == nil && err != nil) { + t.Fatalf("pid = %d, err = %v, want pid = %d, err = %v", gotPid, err, testCase.wantPid, testCase.wantErr) + } + }) + } +} + +// Verifies an Editor started with a path through a symlink is matched to the project root a command +// resolves, which has its symlinks evaluated, so the lookup does not report that Editor as missing. +func TestSelectProjectProcessMatchesAPathThroughASymlink(t *testing.T) { + projectRoot := t.TempDir() + linkedRoot := filepath.Join(t.TempDir(), "linked") + if err := os.Symlink(projectRoot, linkedRoot); err != nil { + t.Skipf("creating a symlink is not permitted here: %v", err) + } + + process, _, err := selectProjectProcess([]UnityProcess{{Pid: 7, projectPath: linkedRoot}}, projectRoot) + + if err != nil || process == nil || process.Pid != 7 { + t.Fatalf("expected the Editor started through the symlink, got %#v, %v", process, err) } } @@ -54,7 +147,6 @@ func TestParseWindowsUnityProcessesSkipsMalformedLines(t *testing.T) { }{ {"no delimiter", validEditorCommand}, {"pid is not a number", "notapid|" + validEditorCommand}, - {"editor command without project path", "456|" + encode(`C:\Editor\Unity.exe -useHub`)}, } for _, testCase := range cases { t.Run(testCase.name, func(t *testing.T) { diff --git a/cli/common/unityprocess/process_test.go b/cli/common/unityprocess/process_test.go index ac8a3c8c4e..3f122fcd5d 100644 --- a/cli/common/unityprocess/process_test.go +++ b/cli/common/unityprocess/process_test.go @@ -116,6 +116,17 @@ func TestParseWindowsUnityProcessesExtractsProjectPath(t *testing.T) { } } +// Verifies a Windows Editor whose command line does not name its project is kept with no project path. +func TestParseWindowsUnityProcessesKeepsAnEditorWithoutProjectPathUnidentified(t *testing.T) { + output := "123|" + base64.StdEncoding.EncodeToString([]byte(`C:\Editor\Unity.exe -useHub -hubIPC`)) + "\r\n" + + processes := parseWindowsUnityProcesses(output) + + if len(processes) != 1 || processes[0].Pid != 123 || processes[0].projectPath != "" { + t.Fatalf("expected one Editor with no project path, got %#v", processes) + } +} + // Verifies non-ASCII project paths survive the PowerShell boundary because command lines travel as UTF-8 Base64. func TestParseWindowsUnityProcessesPreservesNonASCIIProjectPath(t *testing.T) { projectPath := `C:\Users\\test[1] 検証用\proj` @@ -180,6 +191,21 @@ func TestExtractProjectPathSupportsEqualsAndSpaces(t *testing.T) { } } +// Verifies an Editor started to create a project is read as having that project open, since the Editor +// that creates a project opens it in the same session without a -projectPath flag. +func TestExtractProjectPathReadsCreateProject(t *testing.T) { + cases := map[string]string{ + `Unity -createProject "/Users//New Project" -cloneFromTemplate /tmp/template.tgz -useHub`: "/Users//New Project", + `Unity -createproject=/Users//Plain -useHub`: "/Users//Plain", + } + + for command, expected := range cases { + if actual := extractProjectPath(command); actual != expected { + t.Fatalf("project path mismatch for %q: %q", command, actual) + } + } +} + // Verifies comparable project paths preserve case on case-sensitive-capable platforms. func TestNormalizeComparablePathPreservesCaseOutsideWindows(t *testing.T) { if runtime.GOOS == "windows" { @@ -229,6 +255,8 @@ func TestMatchLinuxUnityProcessExtractsProjectPath(t *testing.T) { {"equals form with mixed case", []string{"/opt/Unity/Editor/Unity", "-projectPath=/work/p"}, "/work/p"}, {"project path containing batchmode", []string{"/opt/Unity/Editor/Unity", "-projectpath", "/work/nightly-batchmode"}, "/work/nightly-batchmode"}, {"project path containing assetimportworker", []string{"/opt/Unity/Editor/Unity", "-projectpath", "/work/AssetImportWorker-lab"}, "/work/AssetImportWorker-lab"}, + {"create project", []string{"/opt/Unity/Editor/Unity", "-createProject", "/work/New Project", "-cloneFromTemplate", "/tmp/template.tgz"}, "/work/New Project"}, + {"create project equals form", []string{"/opt/Unity/Editor/Unity", "-createproject=/work/new"}, "/work/new"}, {"unity hub hardlink name with lowercase projectpath", []string{"/home/user/Unity/Hub/Editor/2022.3.62f3/Editor/unityhub-unity-editor-2022.3.62f3", "-projectpath", "/home/user/project", "-acceptSoftwareTermsForThisRunOnly", "-useHub", "-hubIPC", "-cloudEnvironment", "production", "-licensingIpc", "LicenseClient-user", "-hubSessionId", "session-placeholder", "-accessToken", "token-placeholder"}, "/home/user/project"}, } for _, testCase := range cases { @@ -244,6 +272,25 @@ func TestMatchLinuxUnityProcessExtractsProjectPath(t *testing.T) { } } +// Verifies a Linux Editor whose command line does not name its project is kept with no project path. +func TestMatchLinuxUnityProcessKeepsAnEditorWithoutProjectPathUnidentified(t *testing.T) { + cases := []struct { + name string + args []string + }{ + {"no project path flag", []string{"/opt/Unity/Editor/Unity"}}, + {"project path flag without value", []string{"/opt/Unity/Editor/Unity", "-projectpath"}}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + process, matched := matchLinuxUnityProcess(42, testCase.args) + if !matched || process.Pid != 42 || process.projectPath != "" { + t.Fatalf("expected an Editor with no project path, got matched = %v, %#v", matched, process) + } + }) + } +} + // Verifies Linux Unity process matching rejects each non-Editor shape independently. func TestMatchLinuxUnityProcessRejectsNonEditorProcesses(t *testing.T) { cases := []struct { @@ -255,8 +302,6 @@ func TestMatchLinuxUnityProcessRejectsNonEditorProcesses(t *testing.T) { {"executable name is not Unity", []string{"/opt/Unity/Editor/Data/Tools/UnityShaderCompiler", "-projectpath", "/work/p"}}, {"asset import worker with relative hub hardlink name", []string{"unityhub-unity-editor-2022.3.62f3", "-batchMode", "-name", "AssetImportWorker0", "-projectPath", "/home/user/project"}}, {"unrelated executable with unityhub prefix in directory only", []string{"/home/user/Unity/Hub/Editor/2022.3.62f3/Editor/Data/Tools/UnityShaderCompiler", "-projectpath", "/home/user/project"}}, - {"no project path flag", []string{"/opt/Unity/Editor/Unity"}}, - {"project path flag without value", []string{"/opt/Unity/Editor/Unity", "-projectpath"}}, {"empty argv", []string{}}, } for _, testCase := range cases { diff --git a/cli/dispatcher/internal/dispatcher/launch.go b/cli/dispatcher/internal/dispatcher/launch.go index 36c0bbf683..56a313a4e8 100644 --- a/cli/dispatcher/internal/dispatcher/launch.go +++ b/cli/dispatcher/internal/dispatcher/launch.go @@ -34,8 +34,8 @@ const ( launchFreshFocusRetryDelay = 2 * time.Second projectVersionFilePath = "ProjectSettings/ProjectVersion.txt" recoveryDirectoryPath = "Assets/_Recovery" - launchTempDirectoryName = "Temp" - unityLockfileName = "UnityLockfile" + launchTempDirectoryName = unityprocess.TempDirectoryName + unityLockfileName = unityprocess.LockfileName ) var editorVersionPattern = regexp.MustCompile(`(?m)^m_EditorVersion:\s*(.+)$`) @@ -405,7 +405,7 @@ func waitForUnityStartupMarkerOrTimeout(ctx context.Context, lockfilePath string } func unityLockfilePath(projectRoot string) string { - return filepath.Join(projectRoot, launchTempDirectoryName, unityLockfileName) + return unityprocess.LockfilePath(projectRoot) } func resolveLaunchProjectRoot(startPath string, options launchOptions) (string, error) { diff --git a/cli/dispatcher/internal/dispatcher/launch_test.go b/cli/dispatcher/internal/dispatcher/launch_test.go index 6e424f22a5..e1b80655ee 100644 --- a/cli/dispatcher/internal/dispatcher/launch_test.go +++ b/cli/dispatcher/internal/dispatcher/launch_test.go @@ -696,6 +696,29 @@ func TestWaitForLaunchReadinessPreservesNoProcessReachability(t *testing.T) { } } +func TestWaitForLaunchReadinessReportsAnEditorThatExitedDuringStartup(t *testing.T) { + // Verifies an Editor that exited while launch waited for it is reported as not running, with launch + // still safe to run again, rather than as a startup timeout that says Unity is running. + deps := defaultLaunchDeps() + deps.waitForToolReadiness = func(ctx context.Context, projectRoot string, timeout time.Duration) error { + return clierrors.UnityNotRunningError{ProjectRoot: projectRoot, ExitedDuringCommand: true} + } + + err := waitForLaunchReadinessWithDeps(context.Background(), t.TempDir(), deps) + + var startupErr launchStartupTimeoutError + if errors.As(err, &startupErr) { + t.Fatalf("an exited Editor must not be classified as a startup timeout: %v", err) + } + cliErr := clierrors.ClassifyError(err, clierrors.ErrorContext{Command: clicore.LaunchCommandName}) + if cliErr.ErrorCode != clierrors.ErrorCodeUnityNotRunning || !cliErr.SafeToRetry { + t.Fatalf("expected a SafeToRetry %s, got %#v", clierrors.ErrorCodeUnityNotRunning, cliErr) + } + if !strings.HasPrefix(cliErr.Message, "Unity exited before it finished starting") { + t.Fatalf("message should say the Editor exited during startup: %#v", cliErr) + } +} + func TestWaitForLaunchReadinessPreservesParentCancellation(t *testing.T) { // Verifies caller cancellation is not converted into a launch startup timeout. deps := defaultLaunchDeps() diff --git a/cli/dispatcher/shared-inputs-stamp.json b/cli/dispatcher/shared-inputs-stamp.json index 40ae1d7507..3ae049d319 100644 --- a/cli/dispatcher/shared-inputs-stamp.json +++ b/cli/dispatcher/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "b453636d0586a882f075ef2d0e77f2da79354454" + "sharedInputsHash": "201f7fc23d2f35969582d5f290edaf13a1556929" } diff --git a/cli/project-runner/internal/projectrunner/compile_attach.go b/cli/project-runner/internal/projectrunner/compile_attach.go index 078aacd1ea..db74f2aef1 100644 --- a/cli/project-runner/internal/projectrunner/compile_attach.go +++ b/cli/project-runner/internal/projectrunner/compile_attach.go @@ -3,6 +3,7 @@ package projectrunner import ( "context" "encoding/json" + "errors" "fmt" "io" "time" @@ -183,7 +184,7 @@ func attachWaitForPendingCompile( }, deps) if waitErr != nil { spinner.Stop() - logCompileAttachResult(connection, record.RequestID, "error", false) + logCompileAttachResult(connection, record.RequestID, "error", clearCompilePendingRecordAfterEditorExit(connection.ProjectRoot, waitErr)) clierrors.WriteClassifiedError(stderr, waitErr, clierrors.ErrorContext{ ProjectRoot: connection.ProjectRoot, Command: clicore.CompileCommandName, @@ -227,6 +228,18 @@ func attachWaitForPendingCompile( } } +// clearCompilePendingRecordAfterEditorExit removes the pending record when the wait ended because the +// Editor exited, and reports whether it did. Why: the record points at a result held in the Editor's +// memory, which went away with it, so the next compile would only spend its attach probes finding nothing. +func clearCompilePendingRecordAfterEditorExit(projectRoot string, waitErr error) bool { + var notRunning clierrors.UnityNotRunningError + if !errors.As(waitErr, ¬Running) { + return false + } + clearCompilePendingRecord(projectRoot) + return true +} + func waitForAttachedCompileCompletion( ctx context.Context, options compileCompletionOptions, @@ -243,6 +256,7 @@ func waitForAttachedCompileCompletion( logCompileStatusPollStart(options, startedAt, deadline) interim := newCompileWaitInterimState(compileWaitNow(deps)) + exitWatch := newCompileUnityExitWatch(options.connection, deps) ticker := time.NewTicker(options.pollInterval) defer ticker.Stop() @@ -277,6 +291,10 @@ func waitForAttachedCompileCompletion( } } logCompileStatusPollObservedIfChanged(options, startedAt, attempts, status, err, &lastObservationKey) + if exitErr := exitWatch.Observe(ctx, err); exitErr != nil { + logCompileWaitUnityExited(options, startedAt, attempts, lastStatus, lastErr) + return nil, attachWaitFailed, lastObservedCompileStatus(lastStatus, observedStatus), exitErr + } observeCompileWaitInterim(&interim, deps, status, err) select { diff --git a/cli/project-runner/internal/projectrunner/compile_fresh_recovery.go b/cli/project-runner/internal/projectrunner/compile_fresh_recovery.go index 359e48de3b..8a1226fda2 100644 --- a/cli/project-runner/internal/projectrunner/compile_fresh_recovery.go +++ b/cli/project-runner/internal/projectrunner/compile_fresh_recovery.go @@ -101,7 +101,7 @@ type compileRequestMissingTracker struct { // slot the first request still holds. func (tracker *compileRequestMissingTracker) observe(status compileStatusResponse, err error) bool { if err != nil { - if isServerGoneError(err) { + if clierrors.IsUnityServerGoneError(err) { tracker.serverRestartSeen = true } tracker.missingStreak = 0 @@ -118,21 +118,6 @@ func (tracker *compileRequestMissingTracker) observe(status compileStatusRespons return tracker.serverRestartSeen && tracker.missingStreak >= compileAttachMissingResultStreak } -// isServerGoneError reports whether a failed status query shows that Unity's server went away: the -// connection dropped mid-query, or nobody was listening. -func isServerGoneError(err error) bool { - if clierrors.IsTransportDisconnectError(err) { - return true - } - var connectionErr *unityipc.ConnectionAttemptError - if !errors.As(err, &connectionErr) { - return false - } - // Why not a connect timeout or a denied connect: a Windows named pipe times out while all of its - // instances are busy, and a sandbox can deny the connect, both while the server is alive. - return !clierrors.IsFinalResponseTimeoutError(err) && !clierrors.IsPermanentConnectError(err) -} - // canResendCompile reports whether at least compileResendMinimumBudget is left before resendBefore. // A zero resendBefore never allows a resend. func canResendCompile(resendBefore time.Time) bool { diff --git a/cli/project-runner/internal/projectrunner/compile_fresh_recovery_test.go b/cli/project-runner/internal/projectrunner/compile_fresh_recovery_test.go index 4a9129b410..1b596ea6e6 100644 --- a/cli/project-runner/internal/projectrunner/compile_fresh_recovery_test.go +++ b/cli/project-runner/internal/projectrunner/compile_fresh_recovery_test.go @@ -641,7 +641,7 @@ func TestStatusQueryAgainstNoListenerCountsAsTheServerBeingGone(t *testing.T) { if errors.Is(err, os.ErrPermission) { t.Skipf("the environment denied the connection itself, so it cannot show a missing listener: %v", err) } - if !isServerGoneError(err) { + if !clierrors.IsUnityServerGoneError(err) { t.Fatalf("a status query with no listener must count as the server being gone: %v", err) } } diff --git a/cli/project-runner/internal/projectrunner/compile_wait.go b/cli/project-runner/internal/projectrunner/compile_wait.go index 3c2a43fc5a..f8a8fb7cd4 100644 --- a/cli/project-runner/internal/projectrunner/compile_wait.go +++ b/cli/project-runner/internal/projectrunner/compile_wait.go @@ -195,6 +195,7 @@ func waitForCompileCompletionWithDeps( logCompileStatusPollStart(options, startedAt, deadline) interim := newCompileWaitInterimState(compileWaitNow(deps)) missing := compileRequestMissingTracker{serverRestartSeen: options.serverRestartSeen} + exitWatch := newCompileUnityExitWatch(options.connection, deps) ticker := time.NewTicker(options.pollInterval) defer ticker.Stop() @@ -224,6 +225,10 @@ func waitForCompileCompletionWithDeps( return nil, false, lastObservedCompileStatus(lastStatus, observedStatus), errCompileRequestMissing } logCompileStatusPollObservedIfChanged(options, startedAt, attempts, status, err, &lastObservationKey) + if exitErr := exitWatch.Observe(ctx, err); exitErr != nil { + logCompileWaitUnityExited(options, startedAt, attempts, lastStatus, lastErr) + return nil, false, lastObservedCompileStatus(lastStatus, observedStatus), exitErr + } observeCompileWaitInterim(&interim, deps, status, err) select { diff --git a/cli/project-runner/internal/projectrunner/compile_wait_deps.go b/cli/project-runner/internal/projectrunner/compile_wait_deps.go index 959b6807a3..9e5d8c6cd0 100644 --- a/cli/project-runner/internal/projectrunner/compile_wait_deps.go +++ b/cli/project-runner/internal/projectrunner/compile_wait_deps.go @@ -4,7 +4,9 @@ import ( "context" "time" + "github.com/hatayama/unity-cli-loop/common/clicore" "github.com/hatayama/unity-cli-loop/common/unityipc" + "github.com/hatayama/unity-cli-loop/common/unityprocess" ) type compileSendFunc func( @@ -28,6 +30,19 @@ type compileWaitDeps struct { // Zero keeps compileWaitPollInterval for a fresh compile's status wait. Tests shorten it so // they do not wait 1s between status queries. freshWaitPollInterval time.Duration + // findRunningUnityProcess is the Editor process lookup a status wait uses to notice that Unity + // exited. + findRunningUnityProcess func(context.Context, string) (*unityprocess.UnityProcess, error) +} + +// newCompileUnityExitWatch creates the exit watch for one compile status wait, on the wait's clock. +func newCompileUnityExitWatch(connection unityipc.Connection, deps compileWaitDeps) *clicore.UnityExitWatch { + return clicore.NewUnityExitWatch( + connection.ProjectRoot, + connection.Endpoint.Address, + deps.findRunningUnityProcess, + func() time.Time { return compileWaitNow(deps) }, + ) } func freshWaitPollIntervalFor(deps compileWaitDeps) time.Duration { @@ -46,9 +61,10 @@ func compileSendOrDefault(deps compileWaitDeps) compileSendFunc { func defaultCompileWaitDeps() compileWaitDeps { return compileWaitDeps{ - queryCompileStatus: queryCompileStatusFromUnity, - attachProbeTimeout: compileAttachProbeTimeout, - attachProbeInterval: compileAttachProbeInterval, - attachWaitPollInterval: compileWaitPollInterval, + queryCompileStatus: queryCompileStatusFromUnity, + attachProbeTimeout: compileAttachProbeTimeout, + attachProbeInterval: compileAttachProbeInterval, + attachWaitPollInterval: compileWaitPollInterval, + findRunningUnityProcess: unityProcessLookup, } } diff --git a/cli/project-runner/internal/projectrunner/compile_wait_log.go b/cli/project-runner/internal/projectrunner/compile_wait_log.go index 2d3db34f10..86abe66eed 100644 --- a/cli/project-runner/internal/projectrunner/compile_wait_log.go +++ b/cli/project-runner/internal/projectrunner/compile_wait_log.go @@ -259,6 +259,30 @@ func logCompileWaitCancelled( }) } +// logCompileWaitUnityExited closes the poll's start entry when the wait ended because the Editor exited, +// so the log never shows a status poll that started and never finished. +func logCompileWaitUnityExited( + options compileCompletionOptions, + startedAt time.Time, + attempts int, + lastStatus compileStatusResponse, + lastErr error, +) { + writeCompileVibeLog(options.connection.ProjectRoot, func() vibelog.CLIVibeLogEntry { + return vibelog.CLIVibeLogEntry{ + Level: "WARNING", + Operation: "cli_compile_status_poll_unity_exited", + Message: "Stopped polling Unity compile status because the Unity Editor process exited.", + Context: compileWaitLogContext(options, startedAt, map[string]any{ + "poll_attempts": attempts, + "last_status": compileStatusLogContext(lastStatus), + "last_transport_error": clicore.ErrorMessage(lastErr), + }), + CorrelationID: options.requestID, + } + }) +} + func compileWaitLogContext( options compileCompletionOptions, startedAt time.Time, diff --git a/cli/project-runner/internal/projectrunner/compile_wait_test.go b/cli/project-runner/internal/projectrunner/compile_wait_test.go index bf44801812..5c90ceccf0 100644 --- a/cli/project-runner/internal/projectrunner/compile_wait_test.go +++ b/cli/project-runner/internal/projectrunner/compile_wait_test.go @@ -1055,6 +1055,8 @@ func compileWaitTestDeps( attachProbeTimeout: 40 * time.Millisecond, attachProbeInterval: 5 * time.Millisecond, attachWaitPollInterval: 5 * time.Millisecond, + // The scripted status answers stand in for a running Editor, whose server a reload takes down. + findRunningUnityProcess: runningUnityProcessLookup, } } diff --git a/cli/project-runner/internal/projectrunner/compile_wait_unity_exit_test.go b/cli/project-runner/internal/projectrunner/compile_wait_unity_exit_test.go new file mode 100644 index 0000000000..97ec240b25 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/compile_wait_unity_exit_test.go @@ -0,0 +1,163 @@ +package projectrunner + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "syscall" + "testing" + "time" + + clierrors "github.com/hatayama/unity-cli-loop/common/errors" + "github.com/hatayama/unity-cli-loop/common/unityipc" +) + +// exitedEditorCompileDeps answers every status query as a server nobody listens on, finds no Editor +// process, and moves the wait's clock a second on every reading so lookups are spaced without sleeping. +func exitedEditorCompileDeps() compileWaitDeps { + clock := time.Unix(1_700_000_000, 0) + deps := compileWaitTestDeps(func(context.Context, unityipc.Connection, string) (compileStatusResponse, error) { + return compileStatusResponse{}, &unityipc.ConnectionAttemptError{Cause: syscall.ECONNREFUSED} + }) + deps.findRunningUnityProcess = noUnityProcessLookup + deps.now = func() time.Time { + clock = clock.Add(time.Second) + return clock + } + return deps +} + +func assertUnityExitedDuringCommand(t *testing.T, err error) { + t.Helper() + var notRunning clierrors.UnityNotRunningError + if !errors.As(err, ¬Running) { + t.Fatalf("err = %v, want UnityNotRunningError", err) + } + if !notRunning.ExitedDuringCommand { + t.Fatalf("expected the exit to be reported as during the command, got %#v", notRunning) + } +} + +// Verifies a fresh compile's status wait ends with UnityNotRunningError once the Editor is confirmed +// gone, instead of polling a server that cannot come back for the whole compile timeout. +func TestWaitForCompileCompletionStopsOnceUnityHasExited(t *testing.T) { + _, completed, _, err := waitForCompileCompletionWithDeps(context.Background(), compileCompletionOptions{ + connection: compileWaitTestConnection(t), + requestID: "compile_unity_exited", + // Long enough that only the exit watch, never the timeout, can end the wait first. + timeout: 5 * time.Second, + pollInterval: time.Millisecond, + }, exitedEditorCompileDeps()) + + if completed { + t.Fatal("a compile whose Editor exited cannot complete") + } + assertUnityExitedDuringCommand(t, err) +} + +// Verifies a compile status wait that stops on an exited Editor closes its start entry in the CLI vibe +// log, so the log does not show a poll that started and never ended. +func TestWaitForCompileCompletionLogsTheEditorExit(t *testing.T) { + enableCliVibeLog(t) + connection := compileWaitTestConnection(t) + + _, _, _, _ = waitForCompileCompletionWithDeps(context.Background(), compileCompletionOptions{ + connection: connection, + requestID: "compile_unity_exited_log", + timeout: 5 * time.Second, + pollInterval: time.Millisecond, + }, exitedEditorCompileDeps()) + + logContent := readOnlyCliVibeLog(t, connection.ProjectRoot) + if entries := cliVibeEntriesForOperation(t, logContent, "cli_compile_status_poll_unity_exited"); len(entries) != 1 { + t.Fatalf("unity-exited entries = %d, want 1\n%s", len(entries), logContent) + } +} + +// Verifies the attach wait stops the same way when the Editor running the earlier compile is gone. +func TestWaitForAttachedCompileCompletionStopsOnceUnityHasExited(t *testing.T) { + _, outcome, _, err := waitForAttachedCompileCompletion(context.Background(), compileCompletionOptions{ + connection: compileWaitTestConnection(t), + requestID: "compile_attach_unity_exited", + // Long enough that only the exit watch, never the timeout, can end the wait first. + timeout: 5 * time.Second, + pollInterval: time.Millisecond, + }, exitedEditorCompileDeps()) + + if outcome != attachWaitFailed { + t.Fatalf("outcome = %v, want attachWaitFailed", outcome) + } + assertUnityExitedDuringCommand(t, err) +} + +// Verifies uloop compile prints the not-running envelope when the Editor exits during the domain reload +// wait, so the caller is told to launch Unity rather than to wait. +func TestRunCompileReportsUnityNotRunningWhenTheEditorExitsDuringTheWait(t *testing.T) { + deps := exitedEditorCompileDeps() + deps.freshWaitPollInterval = time.Millisecond + deps.sendCompile = func( + context.Context, + unityipc.Connection, + string, + map[string]any, + unityipc.ProgressFunc, + time.Duration, + ) (unityipc.UnitySendOutcome, error) { + return unityipc.UnitySendOutcome{RequestDispatched: true, RequestAccepted: true}, errors.New("read: EOF") + } + var stdout, stderr bytes.Buffer + + code := runCompileWithDomainReloadWaitWithDeps( + context.Background(), + compileWaitTestConnection(t), + map[string]any{}, + &stdout, + &stderr, + deps, + ) + + if code != 1 || stdout.Len() != 0 { + t.Fatalf("code = %d, stdout = %q, want 1 and nothing", code, stdout.String()) + } + jsonStart := bytes.IndexByte(stderr.Bytes(), '{') + if jsonStart < 0 { + t.Fatalf("stderr has no JSON envelope: %s", stderr.String()) + } + var envelope clierrors.CLIErrorEnvelope + if err := json.Unmarshal(stderr.Bytes()[jsonStart:], &envelope); err != nil { + t.Fatalf("decoding stderr failed: %v\n%s", err, stderr.String()) + } + if envelope.Error.ErrorCode != clierrors.ErrorCodeUnityNotRunning { + t.Fatalf("ErrorCode = %q, want %q\n%s", envelope.Error.ErrorCode, clierrors.ErrorCodeUnityNotRunning, stderr.String()) + } +} + +// Verifies an attach wait that ended because the Editor exited drops the pending record, whose result +// lived in the exited Editor, while any other failure keeps it for the next compile to reattach to. +func TestClearCompilePendingRecordAfterEditorExit(t *testing.T) { + cases := []struct { + name string + waitErr error + wantCleared bool + }{ + {name: "Editor exited", waitErr: clierrors.UnityNotRunningError{ExitedDuringCommand: true}, wantCleared: true}, + {name: "other failure", waitErr: errors.New("status query failed"), wantCleared: false}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + projectRoot := t.TempDir() + record := compilePendingRecord{RequestID: "compile_pending", TimedOutAtUtc: time.Now().UTC()} + if err := writeCompilePendingRecord(projectRoot, record); err != nil { + t.Fatalf("writing the pending record failed: %v", err) + } + + cleared := clearCompilePendingRecordAfterEditorExit(projectRoot, testCase.waitErr) + + _, kept := readCompilePendingRecord(projectRoot) + if cleared != testCase.wantCleared || kept == testCase.wantCleared { + t.Fatalf("cleared = %v, record kept = %v, want cleared = %v", cleared, kept, testCase.wantCleared) + } + }) + } +} diff --git a/cli/project-runner/internal/projectrunner/connection_retry.go b/cli/project-runner/internal/projectrunner/connection_retry.go index 3a523ee896..1bf0c7a5ee 100644 --- a/cli/project-runner/internal/projectrunner/connection_retry.go +++ b/cli/project-runner/internal/projectrunner/connection_retry.go @@ -33,7 +33,7 @@ type connectionRetryDeps struct { func defaultConnectionRetryDeps() connectionRetryDeps { return connectionRetryDeps{ - findRunningUnityProcess: unityprocess.FindRunningUnityProcess, + findRunningUnityProcess: unityProcessLookup, focusUnityProcess: unityprocess.FocusUnityProcessWithRestore, retryTimeout: serverConnectionRetryDefaultTimeout, retryPoll: serverConnectionRetryDefaultPoll, @@ -277,7 +277,7 @@ func sendWithTransientConnectionRetryWithDeps( continue } if !shouldRetryUndispatchedConnection(err, outcome) { - return finishNonRetryableConnectionAttempt( + finalOutcome, finalErr := finishNonRetryableConnectionAttempt( ctx, connection, sendAttempt{ @@ -291,6 +291,7 @@ func sendWithTransientConnectionRetryWithDeps( responseTimeout, focusController, ) + return finalOutcome, reportUnityExitDuringRequest(ctx, connection, finalOutcome, finalErr, deps) } // A caller that cancelled the command gets that cancellation back, as everywhere else that diff --git a/cli/project-runner/internal/projectrunner/connection_retry_flow.go b/cli/project-runner/internal/projectrunner/connection_retry_flow.go index ce248030fa..ac1cbc2161 100644 --- a/cli/project-runner/internal/projectrunner/connection_retry_flow.go +++ b/cli/project-runner/internal/projectrunner/connection_retry_flow.go @@ -101,16 +101,40 @@ func finishNonRetryableConnectionAttempt( return currentAttempt.outcome, currentAttempt.err } +// reportUnityExitDuringRequest turns a connection that dropped after the request reached Unity into +// UnityNotRunningError when a process lookup finds no Editor, and returns err unchanged otherwise. +// Why one lookup and no wait: an Editor still listed while it tears down keeps the disconnect, and the +// caller's next command then fails before dispatch, where the same lookup reports the exit. +func reportUnityExitDuringRequest( + ctx context.Context, + connection unityipc.Connection, + outcome unityipc.UnitySendOutcome, + err error, + deps connectionRetryDeps, +) error { + if ctx.Err() != nil || !outcome.RequestDispatched || !clierrors.IsTransportDisconnectError(err) { + return err + } + runningProcess, processErr := deps.findRunningUnityProcess(ctx, connection.ProjectRoot) + if processErr != nil { + logUnityProcessProbeFailure(connection, err, processErr) + return err + } + if runningProcess != nil { + return err + } + return clicore.NewUnityNotRunningError(connection.ProjectRoot, connection.Endpoint.Address, err, true) +} + type sendAttempt struct { outcome unityipc.UnitySendOutcome err error } // finishUndispatchedRetryProbe decides whether the retry loop keeps waiting after a dial that -// never reached Unity. The process probe's only job here is to promote the diagnosis from "not -// reachable" to "running but not responding", so a probe that failed cannot promote anything: it -// observed no process, and claiming one would be an assertion this code has no evidence for. -// Both probe outcomes therefore report the dial error the caller can act on, and the probe +// never reached Unity. A probe that found the process keeps the loop waiting for its server, and a +// probe that answered without one reports that no Editor is running. A probe that failed observed +// nothing, so it can claim neither: it reports the dial error the caller can act on, and the probe // failure goes to the CLI vibe log instead of replacing that error. func finishUndispatchedRetryProbe( retryContext context.Context, @@ -126,6 +150,14 @@ func finishUndispatchedRetryProbe( if runningProcess != nil { return false, currentAttempt.outcome, nil } + if processErr == nil { + return true, currentAttempt.outcome, clicore.NewUnityNotRunningError( + connection.ProjectRoot, + connection.Endpoint.Address, + currentAttempt.err, + false, + ) + } // A busy response seen during the window proves a server answered moments ago, so it is a // truer diagnosis than a final dial cut short by the expiring retry context. if retryContext.Err() != nil && isUnityServerBusyRPCError(lastAttempt.err) { @@ -142,7 +174,7 @@ func logUnityProcessProbeFailure(connection unityipc.Connection, dialErr error, _ = vibelog.WriteCLIVibeLog(connection.ProjectRoot, vibelog.CLIVibeLogEntry{ Level: "WARNING", Operation: "cli_unity_process_probe_failed", - Message: "Could not determine whether Unity is running while recovering an unreachable request.", + Message: "Could not determine whether Unity is running after a request failed to reach it or lost its connection.", Context: map[string]any{ "endpoint": connection.Endpoint.Address, "dial_cause": clicore.ErrorMessage(dialErr), diff --git a/cli/project-runner/internal/projectrunner/connection_retry_flow_test.go b/cli/project-runner/internal/projectrunner/connection_retry_flow_test.go index cfb2b7d3e7..e3d780f158 100644 --- a/cli/project-runner/internal/projectrunner/connection_retry_flow_test.go +++ b/cli/project-runner/internal/projectrunner/connection_retry_flow_test.go @@ -146,6 +146,47 @@ func TestFinishUndispatchedRetryProbeContinuesWhenUnityIsRunning(t *testing.T) { } } +// Verifies a probe that answered without a process ends the retry loop with UnityNotRunningError +// instead of the dial error, whose guidance tells the caller to wait for an Editor that is not there. +// Why a busy answer earlier in the window does not win: the server that answered it is gone now. +func TestFinishUndispatchedRetryProbeReportsUnityNotRunningWhenNoProcessIsFound(t *testing.T) { + busyAttempt := sendAttempt{ + err: &unityipc.RPCError{ + Code: -32603, + Message: "Unity is busy running 'compile'.", + Data: []byte(`{"type":"server_busy"}`), + }, + } + connection := unityipc.Connection{ + ProjectRoot: t.TempDir(), + Endpoint: unityipc.Endpoint{Address: "/tmp/uloop/sample.sock"}, + } + currentAttempt := refusedDialAttempt() + + finished, _, err := finishUndispatchedRetryProbe( + expiredRetryContext(), + connection, + currentAttempt, + nil, + nil, + busyAttempt, + ) + + if !finished { + t.Fatal("expected the retry loop to finish when no Unity process is running") + } + var notRunning clierrors.UnityNotRunningError + if !errors.As(err, ¬Running) { + t.Fatalf("expected UnityNotRunningError, got %v", err) + } + if notRunning.ExitedDuringCommand { + t.Fatal("an undispatched request never reached Unity, so it cannot have exited during it") + } + if notRunning.Cause != currentAttempt.err || notRunning.Endpoint != connection.Endpoint.Address { + t.Fatalf("expected the dial error and endpoint to be kept, got %#v", notRunning) + } +} + // Verifies the swallowed probe failure is still recorded: dropping the diagnosis from the error // must not drop it from the diagnostics too. func TestFinishUndispatchedRetryProbeRecordsTheProbeFailureInTheVibeLog(t *testing.T) { @@ -510,3 +551,87 @@ func TestFinishUnityAliveRetryWaitWhenRetryContextEnds(t *testing.T) { t.Fatalf("finished=%v err=%v, want UnityServerNotRespondingError", finished, err) } } + +// Verifies a connection that dropped after the request reached Unity becomes UnityNotRunningError only +// when a lookup that answered finds no Editor, and stays the disconnect in every other case, so a domain +// reload still reaches the callers that wait it out. +func TestReportUnityExitDuringRequest(t *testing.T) { + dispatched := unityipc.UnitySendOutcome{RequestDispatched: true} + accepted := unityipc.UnitySendOutcome{RequestDispatched: true, RequestAccepted: true} + failedLookup := func(context.Context, string) (*unityprocess.UnityProcess, error) { + return nil, errors.New("sysctl kern.proc.all: operation not permitted") + } + cases := []struct { + name string + ctx context.Context + outcome unityipc.UnitySendOutcome + err error + lookup func(context.Context, string) (*unityprocess.UnityProcess, error) + wantExitErr bool + }{ + {name: "dropped before the accept, no Editor", outcome: dispatched, err: io.EOF, lookup: noUnityProcessLookup, wantExitErr: true}, + {name: "dropped after the accept, no Editor", outcome: accepted, err: io.ErrUnexpectedEOF, lookup: noUnityProcessLookup, wantExitErr: true}, + {name: "dropped, Editor running", outcome: accepted, err: io.EOF, lookup: runningUnityProcessLookup}, + {name: "dropped, lookup failed", outcome: accepted, err: io.EOF, lookup: failedLookup}, + {name: "dropped before dispatch", outcome: unityipc.UnitySendOutcome{}, err: io.EOF, lookup: noUnityProcessLookup}, + {name: "caller cancelled", ctx: cancelledContext(), outcome: accepted, err: io.EOF, lookup: noUnityProcessLookup}, + {name: "not a disconnect", outcome: accepted, err: os.ErrDeadlineExceeded, lookup: noUnityProcessLookup}, + } + for _, testCase := range cases { + t.Run(testCase.name, func(t *testing.T) { + ctx := testCase.ctx + if ctx == nil { + ctx = context.Background() + } + deps := defaultConnectionRetryDeps() + deps.findRunningUnityProcess = testCase.lookup + connection := unityipc.Connection{ + ProjectRoot: t.TempDir(), + Endpoint: unityipc.Endpoint{Address: "/tmp/uloop/sample.sock"}, + } + + err := reportUnityExitDuringRequest(ctx, connection, testCase.outcome, testCase.err, deps) + + var notRunning clierrors.UnityNotRunningError + if !testCase.wantExitErr { + if err != testCase.err { + t.Fatalf("err = %v, want the original error %v", err, testCase.err) + } + return + } + if !errors.As(err, ¬Running) { + t.Fatalf("err = %v, want UnityNotRunningError", err) + } + if !notRunning.ExitedDuringCommand || notRunning.Cause != testCase.err { + t.Fatalf("expected an exit during the command caused by the disconnect, got %#v", notRunning) + } + }) + } +} + +// Verifies a process lookup that failed after a dispatched disconnect leaves a trace in the CLI vibe log, +// as the same failure before dispatch does, since the command output keeps only the disconnect. +func TestReportUnityExitDuringRequestLogsAFailedLookup(t *testing.T) { + enableCliVibeLog(t) + deps := defaultConnectionRetryDeps() + deps.findRunningUnityProcess = func(context.Context, string) (*unityprocess.UnityProcess, error) { + return nil, errors.New("sysctl kern.proc.all: operation not permitted") + } + connection := unityipc.Connection{ + ProjectRoot: t.TempDir(), + Endpoint: unityipc.Endpoint{Address: "/tmp/uloop/sample.sock"}, + } + + _ = reportUnityExitDuringRequest( + context.Background(), + connection, + unityipc.UnitySendOutcome{RequestDispatched: true, RequestAccepted: true}, + io.EOF, + deps, + ) + + logContent := readOnlyCliVibeLog(t, connection.ProjectRoot) + if entries := cliVibeEntriesForOperation(t, logContent, "cli_unity_process_probe_failed"); len(entries) != 1 { + t.Fatalf("probe failure entries = %d, want 1\n%s", len(entries), logContent) + } +} diff --git a/cli/project-runner/internal/projectrunner/connection_retry_test.go b/cli/project-runner/internal/projectrunner/connection_retry_test.go index f6d20556e7..23397e7d9c 100644 --- a/cli/project-runner/internal/projectrunner/connection_retry_test.go +++ b/cli/project-runner/internal/projectrunner/connection_retry_test.go @@ -1685,9 +1685,8 @@ func TestSendWithTransientConnectionRetrySurfacesADroppedConnectionAfterBusy(t * deps := defaultConnectionRetryDeps() deps.retryTimeout = 150 * time.Millisecond deps.retryPoll = 5 * time.Millisecond - deps.findRunningUnityProcess = func(context.Context, string) (*clicore.UnityProcess, error) { - return nil, nil - } + // The Editor is still running: the connection dropped for a domain reload, not an exit. + deps.findRunningUnityProcess = runningUnityProcessLookup connection := busyFirstServerConnection(t, func(conn net.Conn) { accepted := `{"jsonrpc":"2.0","result":{"accepted":true},"uloop":{"phase":"accepted"},"id":1}` _ = unityipc.Write(conn, []byte(accepted)) diff --git a/cli/project-runner/internal/projectrunner/control_play_mode_wait.go b/cli/project-runner/internal/projectrunner/control_play_mode_wait.go index 31f15da437..8ba9cd2dca 100644 --- a/cli/project-runner/internal/projectrunner/control_play_mode_wait.go +++ b/cli/project-runner/internal/projectrunner/control_play_mode_wait.go @@ -120,7 +120,7 @@ func runControlPlayModeWithStateWait( } spinner.Update("Waiting for play mode state...") - response, completed, waitErr := waitForControlPlayModeState(ctx, connection, action, timeout) + response, completed, waitErr := waitForControlPlayModeState(ctx, connection, action, timeout, newUnityExitWatch(connection)) spinner.Stop() if waitErr != nil { writeDebugTiming(stderr, controlPlayModeCommandName, time.Since(startedAt), outcome) @@ -168,6 +168,7 @@ func waitForControlPlayModeState( connection unityipc.Connection, action string, timeout time.Duration, + exitWatch *clicore.UnityExitWatch, ) (controlPlayModeResponse, bool, error) { waitContext, cancel := context.WithTimeout(ctx, timeout) defer cancel() @@ -179,6 +180,11 @@ func waitForControlPlayModeState( defer ticker.Stop() for { response, err := requestControlPlayModeStatus(waitContext, connection, action) + // Why every poll, an answer included: an answer restarts the watch's count of lookups that + // found no Editor. + if exitErr := exitWatch.Observe(waitContext, err); exitErr != nil { + return lastResponse, false, exitErr + } if err == nil { lastResponse = response hasResponse = true diff --git a/cli/project-runner/internal/projectrunner/control_play_mode_wait_test.go b/cli/project-runner/internal/projectrunner/control_play_mode_wait_test.go index 85700b080d..6a6eda3d3c 100644 --- a/cli/project-runner/internal/projectrunner/control_play_mode_wait_test.go +++ b/cli/project-runner/internal/projectrunner/control_play_mode_wait_test.go @@ -13,6 +13,7 @@ import ( "testing" "time" + "github.com/hatayama/unity-cli-loop/common/clicore" clierrors "github.com/hatayama/unity-cli-loop/common/errors" "github.com/hatayama/unity-cli-loop/common/unityipc" @@ -887,6 +888,7 @@ func serveDroppedConnections(listener net.Listener, onSecondAccept func()) { // Verifies a disconnect during the action request moves on to the state wait, and the caller's // context ending during that wait is reported as a failure. func TestRunControlPlayModeWithStateWaitReportsContextEndDuringWait(t *testing.T) { + useRunningUnityProcess(t) originalPoll := controlPlayModeStatePoll controlPlayModeStatePoll = time.Millisecond t.Cleanup(func() { controlPlayModeStatePoll = originalPoll }) @@ -924,11 +926,13 @@ func TestWaitForControlPlayModeStateTimesOutWithLastError(t *testing.T) { controlPlayModeStatePoll = time.Millisecond t.Cleanup(func() { controlPlayModeStatePoll = originalPoll }) + connection := unreachableConnection(t.TempDir()) _, completed, err := waitForControlPlayModeState( context.Background(), - unreachableConnection(t.TempDir()), + connection, "Play", 20*time.Millisecond, + clicore.NewUnityExitWatch(connection.ProjectRoot, "", runningUnityProcessLookup, nil), ) if completed || err == nil { diff --git a/cli/project-runner/internal/projectrunner/hot_reload_busy_wait.go b/cli/project-runner/internal/projectrunner/hot_reload_busy_wait.go index 5a41e201cf..92d221e6c6 100644 --- a/cli/project-runner/internal/projectrunner/hot_reload_busy_wait.go +++ b/cli/project-runner/internal/projectrunner/hot_reload_busy_wait.go @@ -67,7 +67,7 @@ func sendHotReloadWaitingForBusyEditor( logHotReloadBusyWaitDecided(connection, first.correlationID, err) // A plain line for the same reason as the settle wait's: the spinner's Stop erases its own line. _, _ = fmt.Fprintf(stderr, hotReloadBusyWaitingLineFormat+"\n", runningToolName) - wait, waitErr := waitForBusyEditor(ctx, connection, params, stderr, deps, hotReloadEditorReadyWaitDefaults) + wait, waitErr := waitForBusyEditor(ctx, connection, params, stderr, deps, hotReloadEditorReadyWaitDefaults, newUnityExitWatch(connection)) if waitErr != nil { writeHotReloadClassifiedError(stderr, connection, waitErr) logHotReloadBusyWaitComplete(connection, first.correlationID, wait, nil) @@ -128,6 +128,7 @@ func waitForBusyEditor( stderr io.Writer, deps connectionRetryDeps, options hotReloadEditorReadyWaitOptions, + exitWatch *clicore.UnityExitWatch, ) (hotReloadBusyWait, error) { startedAt := time.Now() deadline := startedAt.Add(options.budget) @@ -135,7 +136,12 @@ func waitForBusyEditor( lastResend := startedAt wait := hotReloadBusyWait{} for { - status, answered := probeHotReloadEditorStatus(ctx, connection, options.probeTimeout) + status, probeErr := probeHotReloadEditorStatus(ctx, connection, options.probeTimeout) + answered := probeErr == nil + if exitErr := exitWatch.Observe(ctx, probeErr); exitErr != nil { + wait.waited = time.Since(startedAt) + return wait, exitErr + } if answered && classifyEditorState(status) == statusStateReady { wait.waited, wait.ready = time.Since(startedAt), true return wait, nil diff --git a/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry.go b/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry.go index cf96c6844b..900ccd8ece 100644 --- a/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry.go +++ b/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry.go @@ -86,7 +86,7 @@ func retryHotReloadAfterEditorReady( // A plain line rather than the tool spinner: the spinner is off on a non-TTY stderr, and on a // TTY its Stop erases its own line, so the reader would not see why the command paused. _, _ = fmt.Fprintln(stderr, hotReloadEditorReadyWaitingLine) - waited, ready, err := waitForEditorReady(ctx, connection, hotReloadEditorReadyWaitDefaults) + waited, ready, err := waitForEditorReady(ctx, connection, hotReloadEditorReadyWaitDefaults, newUnityExitWatch(connection)) if err != nil { clicore.WriteJSON(stdout, first.result) writeHotReloadClassifiedError(stderr, connection, err) @@ -200,20 +200,25 @@ func hotReloadRetryParams(params map[string]any, selectedFiles []string) map[str } // waitForEditorReady polls the Editor status until the Editor answers and is ready, the budget -// runs out, or ctx is cancelled. -// Why every probe error counts as "not yet": the domain reload that follows Unity's compile takes -// the server down and brings it back. +// runs out, the Editor exits, or ctx is cancelled. +// Why every other probe error counts as "not yet": the domain reload that follows Unity's compile +// takes the server down and brings it back. func waitForEditorReady( ctx context.Context, connection unityipc.Connection, options hotReloadEditorReadyWaitOptions, + exitWatch *clicore.UnityExitWatch, ) (time.Duration, bool, error) { startedAt := time.Now() deadline := startedAt.Add(options.budget) for { - if probeEditorReady(ctx, connection, options.probeTimeout) { + status, probeErr := probeHotReloadEditorStatus(ctx, connection, options.probeTimeout) + if probeErr == nil && classifyEditorState(status) == statusStateReady { return time.Since(startedAt), true, nil } + if exitErr := exitWatch.Observe(ctx, probeErr); exitErr != nil { + return time.Since(startedAt), false, exitErr + } if !time.Now().Before(deadline) { return time.Since(startedAt), false, nil } @@ -225,18 +230,13 @@ func waitForEditorReady( } } -func probeEditorReady(ctx context.Context, connection unityipc.Connection, probeTimeout time.Duration) bool { - status, answered := probeHotReloadEditorStatus(ctx, connection, probeTimeout) - return answered && classifyEditorState(status) == statusStateReady -} - -// probeHotReloadEditorStatus asks the Editor for its status once; answered is false when it did not answer -// or the answer was not readable. +// probeHotReloadEditorStatus asks the Editor for its status once. The error is non-nil when the Editor +// did not answer or the answer was not readable. func probeHotReloadEditorStatus( ctx context.Context, connection unityipc.Connection, probeTimeout time.Duration, -) (editorStatusResponse, bool) { +) (editorStatusResponse, error) { probeCtx, cancel := context.WithTimeout(ctx, probeTimeout) defer cancel() raw, err := unityipc.NewClient(connection, clicontract.ProjectRunnerVersion()).Send( @@ -244,13 +244,13 @@ func probeHotReloadEditorStatus( editorStatusBridgeCommandName, map[string]any{}) if err != nil { - return editorStatusResponse{}, false + return editorStatusResponse{}, err } var response editorStatusResponse if err := json.Unmarshal(raw, &response); err != nil { - return editorStatusResponse{}, false + return editorStatusResponse{}, err } - return response, true + return response, nil } // composeHotReloadEditorReadyNote appends sentence to the note an earlier wait in this command left diff --git a/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry_test.go b/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry_test.go index a461ee5c88..19b653bee6 100644 --- a/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry_test.go +++ b/cli/project-runner/internal/projectrunner/hot_reload_editor_ready_retry_test.go @@ -175,6 +175,7 @@ func runScriptedHotReload( compileResult compileExecutionResult, ) editorReadyRetryRun { t.Helper() + useRunningUnityProcess(t) listener := newLoopbackIpcListener(t) requests := make(chan scriptedIPCRequest, len(steps)+1) serverErr := make(chan error, len(steps)+1) diff --git a/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go b/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go index f5db9ca269..884c5c1e19 100644 --- a/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go +++ b/cli/project-runner/internal/projectrunner/pause_point_wait_poll.go @@ -5,6 +5,7 @@ import ( "fmt" "time" + "github.com/hatayama/unity-cli-loop/common/clicore" clierrors "github.com/hatayama/unity-cli-loop/common/errors" "github.com/hatayama/unity-cli-loop/common/unityipc" ) @@ -265,6 +266,7 @@ type pausePointWaitLoopState struct { baselineSequence int hasBaseline bool baselineDecided bool + exitWatch *clicore.UnityExitWatch } func waitForPausePointStatus( @@ -284,13 +286,14 @@ func waitForPausePointStatus( baselineSequence: baselineSequence, hasBaseline: hasBaseline, baselineDecided: baselineDecided, + exitWatch: newUnityExitWatch(connection), } triggerDone := triggerHandle.doneChannel() ticker := time.NewTicker(pausePointStatusPoll) defer ticker.Stop() for { response, err := queryPausePointStatus(waitContext, connection, options.id) - waitState, done, pollErr := applyPausePointWaitPoll(&state, response, err, options.markerJustEnabled) + waitState, done, pollErr := applyPausePointWaitPoll(waitContext, &state, response, err, options.markerJustEnabled) if done { return state.lastResponse, waitState, state.triggerResult, state.hasBaseline, pollErr } @@ -315,14 +318,18 @@ func waitForPausePointStatus( } // applyPausePointWaitPoll records one status query. Why abort on a permanent -// connect error: every poll dials again, so a refused connect would otherwise -// burn the whole --timeout before the refusal is reported. +// connect error or an exited Editor: every poll dials again, so either would +// otherwise burn the whole --timeout before it is reported. func applyPausePointWaitPoll( + ctx context.Context, state *pausePointWaitLoopState, response pausePointStatusResponse, err error, markerJustEnabled bool, ) (pausePointWaitState, bool, error) { + if exitErr := state.exitWatch.Observe(ctx, err); exitErr != nil { + return "", true, exitErr + } if err != nil { if clierrors.IsPermanentConnectError(err) { return "", true, err diff --git a/cli/project-runner/internal/projectrunner/status_command.go b/cli/project-runner/internal/projectrunner/status_command.go index b819011be4..014b548713 100644 --- a/cli/project-runner/internal/projectrunner/status_command.go +++ b/cli/project-runner/internal/projectrunner/status_command.go @@ -14,6 +14,7 @@ import ( "github.com/hatayama/unity-cli-loop/common/clicontract" "github.com/hatayama/unity-cli-loop/common/clicore" "github.com/hatayama/unity-cli-loop/common/unityipc" + "github.com/hatayama/unity-cli-loop/common/unityprocess" ) const editorStatusBridgeCommandName = "get-editor-status" @@ -49,7 +50,7 @@ func defaultStatusCommandDeps() statusCommandDeps { probeTimeout: 5 * time.Second, reprobeDelay: 1 * time.Second, isUnityProcessRunning: func(ctx context.Context, projectRoot string) (bool, error) { - process, err := clicore.FindRunningUnityProcess(ctx, projectRoot) + process, err := unityProcessLookup(ctx, projectRoot) return process != nil, err }, } @@ -249,6 +250,10 @@ func reportFromUnreachableServer( var report statusReport running, lookupErr := deps.isUnityProcessRunning(ctx, projectRoot) switch { + case errors.Is(lookupErr, unityprocess.ErrUnidentifiedUnityEditor): + report = newStatusReport(statusStateUnreachable, + "The uloop server is not accepting connections, and a running Unity Editor's command line "+ + "does not name its project, so uloop cannot tell whether Unity is running for this project.") case lookupErr != nil: report = newStatusReport(statusStateUnreachable, fmt.Sprintf( "The uloop server is not accepting connections, and the Unity process list could not be read: %s.", @@ -260,13 +265,22 @@ func reportFromUnreachableServer( "(uloop cannot run in Safe Mode).") report.UnityProcessRunning = &running default: - report = newStatusReport(statusStateNotRunning, "No Unity Editor is running for this project.") + report = notRunningStatusReport(projectRoot, probeErr) report.UnityProcessRunning = &running } report.ConnectionError = probeErr.Error() return report } +// notRunningStatusReport words NotRunning as uloop's not-running error does, so status and a failed +// command agree on whether the Editor shut down normally. +func notRunningStatusReport(projectRoot string, probeErr error) statusReport { + notRunning := clicore.NewUnityNotRunningError(projectRoot, "", probeErr, false) + report := newStatusReport(statusStateNotRunning, notRunning.ToCLIError(clierrors.ErrorContext{}).Message) + report.NextActions = append(notRunning.EditorLogNextActions(), report.NextActions...) + return report +} + func reportFromEditorResponse(response editorStatusResponse) statusReport { state := classifyEditorState(response) report := newStatusReport(state, editorStateMessage(state, response)) diff --git a/cli/project-runner/internal/projectrunner/status_command_test.go b/cli/project-runner/internal/projectrunner/status_command_test.go index fe3fbeb013..9f414ea47a 100644 --- a/cli/project-runner/internal/projectrunner/status_command_test.go +++ b/cli/project-runner/internal/projectrunner/status_command_test.go @@ -9,6 +9,7 @@ import ( "fmt" "net" "os" + "path/filepath" "reflect" "sort" "strings" @@ -16,6 +17,7 @@ import ( "time" "github.com/hatayama/unity-cli-loop/common/unityipc" + "github.com/hatayama/unity-cli-loop/common/unityprocess" ) const statusTestAcceptedResponse = `{"jsonrpc":"2.0","id":1,"uloop":{"phase":"accepted"}}` @@ -436,6 +438,65 @@ func TestRunStatusCommandReportsNotRunning(t *testing.T) { assertStatusConnectionError(t, report, "") } +// Verifies NotRunning says the Editor did not shut down normally when it left its lockfile behind, and +// points at the Editor log before the launch step, because the next launch renames that log. +func TestRunStatusCommandReportsAnAbnormalExitWhenTheLockfileWasLeftBehind(t *testing.T) { + projectRoot := writeFakeUnityProject(t) + lockfilePath := unityprocess.LockfilePath(projectRoot) + if err := os.MkdirAll(filepath.Dir(lockfilePath), 0o755); err != nil { + t.Fatalf("creating Temp failed: %v", err) + } + if err := os.WriteFile(lockfilePath, []byte{}, 0o644); err != nil { + t.Fatalf("writing the lockfile failed: %v", err) + } + home := t.TempDir() + t.Setenv("HOME", home) + t.Setenv("USERPROFILE", home) + t.Setenv("LOCALAPPDATA", home) + editorLogPath := unityprocess.DefaultEditorLogPath() + if err := os.MkdirAll(filepath.Dir(editorLogPath), 0o755); err != nil { + t.Fatalf("creating the log directory failed: %v", err) + } + logContent := "COMMAND LINE ARGUMENTS:\nSuccessfully changed project path to: " + projectRoot + "\n" + if err := os.WriteFile(editorLogPath, []byte(logContent), 0o644); err != nil { + t.Fatalf("writing the Editor log failed: %v", err) + } + deps := statusTestDeps(t) + deps.isUnityProcessRunning = fakeUnityProcessLookup(t, projectRoot, false, nil) + + run := runStatusForTest(context.Background(), unreachableConnection(projectRoot), nil, deps) + + report := assertStatusReport(t, run, "NotRunning") + message, _ := report["Message"].(string) + if !strings.HasPrefix(message, "No Unity Editor is running for this project.") || !strings.Contains(message, "may have crashed") { + t.Fatalf("Message = %q, want the abnormal-exit message", message) + } + nextActions, _ := report["NextActions"].([]any) + if len(nextActions) != 2 { + t.Fatalf("NextActions = %#v, want the Editor log step and the launch step", report["NextActions"]) + } + if first, _ := nextActions[0].(string); !strings.Contains(first, editorLogPath) { + t.Fatalf("NextActions[0] = %q, want the Editor log step", first) + } + if second, _ := nextActions[1].(string); second != "Start it with uloop launch." { + t.Fatalf("NextActions[1] = %q, want the launch step", second) + } +} + +// Verifies a refused connection is Unreachable rather than NotRunning when an Editor whose project the +// lookup cannot read is running, since that Editor may be this project's and alive. +func TestRunStatusCommandReportsUnreachableWhileAnUnidentifiedEditorRuns(t *testing.T) { + projectRoot := writeFakeUnityProject(t) + deps := statusTestDeps(t) + deps.isUnityProcessRunning = fakeUnityProcessLookup(t, projectRoot, false, unityprocess.ErrUnidentifiedUnityEditor) + + run := runStatusForTest(context.Background(), unreachableConnection(projectRoot), nil, deps) + + report := assertStatusReport(t, run, "Unreachable") + assertStatusMessage(t, report, "The uloop server is not accepting connections, and a running Unity Editor's command line "+ + "does not name its project, so uloop cannot tell whether Unity is running for this project.") +} + // Verifies a refused connection whose process lookup fails is Unreachable, naming the lookup // error and leaving out the process flag it could not determine. func TestRunStatusCommandReportsUnreachableWhenProcessLookupFails(t *testing.T) { diff --git a/cli/project-runner/internal/projectrunner/unity_exit_scripted_server_unix_test.go b/cli/project-runner/internal/projectrunner/unity_exit_scripted_server_unix_test.go new file mode 100644 index 0000000000..1ef4fe178a --- /dev/null +++ b/cli/project-runner/internal/projectrunner/unity_exit_scripted_server_unix_test.go @@ -0,0 +1,109 @@ +//go:build !windows + +package projectrunner + +import ( + "bufio" + "context" + "errors" + "testing" + "time" + + "github.com/hatayama/unity-cli-loop/common/clicore" + clierrors "github.com/hatayama/unity-cli-loop/common/errors" + "github.com/hatayama/unity-cli-loop/common/unityipc" + "github.com/hatayama/unity-cli-loop/common/unityprocess" +) + +// scriptedDropServerConnection serves one connection per entry of results, in order: an empty entry reads +// the request and drops the connection, as a server that went away does, and any other entry answers with +// that result. Every connection after the script is dropped too. +func scriptedDropServerConnection(t *testing.T, results []string) unityipc.Connection { + t.Helper() + listener := newLoopbackIpcListener(t) + go func() { + for index := 0; ; index++ { + conn, err := listener.Accept() + if err != nil { + return + } + if _, err := unityipc.Read(bufio.NewReader(conn)); err == nil && index < len(results) && results[index] != "" { + _ = unityipc.Write(conn, []byte(`{"jsonrpc":"2.0","result":`+results[index]+`,"id":1}`)) + } + _ = conn.Close() + } + }() + return unityipc.Connection{ + ProjectRoot: t.TempDir(), + Endpoint: unityipc.Endpoint{Network: "tcp", Address: listener.Addr().String()}, + } +} + +// Verifies a status answer between two lost connections restarts the play mode wait's exit count, so an +// Editor that answered in between is not taken as gone after only two lookups in total. +func TestWaitForControlPlayModeStateRestartsTheExitCountAfterAnAnswer(t *testing.T) { + originalPoll := controlPlayModeStatePoll + controlPlayModeStatePoll = time.Millisecond + t.Cleanup(func() { controlPlayModeStatePoll = originalPoll }) + connection := scriptedDropServerConnection(t, []string{"", `{"IsPlaying":false}`}) + lookups := 0 + clock := time.Unix(1_700_000_000, 0) + exitWatch := clicore.NewUnityExitWatch( + connection.ProjectRoot, + "", + func(context.Context, string) (*unityprocess.UnityProcess, error) { + lookups++ + return nil, nil + }, + func() time.Time { + // Two seconds per reading keeps every lookup past the watch's spacing. + clock = clock.Add(2 * time.Second) + return clock + }, + ) + + _, _, err := waitForControlPlayModeState(context.Background(), connection, "Play", 5*time.Second, exitWatch) + + var notRunning clierrors.UnityNotRunningError + if !errors.As(err, ¬Running) { + t.Fatalf("err = %v, want UnityNotRunningError once the connection keeps dropping", err) + } + if lookups != 3 { + t.Fatalf("lookups = %d, want 3: one before the answer, then two that confirm the exit", lookups) + } +} + +// Verifies a connection that drops after the request reached Unity comes back from the send as +// UnityNotRunningError when no Editor process is left, rather than as a disconnect to wait out. +func TestSendWithTransientConnectionRetryReportsAnEditorThatExitedAfterDispatch(t *testing.T) { + deps := defaultConnectionRetryDeps() + deps.findRunningUnityProcess = noUnityProcessLookup + listener := newLoopbackIpcListener(t) + go func() { + conn, err := listener.Accept() + if err != nil { + return + } + defer func() { _ = conn.Close() }() + if _, err := unityipc.Read(bufio.NewReader(conn)); err != nil { + return + } + accepted := `{"jsonrpc":"2.0","result":{"accepted":true},"uloop":{"phase":"accepted"},"id":1}` + _ = unityipc.Write(conn, []byte(accepted)) + }() + connection := unityipc.Connection{ + ProjectRoot: t.TempDir(), + Endpoint: unityipc.Endpoint{Network: "tcp", Address: listener.Addr().String()}, + } + + outcome, err := sendWithTransientConnectionRetryWithDeps( + context.Background(), connection, "compile", map[string]any{}, nil, 0, deps) + + var notRunning clierrors.UnityNotRunningError + if !errors.As(err, ¬Running) || !notRunning.ExitedDuringCommand { + t.Fatalf("err = %v, want UnityNotRunningError for an exit during the command", err) + } + if !outcome.RequestDispatched { + t.Fatalf("outcome = %+v, want a dispatched request", outcome) + } +} diff --git a/cli/project-runner/internal/projectrunner/unity_exit_waits_test.go b/cli/project-runner/internal/projectrunner/unity_exit_waits_test.go new file mode 100644 index 0000000000..8b2e10dbc8 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/unity_exit_waits_test.go @@ -0,0 +1,106 @@ +package projectrunner + +import ( + "bytes" + "context" + "testing" + "time" + + "github.com/hatayama/unity-cli-loop/common/clicore" +) + +// exitedEditorWatch finds no Editor process and moves its clock a second on every reading, so a wait +// against a server nobody listens on confirms the exit without sleeping. +func exitedEditorWatch(projectRoot string) *clicore.UnityExitWatch { + clock := time.Unix(1_700_000_000, 0) + return clicore.NewUnityExitWatch(projectRoot, "", noUnityProcessLookup, func() time.Time { + clock = clock.Add(time.Second) + return clock + }) +} + +// Verifies the play mode state wait ends with UnityNotRunningError once the Editor is confirmed gone, +// instead of polling until its timeout. +func TestWaitForControlPlayModeStateStopsOnceUnityHasExited(t *testing.T) { + originalPoll := controlPlayModeStatePoll + controlPlayModeStatePoll = time.Millisecond + t.Cleanup(func() { controlPlayModeStatePoll = originalPoll }) + connection := unreachableConnection(t.TempDir()) + + _, completed, err := waitForControlPlayModeState( + context.Background(), + connection, + "Play", + // Long enough that only the exit watch, never the timeout, can end the wait first. + 5*time.Second, + exitedEditorWatch(connection.ProjectRoot), + ) + + if completed { + t.Fatal("a play mode wait whose Editor exited cannot complete") + } + assertUnityExitedDuringCommand(t, err) +} + +// Verifies hot reload's wait for the Editor to settle ends with UnityNotRunningError once the Editor is +// confirmed gone. +func TestWaitForEditorReadyStopsOnceUnityHasExited(t *testing.T) { + connection := unreachableConnection(t.TempDir()) + options := fastEditorReadyWaitOptions() + options.budget = 5 * time.Second + + _, ready, err := waitForEditorReady(context.Background(), connection, options, exitedEditorWatch(connection.ProjectRoot)) + + if ready { + t.Fatal("an Editor that exited cannot become ready") + } + assertUnityExitedDuringCommand(t, err) +} + +// Verifies hot reload's wait for a busy Editor ends with UnityNotRunningError once the Editor is +// confirmed gone. +func TestWaitForBusyEditorStopsOnceUnityHasExited(t *testing.T) { + connection := unreachableConnection(t.TempDir()) + options := fastEditorReadyWaitOptions() + options.budget = 5 * time.Second + var stderr bytes.Buffer + + wait, err := waitForBusyEditor( + context.Background(), + connection, + map[string]any{}, + &stderr, + hotReloadBusyWaitSendDeps(), + options, + exitedEditorWatch(connection.ProjectRoot), + ) + + if wait.ready || wait.answered { + t.Fatalf("an Editor that exited cannot become ready or answer, got %#v", wait) + } + assertUnityExitedDuringCommand(t, err) +} + +// Verifies a pause point wait ends with UnityNotRunningError once polls against a gone server confirm +// that the Editor exited, instead of polling until --timeout. +func TestApplyPausePointWaitPollStopsOnceUnityHasExited(t *testing.T) { + connection := unreachableConnection(t.TempDir()) + state := pausePointWaitLoopState{exitWatch: exitedEditorWatch(connection.ProjectRoot)} + _, pollErr := queryPausePointStatusFromUnity(context.Background(), connection, "marker") + if pollErr == nil { + t.Fatal("a status query against an endpoint nobody listens on must fail") + } + + if _, done, err := applyPausePointWaitPoll(context.Background(), &state, pausePointStatusResponse{}, pollErr, false); done || err != nil { + t.Fatalf("one lookup must not end the wait, got done=%v err=%v", done, err) + } + // The watch's clock moves a second per poll, so a few more polls reach the confirming lookup. + for poll := 0; poll < 5; poll++ { + _, done, err := applyPausePointWaitPoll(context.Background(), &state, pausePointStatusResponse{}, pollErr, false) + if done { + assertUnityExitedDuringCommand(t, err) + return + } + } + t.Fatal("the confirmed exit must end the wait") +} diff --git a/cli/project-runner/internal/projectrunner/unity_process_lookup.go b/cli/project-runner/internal/projectrunner/unity_process_lookup.go new file mode 100644 index 0000000000..fc3a45ccec --- /dev/null +++ b/cli/project-runner/internal/projectrunner/unity_process_lookup.go @@ -0,0 +1,20 @@ +package projectrunner + +import ( + "github.com/hatayama/unity-cli-loop/common/clicore" + "github.com/hatayama/unity-cli-loop/common/unityipc" + "github.com/hatayama/unity-cli-loop/common/unityprocess" +) + +// unityProcessLookup is the lookup every default dependency set uses to find this project's Editor. +// Why a package variable: whole-command tests drive a scripted server through the default +// dependencies, and they must be able to say that an Editor process stands behind it. The tests of +// this package are not parallel. Why the definitive lookup: its callers conclude from a nil result that +// Unity is not running. +var unityProcessLookup = unityprocess.FindRunningUnityProcessDefinitively + +// newUnityExitWatch creates the exit watch for one wait on the connection, using the default Editor +// process lookup and the real clock. +func newUnityExitWatch(connection unityipc.Connection) *clicore.UnityExitWatch { + return clicore.NewUnityExitWatch(connection.ProjectRoot, connection.Endpoint.Address, unityProcessLookup, nil) +} diff --git a/cli/project-runner/internal/projectrunner/unity_process_lookup_helpers_test.go b/cli/project-runner/internal/projectrunner/unity_process_lookup_helpers_test.go new file mode 100644 index 0000000000..92f1f19ba4 --- /dev/null +++ b/cli/project-runner/internal/projectrunner/unity_process_lookup_helpers_test.go @@ -0,0 +1,59 @@ +package projectrunner + +import ( + "context" + "errors" + "os" + "reflect" + "testing" + + "github.com/hatayama/unity-cli-loop/common/unityprocess" +) + +// useUnityProcessLookup replaces the default Editor process lookup for one test. +func useUnityProcessLookup( + t *testing.T, + lookup func(context.Context, string) (*unityprocess.UnityProcess, error), +) { + t.Helper() + original := unityProcessLookup + unityProcessLookup = lookup + t.Cleanup(func() { + unityProcessLookup = original + }) +} + +// useRunningUnityProcess makes the default lookup find an Editor, as a scripted server stands in for one. +// Why this test process's own pid: a focus attempt on it can never raise another application's window. +// The default focus is still the real one, so a test whose command ends in a dial that never reaches +// the server should also stub focusUnityProcess, or it waits out a real focus attempt. +func useRunningUnityProcess(t *testing.T) { + t.Helper() + useUnityProcessLookup(t, runningUnityProcessLookup) +} + +func runningUnityProcessLookup(context.Context, string) (*unityprocess.UnityProcess, error) { + return &unityprocess.UnityProcess{Pid: os.Getpid()}, nil +} + +func noUnityProcessLookup(context.Context, string) (*unityprocess.UnityProcess, error) { + return nil, nil +} + +// Verifies the lookup behind every default dependency set, status included, is the one that reports an +// Editor of unknown project instead of "none", since those callers conclude from "none" that Unity is +// not running. +func TestDefaultUnityProcessLookupDoesNotTakeAnUnidentifiedEditorAsNone(t *testing.T) { + if reflect.ValueOf(unityProcessLookup).Pointer() != reflect.ValueOf(unityprocess.FindRunningUnityProcessDefinitively).Pointer() { + t.Fatal("the default lookup must be unityprocess.FindRunningUnityProcessDefinitively") + } + useUnityProcessLookup(t, func(context.Context, string) (*unityprocess.UnityProcess, error) { + return nil, unityprocess.ErrUnidentifiedUnityEditor + }) + + _, err := defaultStatusCommandDeps().isUnityProcessRunning(context.Background(), t.TempDir()) + + if !errors.Is(err, unityprocess.ErrUnidentifiedUnityEditor) { + t.Fatalf("status lookup err = %v, want the default lookup's answer", err) + } +} diff --git a/cli/project-runner/shared-inputs-stamp.json b/cli/project-runner/shared-inputs-stamp.json index 44a5818d18..466b358c17 100644 --- a/cli/project-runner/shared-inputs-stamp.json +++ b/cli/project-runner/shared-inputs-stamp.json @@ -1,4 +1,4 @@ { "schemaVersion": 1, - "sharedInputsHash": "7b1a205dce89310e512997eef8defeb7ecc4e533" + "sharedInputsHash": "30c21c3eae97d957604131bdd7f60b201fd1b64a" }