tstest/integration: add Windows unattended-mode test coverage

Updates #20751

Signed-off-by: Yaruk Asghar <yaruk@tailscale.com>

.github/actions/windows-slack-payload/action.yml

@@ -18,8 +18,12 @@ inputs:
   tests-label:
     description: Label for the line with the test counts
     required: true
+  suites:
+    description: Lines of "<artifact>=<label suffix>", one count line each from results/<artifact>, or empty for one line over all files
+    required: false
+    default: ""
   expected-files:
-    description: Number of result files the run should have uploaded
+    description: Number of result files the run should have uploaded, per suite when suites is set
     required: true
 
 runs:
@@ -34,34 +38,54 @@ runs:
         HEADER: ${{ inputs.header }}
         STATUS_LABEL: ${{ inputs.status-label }}
         TESTS_LABEL: ${{ inputs.tests-label }}
+        SUITES: ${{ inputs.suites }}
         EXPECTED_FILES: ${{ inputs.expected-files }}
         RUN_URL: ${{ github.server_url }}/${{ github.repository }}/actions/runs/${{ github.run_id }}
       run: |
-        # Read each artifact separately so one unreadable file can't discard the others.
-        rows='[]'; unreadable=0; reported=0
-        while IFS= read -r -d '' f; do
-          reported=$((reported + 1))
-          [ -s "$f" ] || continue
-          case "$(jq -r 'type' "$f" 2>/dev/null)" in
-            array) rows=$(printf '%s\n%s' "$rows" "$(jq -c . "$f")" | jq -s 'add') ;;
-            null)  ;; # ran none of the filtered package
-            *)     unreadable=$((unreadable + 1)) ;;
-          esac
-        done < <(find results -name windows-integration-results.json -print0 2>/dev/null)
-        # Dedupe so a re-shard reporting a test twice can't double every count.
-        rows=$(printf '%s' "$rows" | jq '[ .[] | objects ] | unique_by([.Package, .Test])')
         count() { printf '%s' "$rows" | jq "[ (. // [])[] | select(.Outcome == \"$1\") ] | length"; }
-        retried=$(count retried)
-        passed=$(( $(count pass) + retried ))
-        failed=$(count fail)
-        skipped=$(count skip)
-        executed=$((passed + failed))
-        if [ "$executed" -gt 0 ]; then pct=$(( (passed * 100) / executed )); else pct=0; fi
-        # Names behind the retried count, capped so one bad run can't flood the message.
-        flaky=$(printf '%s' "$rows" | jq -r '
-          [ .[] | select(.Outcome == "retried") | (.Package | sub("^tailscale\\.com/"; "")) + "." + .Test ]
-          | sort
-          | if length > 5 then (.[:5] | join(", ")) + ", and \(length - 5) more" else join(", ") end')
+        # tally counts the results under the directory $1.
+        tally() {
+          # Read each artifact separately so one unreadable file can't discard the others.
+          rows='[]'; unreadable=0; reported=0
+          while IFS= read -r -d '' f; do
+            reported=$((reported + 1))
+            [ -s "$f" ] || continue
+            case "$(jq -r 'type' "$f" 2>/dev/null)" in
+              array) rows=$(printf '%s\n%s' "$rows" "$(jq -c . "$f")" | jq -s 'add') ;;
+              null)  ;; # ran none of the filtered package
+              *)     unreadable=$((unreadable + 1)) ;;
+            esac
+          done < <(find "$1" -name windows-integration-results.json -print0 2>/dev/null)
+          # Dedupe so a re-shard reporting a test twice can't double every count.
+          rows=$(printf '%s' "$rows" | jq '[ .[] | objects ] | unique_by([.Package, .Test])')
+          retried=$(count retried)
+          passed=$(( $(count pass) + retried ))
+          failed=$(count fail)
+          skipped=$(count skip)
+          executed=$((passed + failed))
+          if [ "$executed" -gt 0 ]; then pct=$(( (passed * 100) / executed )); else pct=0; fi
+          # Names behind the retried count, capped so one bad run can't flood the message.
+          flaky=$(printf '%s' "$rows" | jq -r '
+            [ .[] | select(.Outcome == "retried") | (.Package | sub("^tailscale\\.com/"; "")) + "." + .Test ]
+            | sort
+            | if length > 5 then (.[:5] | join(", ")) + ", and \(length - 5) more" else join(", ") end')
+        }
+
+        # One count line per suite, or a single line over every file when no suites are given.
+        dirs=(results); suffixes=("")
+        if [ -n "$SUITES" ]; then
+          dirs=(); suffixes=()
+          while IFS='=' read -r artifact suffix; do
+            [ -n "$artifact" ] || continue
+            dirs+=("results/$artifact"); suffixes+=(" $suffix")
+          done <<< "$SUITES"
+        fi
+        # A job that failed with no failed test in any suite hit a build error or timeout.
+        total_failed=0
+        for dir in "${dirs[@]}"; do
+          tally "$dir"
+          total_failed=$((total_failed + failed))
+        done
 
         case "$RESULT" in
           success)   emoji=":white_check_mark:"; color="good" ;;
@@ -71,42 +95,55 @@ runs:
           *)         emoji=":question:";         color="warning" ;;
         esac
 
-        tests="${passed} passed, ${failed} failed, ${skipped} skipped"
-        if [ "$executed" -gt 0 ]; then
-          tests="${tests} (${pct}%)"
+        lines=""; listed=0
+        for i in "${!dirs[@]}"; do
+          tally "${dirs[$i]}"
+          listed=$((listed + reported))
+          tests="${passed} passed, ${failed} failed, ${skipped} skipped"
+          if [ "$executed" -gt 0 ]; then
+            tests="${tests} (${pct}%)"
+          fi
+          # Anything that makes the numbers incomplete or overstated is said out loud.
+          caveats=""
+          if [ "$unreadable" -gt 0 ]; then
+            caveats="${caveats} — ${unreadable} result file(s) unreadable"
+          fi
+          if [ "$reported" -lt "$EXPECTED_FILES" ]; then
+            caveats="${caveats} — only ${reported} of ${EXPECTED_FILES} result files reported"
+          fi
+          # Retries get their own line below, so they don't warn the count line.
+          if [ "$executed" -eq 0 ]; then
+            caveats="${caveats} — no tests executed"
+          fi
+          if [ "$RESULT" = "failure" ] && [ "$total_failed" -eq 0 ]; then
+            caveats="${caveats} — results may be partial (build error or timeout)"
+          fi
+          tests="${tests}${caveats}"
+          if [ "$failed" -gt 0 ]; then tests_emoji=":x:"
+          elif [ -n "$caveats" ]; then tests_emoji=":warning:"
+          else tests_emoji=":white_check_mark:"; fi
+          [ -z "$lines" ] || lines+=$'\n'
+          lines+="${tests_emoji} ${TESTS_LABEL}${suffixes[$i]}: ${tests}"
+          if [ -n "$flaky" ]; then
+            lines+=$'\n'"    ↳ :warning: Flaky Tests (${retried} passed on retry): ${flaky}"
+          fi
+        done
+        # A result file outside every suite would otherwise drop out of the counts unseen.
+        tally results
+        if [ "$reported" -gt "$listed" ]; then
+          lines+=$'\n'":warning: $((reported - listed)) result file(s) not in any suite"
         fi
-        # Anything that makes the numbers incomplete or overstated is said out loud.
-        caveats=""
-        if [ "$unreadable" -gt 0 ]; then
-          caveats="${caveats} — ${unreadable} result file(s) unreadable"
-        fi
-        if [ "$reported" -lt "$EXPECTED_FILES" ]; then
-          caveats="${caveats} — only ${reported} of ${EXPECTED_FILES} result files reported"
-        fi
-        # Retries get their own line below, so they don't warn the count line.
-        if [ "$executed" -eq 0 ]; then
-          caveats="${caveats} — no tests executed"
-        fi
-        if [ "$RESULT" = "failure" ] && [ "$failed" -eq 0 ]; then
-          caveats="${caveats} — results may be partial (build error or timeout)"
-        fi
-        tests="${tests}${caveats}"
-        if [ "$failed" -gt 0 ]; then tests_emoji=":x:"
-        elif [ -n "$caveats" ]; then tests_emoji=":warning:"
-        else tests_emoji=":white_check_mark:"; fi
 
-        jq -n --arg emoji "$emoji" --arg status "$RESULT" --arg tests_emoji "$tests_emoji" \
-          --arg tests "$tests" --arg flaky "$flaky" --arg retried "$retried" \
+        jq -n --arg emoji "$emoji" --arg status "$RESULT" --arg lines "$lines" \
           --arg title "$TITLE" --arg header "$HEADER" --arg status_label "$STATUS_LABEL" \
-          --arg tests_label "$TESTS_LABEL" --arg color "$color" --arg run_url "$RUN_URL" \
+          --arg color "$color" --arg run_url "$RUN_URL" \
           '{attachments: [{
             title: ($emoji + " " + $title + ": " + $status),
             title_link: $run_url,
             text: (
               $header
               + "\n\n" + (if $status_label == "" then "" else $emoji + " " + $status_label + ": " + $status + "\n" end)
-              + $tests_emoji + " " + $tests_label + ": " + $tests
-              + (if $flaky == "" then "" else "\n    ↳ :warning: Flaky Tests (" + $retried + " passed on retry): " + $flaky end)
+              + $lines
               + "\n\n<" + $run_url + "|View run details>"
             ),
             color: $color,

.github/workflows/windows-nightly.yml

@@ -1,4 +1,4 @@
-# Runs the whole tstest/integration suite against the Windows service three times a night.
+# Runs tstest/integration on the Windows service three times a night, unattended mode on and off.
 name: windows-nightly
 
 on:
@@ -29,16 +29,21 @@ jobs:
       contents: read # for actions/checkout
     runs-on: ci-windows-github-1
     name: Windows nightly (${{ matrix.suite }})
-    timeout-minutes: 30
+    timeout-minutes: 45 # room for a 20m test run plus 10m of retries
     strategy:
       fail-fast: false
       matrix:
         include:
-          # One row per suite, so smoke and regression suites can be added as rows later.
+          # One row per suite or mode, and each needs a line in notify's suites input.
           - suite: integration
             packages: ./tstest/integration
             results_pkg: tailscale.com/tstest/integration
             flags: --run-windows-service-tests
+          # -timeout=20m, so tests that fail slowly can't time out the package and drop the rest.
+          - suite: integration-unattended-off
+            packages: ./tstest/integration
+            results_pkg: tailscale.com/tstest/integration
+            flags: --run-windows-service-tests --windows-unattended=false -timeout=20m
     steps:
     - name: checkout
       uses: actions/checkout@de0fac2e4500dabe0009e67214ff5f5447ce83dd # v6.0.2
@@ -116,8 +121,15 @@ jobs:
         title: Windows Nightly Tests
         header: "*${{ github.ref_name }}*"
         tests-label: smoke and integration tests
-        # One result file per suite row.
+        suites: |
+          windows-nightly-integration-results=(unattended on)
+          windows-nightly-integration-unattended-off-results=(unattended off)
+        # One result file per matrix row.
         expected-files: "1"
+    - name: Print Slack payload
+      if: always()
+      # PR runs don't post, so this shows what a nightly run would send.
+      run: jq . payload.json
     - name: Send Slack notification
       env:
         WEBHOOK: ${{ secrets.WINDOWS_SLACK_WEBHOOK_URL }}

ipn/ipnlocal/profiles.go

@@ -889,6 +889,22 @@ func ReadStartupPrefsForTest(logf logger.Logf, store ipn.StateStore) (ipn.PrefsV
 	return pm.CurrentPrefs(), nil
 }
 
+// ReadUserPrefsForTest reads uid's default profile prefs from disk. It is only used for testing.
+func ReadUserPrefsForTest(logf logger.Logf, store ipn.StateStore, uid ipn.WindowsUserID) (ipn.PrefsView, error) {
+	testenv.AssertInTest()
+	bus := eventbus.New()
+	defer bus.Close()
+	ht := health.NewTracker(bus) // in tests, don't care about the health status
+	pm, err := newProfileManager(store, logf, ht)
+	if err != nil {
+		return ipn.PrefsView{}, err
+	}
+	if _, _, err := pm.SwitchToDefaultProfileForUser(uid); err != nil {
+		return ipn.PrefsView{}, err
+	}
+	return pm.CurrentPrefs(), nil
+}
+
 // newProfileManager creates a new [profileManager] using the provided [ipn.StateStore].
 // It also loads the list of known profiles from the store.
 func newProfileManager(store ipn.StateStore, logf logger.Logf, health *health.Tracker) (*profileManager, error) {

tstest/integration/integration.go

@@ -23,6 +23,7 @@ import (
 	"net/netip"
 	"os"
 	"os/exec"
+	"os/user"
 	"path"
 	"path/filepath"
 	"regexp"
@@ -65,6 +66,8 @@ var (
 
 	// runWindowsServiceTests enables the Windows service-mode integration tests, on by default in CI.
 	runWindowsServiceTests = flag.Bool("run-windows-service-tests", cibuild.On(), "run Windows service-mode integration tests")
+	// When false, each Windows node keeps an IPN bus watcher open instead, as the GUI does.
+	windowsUnattended = flag.Bool("windows-unattended", true, "pass --unattended to tailscale up on Windows")
 )
 
 // MainError is an error that's set if an error conditions happens outside of a
@@ -761,6 +764,18 @@ func (n *TestNode) diskPrefs() *ipn.Prefs {
 	if err != nil {
 		t.Fatalf("reading prefs, NewFileStore: %v", err)
 	}
+	if runtime.GOOS == "windows" && !*windowsUnattended {
+		// Windows saves a startup profile only in unattended mode, so read the CLI user's profile.
+		u, err := user.Current()
+		if err != nil {
+			t.Fatalf("reading prefs, user.Current: %v", err)
+		}
+		p, err := ipnlocal.ReadUserPrefsForTest(t.Logf, fs, ipn.WindowsUserID(u.Uid))
+		if err != nil {
+			t.Fatalf("reading prefs, ReadUserPrefsForTest: %v", err)
+		}
+		return p.AsStruct()
+	}
 	p, err := ipnlocal.ReadStartupPrefsForTest(t.Logf, fs)
 	if err != nil {
 		t.Fatalf("reading prefs, ReadDiskPrefsForTest: %v", err)
@@ -1056,12 +1071,36 @@ func (n *TestNode) StartDaemonAsIPNGOOS(ipnGOOS string) *Daemon {
 		cmd.Process.Kill()
 		cmd.Process.Wait()
 	})
+	if ipnGOOS == "windows" && !*windowsUnattended {
+		n.watchIPNBusLikeGUI()
+	}
 	return &Daemon{
 		Process: cmd.Process,
 		node:    n,
 	}
 }
 
+// watchIPNBusLikeGUI keeps an IPN bus watcher open for the rest of the test, as the GUI does.
+func (n *TestNode) watchIPNBusLikeGUI() {
+	t := n.env.t
+	t.Helper()
+	var w *local.IPNBusWatcher
+	if err := tstest.WaitFor(90*time.Second, func() (err error) {
+		w, err = n.LocalClient().WatchIPNBus(t.Context(), 0)
+		return err
+	}); err != nil {
+		t.Fatalf("watching IPN bus: %v", err)
+	}
+	t.Cleanup(func() { w.Close() })
+	go func() {
+		for {
+			if _, err := w.Next(); err != nil {
+				return
+			}
+		}
+	}()
+}
+
 func (n *TestNode) MustUp(extraArgs ...string) {
 	t := n.env.t
 	t.Helper()
@@ -1234,12 +1273,15 @@ func (n *TestNode) TailscaleForOutput(arg ...string) *exec.Cmd {
 // It does not start the process.
 func (n *TestNode) Tailscale(arg ...string) *exec.Cmd {
 	isUp := len(arg) > 0 && arg[0] == "up"
-	if isUp && cmp.Or(n.upFlagGOOS, runtime.GOOS) == "windows" {
+	if isUp && *windowsUnattended && cmp.Or(n.upFlagGOOS, runtime.GOOS) == "windows" {
 		isBareUp := len(arg) == 1
 		// --unattended keeps the current profile after the CLI exits; without it
 		// Windows switches to an empty background profile and the node drops to NoState.
-		// TODO(yaruk): also run tests without --unattended; see #20751.
-		if !isBareUp && !slices.Contains(arg, "--unattended") {
+		setsUnattended := slices.ContainsFunc(arg, func(a string) bool {
+			name, _, _ := strings.Cut(a, "=")
+			return name == "--unattended" || name == "-unattended"
+		})
+		if !isBareUp && !setsUnattended {
 			arg = append(arg, "--unattended")
 		}
 	}

tstest/integration/integration_test.go

@@ -1385,6 +1385,61 @@ func TestOneNodeUpWindowsStyle(t *testing.T) {
 	d1.MustCleanShutdown(t)
 }
 
+// TestUnattendedModeOn checks that --unattended keeps a Windows node connected after the CLI exits.
+func TestUnattendedModeOn(t *testing.T) {
+	env, n1, d1 := startUnattendedModeNode(t)
+	n1.MustUp("--unattended=true")
+
+	stayed := regexp.MustCompile(`client disconnected \(.*\): staying on profile `)
+	disconnects := regexp.MustCompile(`client disconnected .*`)
+	if err := tstest.WaitFor(20*time.Second, func() error {
+		logs := env.LogCatcher.logsString()
+		if !stayed.MatchString(logs) {
+			return fmt.Errorf("want a log line matching %#q, got %q", stayed, disconnects.FindAllString(logs, -1))
+		}
+		return nil
+	}); err != nil {
+		t.Fatal(err)
+	}
+	if n := env.Control.InServeMap(); n == 0 {
+		t.Fatalf("no control connection after the CLI exited: in serve map = %d; want 1", n)
+	}
+
+	d1.MustCleanShutdown(t)
+}
+
+// TestUnattendedModeOff checks that a Windows node with no GUI disconnects once the CLI exits.
+func TestUnattendedModeOff(t *testing.T) {
+	env, n1, d1 := startUnattendedModeNode(t)
+	n1.MustUp("--unattended=false")
+
+	if err := tstest.WaitFor(20*time.Second, func() error {
+		if n := env.Control.InServeMap(); n != 0 {
+			return fmt.Errorf("in serve map = %d; want 0", n)
+		}
+		return nil
+	}); err != nil {
+		t.Fatalf("control connection left open after the CLI exited: %v", err)
+	}
+
+	d1.MustCleanShutdown(t)
+}
+
+// startUnattendedModeNode starts a node that acts as Windows, so the tests run on every platform.
+func startUnattendedModeNode(t *testing.T) (*TestEnv, *TestNode, *Daemon) {
+	t.Helper()
+	if !*windowsUnattended {
+		t.Skip("tests unattended mode with no GUI attached (re-run with --windows-unattended)")
+	}
+	tstest.Parallel(t)
+	env := NewTestEnv(t)
+	n1 := NewTestNode(t, env)
+	n1.upFlagGOOS = "windows"
+	d1 := n1.StartDaemonAsIPNGOOS("windows")
+	n1.AwaitResponding()
+	return env, n1, d1
+}
+
 // TestClientSideJailing tests that when one node is jailed for another, the
 // jailed node cannot initiate connections to the other node however the other
 // node can initiate connections to the jailed node.

tstest/integration/service_windows.go

@@ -71,6 +71,10 @@ func (n *TestNode) startWindowsServiceDaemon() *Daemon {
 	})
 
 	proc = n.startService()
+	if !*windowsUnattended {
+		// Before waitServiceReady, so its status polls don't each connect and disconnect the user.
+		n.watchIPNBusLikeGUI()
+	}
 	n.waitServiceReady(90 * time.Second)
 	return &Daemon{node: n, svc: n, Process: proc}
 }